Commit ab7e0d0
authored
Rollup merge of rust-lang#131936 - jalil-salame:rustdoc-types-rustc-hash, r=aDotInTheVoid
feat(rustdoc-json-types): introduce rustc-hash feature
This allows the public `rustdoc-types` crate to expose this feature easily and allows consumers of the crate to get the performance advantages from doing so.
The reasoning for this was discussed on [Zulip][1]
Changes:
- Make `rustc-hash` optional but default to including it
- Rename all occurrences of `FxHashMap` to `HashMap`.
- Feature gate the import and rename the imported `FxHashMap` to `HashMap`
- Introduce a type alias `FxHashMap` which resolves to the currently used `HashMap` (`rustc_hash::FxHashMap` or `std::collections::HashMap`) for use in `src/librustdoc`.
[1]: https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/rustc-hash.20and.20performance.20of.20rustdoc-types
**extra context from the zulip thread:**
- `@obi1kenobi` requested benchmarks of the switch to `rustc-hash`
- I benchmarked switching `rustdoc-types` to `rustc-hash` which yielded a ~300ms improvement to `cargo-semver-checks`'s index building step (this step is done twice so the improvements are ~150ms per index).
- The benchmarks were presented in Zulip and people were in favor of introducing `rustc-hash` to the public `rustdoc-types` crate.
- There were differing opinions on how to introduce the dependency:
1. "Hard" dependency: remove use of `std::collections::HashMap` in favor of `FxHashMap`.
2. "Soft" dependency: make optional and introduce a feature then enable/disable it by default (this PR).
3. ~~Make `rustdoc-types` generic and expose the `RandomState`~~ (a lot of work & complexity for little gain over a feature gate).
`@obi1kenobi` and I prefer the feature gate so that is what I am adding here.
My reasons for the preference are:
- `cargo-semver-checks` is especially perf sensitive, we don't expect people to care about ~150ms extra time when reading in a 500MB file (the size of the sample we used for benchmarking).
- Keeping `rustdoc-types` lean by having its only direct dependency be `serde` is nice for the general consumer of the crate.
- `rustc-hash` is not HashDOS resistant (but it is questionable whether `rustdoc-types` would be used on adversarial inputs).
r? `@aDotInTheVoid`2 files changed
+14
-6
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
9 | 12 | | |
10 | 13 | | |
11 | | - | |
| 14 | + | |
12 | 15 | | |
13 | 16 | | |
14 | 17 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
| 7 | + | |
6 | 8 | | |
7 | 9 | | |
8 | | - | |
| 10 | + | |
| 11 | + | |
9 | 12 | | |
10 | 13 | | |
| 14 | + | |
| 15 | + | |
11 | 16 | | |
12 | 17 | | |
13 | 18 | | |
| |||
30 | 35 | | |
31 | 36 | | |
32 | 37 | | |
33 | | - | |
| 38 | + | |
34 | 39 | | |
35 | | - | |
| 40 | + | |
36 | 41 | | |
37 | | - | |
| 42 | + | |
38 | 43 | | |
39 | 44 | | |
40 | 45 | | |
| |||
95 | 100 | | |
96 | 101 | | |
97 | 102 | | |
98 | | - | |
| 103 | + | |
99 | 104 | | |
100 | 105 | | |
101 | 106 | | |
| |||
0 commit comments