all: Switch to deterministic BTreeMap/BTreeSet #235
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ReillyBrogan
force-pushed
the
fix-determinism
branch
from
9ed240e
to
a6a0ad0
Compare
ReillyBrogan
force-pushed
the
fix-determinism
branch
from
a6a0ad0
to
e031725
Compare
ReillyBrogan
changed the title
|
@tarkah Your input on this would be appreciated. I don't see any reason why it shouldn't go in, but maybe I'm missing something...? |
|
This sounds good in theory. Let me dig through each change tomorrow to confirm there's no sneaky regressions due to change from Hash/Eq to Ord |
Any luck with this... @tarkah ? |
While testing ccache I noticed that cache hit rate was extremely low. After some debugging I noticed that the CFLAGS and other environmental variables and macro scripts were not being set deterministically. This is problematic for ccache as it hashes the compiler arguments as part of the key and the order matters to get the same key on a future run. The cause of this non-determinism is the Rust stdlib HashMap and HashSet, for which the default hasher used a random seed. To solve this switch to BTreeMap and BTreeSet from the stdlib. Since non-determinism is just generally a bad idea for build tools and package management software replace ALL uses of HashMap and HashSet repo-wide. Tested by building a package with ccache enabled and using `ccache --show-stats` in the package recipe to verify that the hit rate was now 100% and that the build stage time had dropped to just the time it spent linking. Signed-off-by: Reilly Brogan <reilly@reillybrogan.com>
ReillyBrogan
force-pushed
the
fix-determinism
branch
from
e031725
to
d018311
Compare
ikeycode
approved these changes
Jun 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While testing ccache I noticed that cache hit rate was extremely low. After some debugging I noticed that the CFLAGS and other environmental variables and macro scripts were not being set deterministically. This is problematic for ccache as it hashes the compiler arguments as part of the key and the order matters to get the same key on a future run.
The cause of this non-determinism is the Rust stdlib HashMap and HashSet, for which the default hasher used a random seed. To solve this switch to BTreeMap and BTreeSet from the stdlib. Since non-determinism is just generally a bad idea for build tools and package management software replace ALL uses of HashMap and HashSet repo-wide.
Tested by building a package with ccache enabled and using
ccache --show-statsin the package recipe to verify that the hit rate was now 100% and that the build stage time had dropped to just the time it spent linking.