Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small linear factor remaining #3

Open
jonhoo opened this issue Jun 30, 2020 · 2 comments
Open

Small linear factor remaining #3

jonhoo opened this issue Jun 30, 2020 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@jonhoo
Copy link
Owner

jonhoo commented Jun 30, 2020

Even though griddle spreads out most of the cost of the resize, there is still a non-trivial additional cost at the time of the resize that appears to be proportional to the size of the map. That's unfortunate, and we should try to fix it.

I believe this is due to the code here:

griddle/src/lib.rs

Lines 775 to 782 in b2a063c

// It is safe to continue to access this iterator because:
// - we have not de-allocated the table it points into
// - we have not grown or shrunk the table it points into
//
// NOTE: Calling next here could be expensive, as the iter needs to search for the
// next non-empty bucket. as the map grows in size, that search time will increase
// linearly.
if let Some(e) = lo.items.next() {

Specifically, the first time we try to carry elements from the old map to the new, we need to find the first non-empty bucket, which may actually take a while as the map grows. I wonder if hashmap could somehow keep track of the index of the first non-empty bucket?

@jonhoo jonhoo added help wanted Extra attention is needed question Further information is requested enhancement New feature or request labels Jun 30, 2020
@Amanieu
Copy link

Amanieu commented Jun 30, 2020

Hashbrown iterators work in groups of 16 elements, so finding the first full bucket takes constant time if it is within those first 16 elements. Since the load factor is 87.5%, you would need a very large table for for this to become an issue.

@jonhoo
Copy link
Owner Author

jonhoo commented Jun 30, 2020

Yeah, it only really becomes visible around ~500k elements:
https://github.com/jonhoo/griddle/blob/master/misc/vroom.png

It also increases as the table grows, although even at ~1.7M element the time it takes is pretty small. I more raised it as it seems to be the only remaining operation that takes time proportional to the size of the map.

jonhoo pushed a commit that referenced this issue Oct 19, 2024
`cargo test --all-features` does not run doc-tests. For more information
see rust-lang/cargo#6669.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants