use DashMap instead of Mutex<HashSet> to reduce lock contention #1
Conversation
| m.insert(b); | ||
| p | ||
| let m = Self::get_state(); | ||
| let b = m.entry(Arc::new(val)).or_insert(()); |
There was a problem hiding this comment.
Having to create an Arc here unconditionally seems suboptimal. I also notice that the entry API is gone in the latest dashmap (4.0rc). Do you know how to implement equivalent functionality there?
There was a problem hiding this comment.
I couldn't see a way to avoid the Arc creation without introducing a race, as the dashmap entry api needs the key to acquire the shard's mutex.
Looking at the dashmap 4.0 PR xacrimon/dashmap#81 there are quite a few features remaining to be implemented, of which one insert_if_not_exists_and_get sounds like it would be the equivalent operation. I think its likely it would return a guard similar to https://github.com/xacrimon/dashmap/blob/acrimon/experimental/src/lib.rs#L136.
There was a problem hiding this comment.
This is beautiful, thank you so much for this contribution! Do you know, by any chance, how this PR affects the single-threaded performance?
You're welcome. I think we probably hit the internment race the same week, thanks for the alternative crate!
I've not run any single threaded benchmarks. Going by the DashMap benchmark graphs it's probably ok:
There was a problem hiding this comment.
Thanks for the clarifications! I plan to merge and publish this later today or tomorrow. I wanted to add a sequential performance test first.
PS. I think there is a way to further reduce contention by not taking a lock on each reference decrement, but I haven't had a chance to think this through yet.
Add a single-threaded test.
Document the DashMap-based API in README.md.
|
I notice 15% drop in performance in a purely sequential test, but concurrent test goes 3x faster, so I think the use if |
|
I just published it on crates.io as |
I found that in my multithreaded use case ArcIntern was heavily contended on the single mutex per interned type.
Switching from Mutex to DashMap reduced contention as DashMap shards the keys under a number of mutexes.
Benefits will obviously will depend on your use case, but for me the time spent in ArcIntern::new or ArcIntern::drop has been reduced to between half and one-third of the original time.