Skip to content

serde support #6

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

Closed
anna-is-cute opened this issue Oct 30, 2018 · 6 comments
Closed

serde support #6

anna-is-cute opened this issue Oct 30, 2018 · 6 comments

Comments

@anna-is-cute
Copy link
Contributor

A feature that enables these Hash{Map,Set}s to be used with serde (like std::collections) would be awesome.

@hcpl
Copy link
Contributor

hcpl commented Oct 30, 2018

See https://github.com/Amanieu/hashbrown/issues/9 as an alternative solution to this issue, because serde already provides:

So hashbrown only needs to expose its hasher for serde (and many other crates) to work!

Nevermind, hashbrown is a new implementation of HashMap and HashSet that is generic over any hasher as well. In that case I'll send a PR for serde support sometime today.

@hcpl hcpl mentioned this issue Oct 31, 2018
bors bot referenced this issue Oct 31, 2018
14: Add Serde support r=Amanieu a=hcpl

The code is taken as is from upstream Serde impls for `std::collections::{HashMap, HashSet}` barring few syntactic sugar changes.

Closes https://github.com/Amanieu/hashbrown/issues/6.

Co-authored-by: hcpl <[email protected]>
@bors bors bot closed this as completed in #14 Oct 31, 2018
@clbarnes
Copy link

#14 was merged before the 0.1.5 release, but I still can't #[derive(Deserialize)] on structs containing hashbrown::HashMaps in the same way that I can hashmaps from std, fxhash, or fnv. Is this expected?

error[E0277]: the trait bound `hashbrown::HashMap<T, T>: serde::Deserialize<'_>` is not satisfied
  --> src/main.rs:32:10
   |
32 | #[derive(Deserialize)]
   |          ^^^^^^^^^^^ the trait `serde::Deserialize<'_>` is not implemented for `hashbrown::HashMap<T, T>`
   |
   = note: required by `serde::de::SeqAccess::next_element`

error[E0277]: the trait bound `hashbrown::HashMap<T, T>: serde::Deserialize<'_>` is not satisfied
  --> src/main.rs:32:10
   |
32 | #[derive(Deserialize)]
   |          ^^^^^^^^^^^ the trait `serde::Deserialize<'_>` is not implemented for `hashbrown::HashMap<T, T>`
   |
   = note: required by `serde::de::MapAccess::next_value`

@hcpl
Copy link
Contributor

hcpl commented Nov 19, 2018

@clbarnes sorry for a very late response, but if your issue hasn't been resolved yet, could you please share code that produces these errors?

Is this expected?

The Deserialize impl for hashbrown::HashMap is a port from upstream implementation for std HashMap (https://docs.rs/serde/1.0.80/src/serde/de/impls.rs.html#1192-1236), so if your code worked for std, fxhash, or fnv then hashbrown should have worked out of the box too.

@clbarnes
Copy link

clbarnes commented Nov 19, 2018

Sure thing!

https://github.com/clbarnes/hash_test/tree/master

As the readme says, that repo is in a state which builds and runs (demonstrating that std, fx and fnv all work); uncomment L51 on src/main.rs (which attempts to deserialize a JSON string into a hashbrown::HashMap) and it'll break.

Can't wait to use this library as soon as I can get the deserializing to work (my application is a port from JS and very lookup-heavy, hashbrown gives a 2x speedup over fx); thanks very much for your work.

@hcpl
Copy link
Contributor

hcpl commented Nov 19, 2018

Oh, Serde support was made optional for hashbrown to reduce compile times for those who don't need it. So to make your code compile, just replace

hashbrown = "0.1.6"

with

hashbrown = { version = "0.1.6", features = ["serde"] }

The reason why Serde support worked for other cases is thanks to the combination of distinct factors:

  • std provides two building blocks for creating hash maps: RandomState hasher implementation (which uses common Hasher/BuildHasher API) and HashMap implementation. Both are designed to be independently replaceable by third-party means.
  • fnv and fxhash provide alternative hasher implementations (which use faster hashing algorithms). For convenience, they provide type aliases FnvHashMap<K, V> and FxHashMap<K, V> that use the standard HashMap implementation so that you didn't have to write HashMap<K, V, FnvBuildHasher> and HashMap<K, V, FxBuildHasher> every single time.
  • hashbrown, on the other hand, provides an alternative HashMap implementation (which is faster than the standard one).
  • serde's definition of Deserialize for the standard HashMap implementation is generic over any hasher. That's why your std_map, fx_map and fnv_map cases all worked. But the definition of Deserialize for hashbrown::HashMap is in hashbrown and needs to be manually switched on.

Hope the explanation is able to clear your confusion!

@clbarnes
Copy link

Got it, didn't think about the feature flag. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants