comparison 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
comparison
equal deleted inserted replaced
47115:1b4f0f819f92 47116:04bcba539c96
167 WithBasename::inclusive_ancestors_of(path); 167 WithBasename::inclusive_ancestors_of(path);
168 let mut ancestor_path = inclusive_ancestor_paths 168 let mut ancestor_path = inclusive_ancestor_paths
169 .next() 169 .next()
170 .expect("expected at least one inclusive ancestor"); 170 .expect("expected at least one inclusive ancestor");
171 loop { 171 loop {
172 // TODO: can we avoid double lookup in all cases without allocating 172 // TODO: can we avoid allocating an owned key in cases where the
173 // an owned key in cases where the map already contains that key? 173 // map already contains that key, without introducing double
174 // lookup?
174 let child_node = 175 let child_node =
175 if child_nodes.contains_key(ancestor_path.base_name()) { 176 child_nodes.entry(ancestor_path.to_owned()).or_default();
176 child_nodes.get_mut(ancestor_path.base_name()).unwrap()
177 } else {
178 // This is always a vacant entry, using `.entry()` lets us
179 // return a `&mut Node` of the newly-inserted node without
180 // yet another lookup. `BTreeMap::insert` doesn’t do this.
181 child_nodes.entry(ancestor_path.to_owned()).or_default()
182 };
183 if let Some(next) = inclusive_ancestor_paths.next() { 177 if let Some(next) = inclusive_ancestor_paths.next() {
184 ancestor_path = next; 178 ancestor_path = next;
185 child_nodes = &mut child_node.children; 179 child_nodes = &mut child_node.children;
186 } else { 180 } else {
187 return child_node; 181 return child_node;