Skip to content

Commit 7fdb5dc

Browse files
authored
Unrolled build for rust-lang#131936
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 parents da93539 + d1fa49b commit 7fdb5dc

File tree

2 files changed

+14
-6
lines changed

2 files changed

+14
-6
lines changed

src/rustdoc-json-types/Cargo.toml

+4-1
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@ edition = "2021"
66
[lib]
77
path = "lib.rs"
88

9+
[features]
10+
default = ["rustc-hash"]
11+
912
[dependencies]
1013
serde = { version = "1.0", features = ["derive"] }
11-
rustc-hash = "1.1.0"
14+
rustc-hash = { version = "1.1.0", optional = true }
1215

1316
[dev-dependencies]
1417
serde_json = "1.0"

src/rustdoc-json-types/lib.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33
//! These types are the public API exposed through the `--output-format json` flag. The [`Crate`]
44
//! struct is the root of the JSON blob and all other items are contained within.
55
6+
#[cfg(not(feature = "rustc-hash"))]
7+
use std::collections::HashMap;
68
use std::path::PathBuf;
79

8-
pub use rustc_hash::FxHashMap;
10+
#[cfg(feature = "rustc-hash")]
11+
use rustc_hash::FxHashMap as HashMap;
912
use serde::{Deserialize, Serialize};
1013

14+
pub type FxHashMap<K, V> = HashMap<K, V>; // re-export for use in src/librustdoc
15+
1116
/// The version of JSON output that this crate represents.
1217
///
1318
/// This integer is incremented with every breaking change to the API,
@@ -30,11 +35,11 @@ pub struct Crate {
3035
pub includes_private: bool,
3136
/// A collection of all items in the local crate as well as some external traits and their
3237
/// items that are referenced locally.
33-
pub index: FxHashMap<Id, Item>,
38+
pub index: HashMap<Id, Item>,
3439
/// Maps IDs to fully qualified paths and other info helpful for generating links.
35-
pub paths: FxHashMap<Id, ItemSummary>,
40+
pub paths: HashMap<Id, ItemSummary>,
3641
/// Maps `crate_id` of items to a crate name and html_root_url if it exists.
37-
pub external_crates: FxHashMap<u32, ExternalCrate>,
42+
pub external_crates: HashMap<u32, ExternalCrate>,
3843
/// A single version number to be used in the future when making backwards incompatible changes
3944
/// to the JSON output.
4045
pub format_version: u32,
@@ -95,7 +100,7 @@ pub struct Item {
95100
/// Some("") if there is some documentation but it is empty (EG `#[doc = ""]`).
96101
pub docs: Option<String>,
97102
/// This mapping resolves [intra-doc links](https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md) from the docstring to their IDs
98-
pub links: FxHashMap<String, Id>,
103+
pub links: HashMap<String, Id>,
99104
/// Stringified versions of the attributes on this item (e.g. `"#[inline]"`)
100105
pub attrs: Vec<String>,
101106
/// Information about the item’s deprecation, if present.

0 commit comments

Comments
 (0)