Only hash keys once

[arthurprs]

Only hash keys once, by taking advantage of hashbrown raw entry api. This is roughly the same as #194.

[xacrimon]

Won't this break on resize AKA the reason the previous PR was reverted?

[arthurprs]

Maybe I'm missing something, but I don't know how that would break. Can you point me to prior discussion?

AFAICT the number of shards and the Hasher are fixed for the lifetime of the DashMap. Thus, the routing result should remain correct for the lifetime of DashMap and so is the hash passed down to the hashbrown shard.

[xacrimon]

lgtm

[thalesfragoso]

I believe this violates stacked borrows, the entry.get_mut() invalidates the *const to the key. Inverting the order of the operations should be enough though.

[Turbo87]

👋 I'm not sure how exactly, but it looks like this change broke the crate download counting code on crates.io (see rust-lang/crates.io#7039). Admittedly, we are us the dashmap API in some interesting ways, but I still didn't expect these changes to have such a big effect on us... 😅

[xacrimon]

hm, interesting. I'll yank and revert for now.

[xacrimon]

@Turbo87 Thanks for reporting. v5.5.2 is now out with this PR reverted. Since the earlier release has some other changes, would you guys mind updating to 5.5.2 and seeing if the issue is still resolved?

[arthurprs]

This is quite surprising. Seems like some bad interaction with dashmap raw-api.

[Turbo87]

would you guys mind updating to 5.5.2 and seeing if the issue is still resolved?

yep, we'll try that. can't promise an exact timeframe yet, but we'll probably test it within the next couple of days.

[arthurprs]

The problem seems to be that crates.io is replacing the Hashmaps shards yielded by the raw-api 😬 which in turn causes them to get a different hasher from Default::default.

Specifically the mem::take(..)
https://github.com/rust-lang/crates.io/blob/5b4b6aae45241c648379ed968e8ed02642db805f/src/downloads_counter.rs#L80
and
https://github.com/rust-lang/crates.io/blob/5b4b6aae45241c648379ed968e8ed02642db805f/src/downloads_counter.rs#L93C5-L93C63

I believe the issue would be prevented by replacing the hashmap with a new hashmap with the same hasher as before. This hasher can be acquired via DashMap::hasher().

[Turbo87]

Since the earlier release has some other changes, would you guys mind updating to 5.5.2 and seeing if the issue is still resolved?

just deployed with v5.5.3 and I can confirm that the issue is still resolved