-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
libstore: Make writable dummy store thread-safe #14038
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
||
namespace nix { | ||
|
||
inline std::size_t hash_value(const StorePath & path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think defining hash_value
is unnecessary if you add
using is_avalanching = std::true_type;
to struct hash<nix::StorePath>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is unrelated:
When using a hash function directly suitable for open addressing, post-mixing can be opted out by via a dedicated hash_is_avalanchingtrait. boost::hash specializations for string types are marked as avalanching.
https://www.boost.org/doc/libs/1_85_0/libs/unordered/doc/html/unordered.html
Also this isn't really the case here.
A hash function is said to have the avalanching property if small changes in the input translate to large changes in the returned hash code
Because we just take the first 8 bytes of the hash part as the hash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also it doesn't help with the build anyway:
boost/container_hash/hash.hpp:537:20: error: no matching function for call to 'hash_value'
537 | return hash_value( val );
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3d843ad
to
ac47cb2
Compare
Tested by building with b_sanitize=thread and running: nix flake prefetch-inputs --store "dummy://?read-only=false" It might make sense to move this utility class out of dummy-store.cc, but it seems fine for now.
This is no longer necessary. This reverts commit 4df60e6.
ac47cb2
to
5915fe3
Compare
Motivation
Tested by building with
b_sanitize=thread
and running:It might make sense to move this utility class out of
dummy-store.cc
,but it seems fine for now.
Context
Fixes #14036
Follow-up to #14023
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.