Skip to content

Reproducibility regression caused by 57edf88 #76496

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
jgalenson opened this issue Sep 8, 2020 · 5 comments · Fixed by #76515
Closed

Reproducibility regression caused by 57edf88 #76496

jgalenson opened this issue Sep 8, 2020 · 5 comments · Fixed by #76515
Assignees
Labels
A-reproducibility Area: Reproducible / deterministic builds C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jgalenson
Copy link

jgalenson commented Sep 8, 2020

My nightly script that checks for reproducible builds of rustc failed this morning. Git bisect pointed me to 57edf88 (#75138).

With that commit, about 12 rlibs have differences in my parallel builds (librustc_privacy is one) and nothing in the diff of the binaries jumps out as an obvious culprit.

@jyn514 jyn514 added A-reproducibility Area: Reproducible / deterministic builds T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Sep 9, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Sep 9, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 9, 2020

The PR you linked to wasn't merged; I think you meant #75138.

@jgalenson
Copy link
Author

Ah, yes, sorry about that. The commit should be correct, though.

@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Sep 9, 2020
@LeSeulArtichaut
Copy link
Contributor

cc @jumbatm @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Sep 9, 2020

oops, yea this is likely

let args = referenced_fields.into_iter().map(|field: String| {
(not sure if there are other hashset/hashmap iterations happening, but that's the only one I found).

So the fix is twofold: First change all the HashSet and HashMap to FxHashSet and FxHashMap where possible, and second, collect to a Vec and sort that Vec before iterating over the set/map

@oli-obk oli-obk added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Sep 9, 2020
@LeSeulArtichaut LeSeulArtichaut added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Sep 9, 2020
@jumbatm
Copy link
Contributor

jumbatm commented Sep 9, 2020

Ah, my bad. I can take this.

@rustbot claim

@bors bors closed this as completed in 98f59bc Sep 10, 2020
@jyn514 jyn514 removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Feb 28, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-reproducibility Area: Reproducible / deterministic builds C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants