rustc: Move to FNV hashing for node/def ids #12635
Conversation
|
This is great. I'm thrilled the new hash framework is already proving useful. |
|
I embarassingly never built this past a stage1 rustc, so this ended up not compiling at all on the bots. The reason for this is that deriving(Hash) expansion did not use type parameters after stage0 (it was changed recently). I added a commit to modify how deriving(Hash) expands if default type parameters are enabled for the local crate. re-r? |
| pub mod NodeMap { | ||
| use collections::HashMap; | ||
| pub fn new<T>() -> super::NodeMap<T> { | ||
| HashMap::with_hasher(super::FnvHasher) |
There was a problem hiding this comment.
Why isn't HashMap::new implemented for all Hasher types that implement Default?
There was a problem hiding this comment.
Sadly that can't be done because it would resolve in ambiguities:
let mut map = HashMap::new();
map.insert(1u, 2u);What type does the hasher have?
|
The |
|
r=me, but the conditional deriving thing strikes me as a bug that i hope goes away. |
This leverages the new hashing framework and hashmap implementation to provide a much speedier hashing algorithm for node ids and def ids. The hash algorithm used is currentl FNV hashing, but it's quite easy to swap out. I originally implemented hashing as the identity function, but this actually ended up in slowing down rustc compiling libstd from 8s to 13s. I would suspect that this is a result of a large number of collisions. With FNV hashing, we get these timings (compiling with --no-trans, in seconds): | | before | after | |-----------|---------:|--------:| | libstd | 8.324 | 6.703 | | stdtest | 47.674 | 46.857 | | libsyntax | 9.918 | 8.400 |
If #[feature(default_type_parameters)] is enabled for a crate, then deriving(Hash) will expand with Hash<W: Writer> instead of Hash<SipState> so more hash algorithms can be used.
This leverages the new hashing framework and hashmap implementation to provide a much speedier hashing algorithm for node ids and def ids. The hash algorithm used is currentl FNV hashing, but it's quite easy to swap out. I originally implemented hashing as the identity function, but this actually ended up in slowing down rustc compiling libstd from 8s to 13s. I would suspect that this is a result of a large number of collisions. With FNV hashing, we get these timings (compiling with --no-trans, in seconds): | | before | after | |-----------|---------:|--------:| | libstd | 8.324 | 6.703 | | stdtest | 47.674 | 46.857 | | libsyntax | 9.918 | 8.400 |
Correct target_feature completion I changed the `target_feature` to match the description given in rust-lang#12616.
Use `check_attributes` in doc lints Ensures we catch all the places that doc comments could occur, found one that we were currently missing - docs on `extern` items changelog: none
This leverages the new hashing framework and hashmap implementation to provide a
much speedier hashing algorithm for node ids and def ids. The hash algorithm
used is currentl FNV hashing, but it's quite easy to swap out.
I originally implemented hashing as the identity function, but this actually
ended up in slowing down rustc compiling libstd from 8s to 13s. I would suspect
that this is a result of a large number of collisions.
With FNV hashing, we get these timings (compiling with --no-trans, in seconds):