automatic shrinking of hash table capacity is very expensive #17645
Comments
thestinger
added
A-libs
I-slow
|
If you ask for a hashmap |
|
I think it's trying to be too clever. It will always have problems reminiscent of the segmented stack thrashing issues. It would be better to just have the caller use |
|
cc @pczarn |
|
Automatic shrinking is pretty surprising behavior, and only seems to be hinted at in the rustdoc via the |
|
+1 on explicit calls for more predictable performance. I was quite surprised to learn that this is what it does currently. |
|
+1 same as @mitsuhiko |
|
I don't mind explicit calls. This is a simple approach. Just keep in mind that the capacity of a hash table affects the performance of accesses, even though it's more predicable. (Is it?) |
|
Fixed by #18770. |
|
🎊 |
Fix path resolution for child mods of those expanded by `include!` Child modules wouldn't use the correct candidate paths due to a branch that doesn't seem to be doing what it's intended to do. Removing the branch fixes the problem and all existing test cases pass. Having no knowledge of how any of this works, I believe this fixes rust-lang#17645. Using another test that writes the included mod directly into `lib.rs` instead, I found the difference can be traced to the candidate files we use to look up mods. A separate branch for if the file comes from an `include!` macro doesn't take into account the original mod we're contained within: ```rust None if file_id.macro_file().map_or(false, |it| it.is_include_macro(db.upcast())) => { candidate_files.push(format!("{}.rs", name.display(db.upcast()))); candidate_files.push(format!("{}/mod.rs", name.display(db.upcast()))); } ``` I'm not sure why this branch exists. Tracing the branch back takes us to 3bb9efb but it doesn't say *why* the branch was added. The test case that was added in this commit passes with the branch removed, so I think it's just superfluous at this point.
Fix path resolution for child mods of those expanded by `include!` Child modules wouldn't use the correct candidate paths due to a branch that doesn't seem to be doing what it's intended to do. Removing the branch fixes the problem and all existing test cases pass. Having no knowledge of how any of this works, I believe this fixes rust-lang#17645. Using another test that writes the included mod directly into `lib.rs` instead, I found the difference can be traced to the candidate files we use to look up mods. A separate branch for if the file comes from an `include!` macro doesn't take into account the original mod we're contained within: ```rust None if file_id.macro_file().map_or(false, |it| it.is_include_macro(db.upcast())) => { candidate_files.push(format!("{}.rs", name.display(db.upcast()))); candidate_files.push(format!("{}/mod.rs", name.display(db.upcast()))); } ``` I'm not sure why this branch exists. Tracing the branch back takes us to 3bb9efb but it doesn't say *why* the branch was added. The test case that was added in this commit passes with the branch removed, so I think it's just superfluous at this point.
This interacts poorly with
with_capacitysince it will just end up shrinking the allocation immediately. It's very widely used sinceFromIteruses it with the iterator size hint. Even in code that's not pre-allocating capacity, the resize strategy results in many reallocations as the hash table shrinks and then grows again. Allocator improvements are possible but this is always going to be extremely expensive for huge collections on most platforms.The text was updated successfully, but these errors were encountered: