Remove unused DefPathTable::retrace_path()#43361
Conversation
|
r? @nikomatsakis Since I'm not sure what exact role it played. |
alexcrichton
added
the
S-waiting-on-review
nikomatsakis
left a comment
nikomatsakis
left a comment
There was a problem hiding this comment.
left a nit, but r=me once it is resolved to your satisfaction @michaelwoerister
|
|
||
| pub fn size(&self) -> usize { | ||
| self.key_to_index.len() | ||
| self.index_to_key.iter().map(|v| v.len()).sum() |
There was a problem hiding this comment.
Do you know who calls this? This new version seems significantly more expensive (O(n) vs O(1)), so if it's in any sort of tight-loop, it may be worth keeping a separate counter. I didn't find any obvious callers using ripgrep, but I could easily have missed things...
...if we do keep the O(n) version, maybe rename it to count_keys or something more suggestive that this requires computation? This looks like a "pure getter" to me.
There was a problem hiding this comment.
index_to_key here is an array with two elements as far as I can tell (https://github.com/rust-lang/rust/pull/43361/files#diff-b4047ba27734e3c7e860eb844810a301R38) so probably not an issue.
There was a problem hiding this comment.
@Mark-Simulacrum is right, this is a 2-element array. And it's called only once per upstream crate IIRC. I would not worry about it.
|
@bors r=nikomatsakis |
|
📌 Commit fa91eeb has been approved by |
|
⌛ Testing commit fa91eeb with merge c8b5f48621fa9e450b7d31836fe45c95e26c193f... |
|
💔 Test failed - status-appveyor |
|
@bors rollup |
|
@bors retry |
|
I've filed #43453 for that spurious failure. |
…h, r=nikomatsakis Remove unused DefPathTable::retrace_path() `DefPathTable::retrace_path()` is not used anymore for a while now and removing it also removes the need to build the costly `DefPathTable::key_to_index` map for every upstream crate. cc rust-lang#43300 r? @eddyb
|
Although a bit noisy, this seems the most likely candidate in the rollup it landed in to be the cause of a nearly 5% reduction in memory usage on many benchmarks. 🌮 !! |
|
Nice |
6 participants
DefPathTable::retrace_path()is not used anymore for a while now and removing it also removes the need to build the costlyDefPathTable::key_to_indexmap for every upstream crate.cc #43300
r? @eddyb