Optimized HashMap. RawTable exposes a safe interface. (reopen) #16628
Conversation
| impl DefaultResizePolicy { | ||
| fn new(new_capacity: uint) -> DefaultResizePolicy { | ||
| DefaultResizePolicy { | ||
| minimum_capacity2: new_capacity << 1 |
There was a problem hiding this comment.
So I've never been totally clear on what Rust's policy on what to do about a user requesting, or actually managing to construct catastrophically large datastructures is. It is of course absurd to demand a HashMap with capacity uint::MAX/2+1, but if they do, this will overflow and they'll actually get a HashMap with very low minimum capacity. Is this a concern?
There was a problem hiding this comment.
This was a concern with RawTable, because of a bug in new. But it shouldn't be a concern in hashmap.rs, because constructing RawTable with capacity >= uint::MAX / size_of::<u64>() will fail anyway. In general, it's fine if it doesn't introduce unsafety, and hashmap.rs already uses a safe interface.
There was a problem hiding this comment.
@gankro: For example, the with_capacity method on Vec<T> is often used to obtain a large buffer to write into. Returning a vector of at least that much capacity is part of the contract of the function. It's a bit odd to deviate from that in other containers.
There was a problem hiding this comment.
@thestinger Did you mean to respond to a different comment? I'm not sure how this applies.
pczarn
force-pushed
the
hashmap-opt
branch
2 times, most recently
from
af7245d to
2409e0d
Compare
| table: M | ||
| } | ||
|
|
||
| pub struct EmptyBucket<K, V, M> { |
There was a problem hiding this comment.
Is there any reason why EmptyBucket and FullBucket duplicate the fields of Bucket? Why not just have a wrapper bucket: Bucket<K, V, M> field?
There was a problem hiding this comment.
I've found this way easier to reason about, even if it makes code a little longer.
Introduced a new growth algorithm.
pczarn
force-pushed
the
hashmap-opt
branch
2 times, most recently
from
a6eb263 to
fac1dab
Compare
|
@pczarn so I tracked down the problems that are plaguing you and understand their cause, though I don't yet know either the best way to fix them nor why they were not triggered before. They do not seem related to your branch at all. I filed issue #16994 describing the problem and a much reduced test case. I think we should workaround this issue and land this PR! |
|
I will push a commit to a local branch showing the workaround. |
|
workaround: nikomatsakis@c4bd9da though I think similar errors arise in rustdoc, waiting for build, presumably amenable to a similar workaround |
* branchless `bucket.next()` * robin_hood is a free function * fixed the resize policy that was off by one * documented the growth algorithm * updated documentation after interface changes * removed old fixmes
This is #15720, rebased and reopened. cc @nikomatsakis
|
\o/ Congrats @pczarn! |
This is #15720, rebased and reopened.
cc @nikomatsakis