PosixSourceAccessor: Use concurrent_flat_map #13478
Conversation
| @@ -90,23 +90,21 @@ bool PosixSourceAccessor::pathExists(const CanonPath & path) | |||
|
|
|||
| std::optional<struct stat> PosixSourceAccessor::cachedLstat(const CanonPath & path) | |||
| { | |||
| static SharedSync<std::unordered_map<Path, std::optional<struct stat>>> _cache; | |||
| using Cache = boost::concurrent_flat_map<Path, std::optional<struct stat>>; | |||
| static Cache cache; | |||
There was a problem hiding this comment.
Is there any reason why this cache is a global? I was really confused by this when reading through the code some time prior.
There was a problem hiding this comment.
Yes that is suspicious, when there can be different ones of these with different roots
There was a problem hiding this comment.
Is there a reason it shouldn't be global? There can be any number of instances of PosixSourceAccessor, but they all represent the same filesystem.
There was a problem hiding this comment.
No they don't, because of the root field.
There was a problem hiding this comment.
Yeah but the root field is included in the cache keys.
There was a problem hiding this comment.
Oh. OK that does work, but I would still prefer less global state. It is just bad luck for things like forking, etc.
Motivation
This should be more performant than
SharedSync<std::unordered_map>.Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.