Mercurial > hg
diff rust/hg-core/src/dirstate_tree/dirstate_map.rs @ 47116:04bcba539c96
dirstate-tree: Avoid BTreeMap double-lookup when inserting a dirstate entry
The child nodes of a given node in the tree-shaped dirstate are kept in a
`BTreeMap` where keys are file names as strings. Finding or inserting a value
in the map takes `O(log(n))` string comparisons, which adds up when constructing
the tree.
The `entry` API allows finding a "spot" in the map that may or may not be
occupied and then access that value or insert a new one without doing map
lookup again. However the current API is limited in that calling `entry`
requires an owned key (and so a memory allocation), even if it ends up not
being used in the case where the map already has a value with an equal key.
This is still a win, with 4% better end-to-end time for `hg status` measured
here with hyperfine:
```
Benchmark #1: ../hg2/hg status -R $REPO --config=experimental.dirstate-tree.in-memory=1
Time (mean ± σ): 1.337 s ± 0.018 s [User: 892.9 ms, System: 437.5 ms]
Range (min … max): 1.316 s … 1.373 s 10 runs
Benchmark #2: ./hg status -R $REPO --config=experimental.dirstate-tree.in-memory=1
Time (mean ± σ): 1.291 s ± 0.008 s [User: 853.4 ms, System: 431.1 ms]
Range (min … max): 1.283 s … 1.309 s 10 runs
Summary
'./hg status -R $REPO --config=experimental.dirstate-tree.in-memory=1' ran
1.04 ± 0.02 times faster than '../hg2/hg status -R $REPO --config=experimental.dirstate-tree.in-memory=1'
```
* ./hg is this revision
* ../hg2/hg is its parent
* $REPO is an old snapshot of mozilla-central
Differential Revision: https://phab.mercurial-scm.org/D10550
author | Simon Sapin <simon.sapin@octobus.net> |
---|---|
date | Tue, 27 Apr 2021 12:42:21 +0200 |
parents | be579775c2d9 |
children | c92e63762573 |
line wrap: on
line diff
--- a/rust/hg-core/src/dirstate_tree/dirstate_map.rs Mon Apr 26 19:28:56 2021 +0200 +++ b/rust/hg-core/src/dirstate_tree/dirstate_map.rs Tue Apr 27 12:42:21 2021 +0200 @@ -169,17 +169,11 @@ .next() .expect("expected at least one inclusive ancestor"); loop { - // TODO: can we avoid double lookup in all cases without allocating - // an owned key in cases where the map already contains that key? + // TODO: can we avoid allocating an owned key in cases where the + // map already contains that key, without introducing double + // lookup? let child_node = - if child_nodes.contains_key(ancestor_path.base_name()) { - child_nodes.get_mut(ancestor_path.base_name()).unwrap() - } else { - // This is always a vacant entry, using `.entry()` lets us - // return a `&mut Node` of the newly-inserted node without - // yet another lookup. `BTreeMap::insert` doesn’t do this. - child_nodes.entry(ancestor_path.to_owned()).or_default() - }; + child_nodes.entry(ancestor_path.to_owned()).or_default(); if let Some(next) = inclusive_ancestor_paths.next() { ancestor_path = next; child_nodes = &mut child_node.children;