Evaluate replacing RwLock<HashMap> with DashMap
Categories
(Data Platform and Tools :: Glean: SDK, enhancement, P4)
Tracking
(Not tracked)
People
(Reporter: janerik, Unassigned)
References
(Blocks 1 open bug)
Details
Right now we use RwLock<HashMap> in a bunch of places, in m-c for all the global metric *_MAPs.
In the Glean SDK we also alterantively use Mutex<HashMap>.
That works and is thread-safe, but might not be the most performant as the lock becomes contended.
E.g. bug 1977614 seems to run into some of that.
DashMap implements a "fast concurrent map in Rust" and is a near-drop-in replacement for our use-cases.
We should see if it's worth a switch.
|
Reporter | |
Comment 1•1 year ago
|
||
I launched a perf run here: https://perf.compare/compare-lando-results?baseLando=143042&newLando=143839&baseRepo=try&newRepo=try&framework=13
This uses dashmap for a DUAL_LABELED_METRICS_TO_IDS, see the patch: https://hg-edge.mozilla.org/try/rev/01667b6fbe277770e9ea38df1420bd8fbbb9fc82
It's not an obvious win, but maybe combined with other work could lead to better performance.
Currently the patch relies on the v7.0.0-rc2 of dashmap, due to its use of hashbrown. m-c patches together hashbrown versions and so an older dashmap doesn't compile.
|
|
Reporter | |
Updated•1 year ago
|
|
|
Reporter | |
Comment 2•1 year ago
|
||
Re-did that on top of autoland with Travis' other fixes (disabling the heavy metric): https://perf.compare/compare-lando-results?baseLando=145597&newLando=145598&baseRepo=try&newRepo=try&framework=13
Mixed results (with low confidence). Unsurprising though given it's just a single metric type for now. We might re-run that after changing more metric types.
Dropping this in priority for the moment, given the dashmap version is rc, we wouldn't land it anyway.
Description
•