Skip to content
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.

Sign up for GitHub

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

Stop cloning Context so much #133345

Merged
merged 8 commits into from
Dec 2, 2024
Merged

Stop cloning Context so much #133345

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8047340
Stop cloning `Context` so much
GuillaumeGomez Nov 21, 2024
46afbc0
Split ID maps in two parts: the constant one and the updated one
GuillaumeGomez Nov 23, 2024
2e242f8
Store default ID map in a static
GuillaumeGomez Nov 25, 2024
5fa1653
Move `SharedContext` out of `Rc`
GuillaumeGomez Nov 25, 2024
5f9e716
Add documentation for new `FormatRenderer` trait items
GuillaumeGomez Nov 27, 2024
e60a7a4
Add `unreachable!` in new `FormatRenderer` methods for `JsonRenderer`…
GuillaumeGomez Nov 28, 2024
b492170
Rename `FormatRenderer::InfoType` into `ModuleData` and rename `Forma…
GuillaumeGomez Nov 28, 2024
69ed026
Fix link to `FormatRenderer` method
GuillaumeGomez Dec 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Split ID maps in two parts: the constant one and the updated one
  • Loading branch information
@GuillaumeGomez
GuillaumeGomez committed Dec 1, 2024
commit 46afbc0588316cb3e5def6c8b4ccbaefdfaedb7f
118 changes: 57 additions & 61 deletions src/librustdoc/html/markdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ use std::iter::Peekable;
use std::ops::{ControlFlow, Range};
use std::path::PathBuf;
use std::str::{self, CharIndices};
use std::sync::OnceLock;

use pulldown_cmark::{
BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, TagEnd, html,
Expand Down Expand Up @@ -1883,83 +1882,81 @@ pub(crate) fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec<Rust
#[derive(Clone, Default, Debug)]
pub struct IdMap {
map: FxHashMap<Cow<'static, str>, usize>,
defined_ids: FxHashSet<&'static str>,
existing_footnotes: usize,
}

// The map is pre-initialized and cloned each time to avoid reinitializing it repeatedly.
static DEFAULT_ID_MAP: OnceLock<FxHashSet<Cow<'static, str>>> = OnceLock::new();

fn init_id_map() -> FxHashSet<Cow<'static, str>> {
fn init_id_map() -> FxHashSet<&'static str> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does performance compare if this is a function that returns bool?

fn is_builtin_id(id: &str) -> bool {
    match id {
        // This is the list of IDs used in JavaScript.
		| "help"
		| "settings"
		| "not-displayed"
		| "alternative-display"
		| "search"
		| "crate-search"
		| "crate-search-div"
		// This is the list of IDs used in HTML generated in Rust (including the ones
		// used in tera template files).
		| "themeStyle"
		| "settings-menu"
		| "help-button"
		| "sidebar-button"
		| "main-content"
		| "toggle-all-docs"
		| "all-types"
		| "default-settings"
		| "sidebar-vars"
		| "copy-path"
		| "rustdoc-toc"
		| "rustdoc-modnav"
		// This is the list of IDs used by rustdoc sections (but still generated by
		// rustdoc).
		| "fields"
		| "variants"
		| "implementors-list"
		| "synthetic-implementors-list"
		| "foreign-impls"
		| "implementations"
		| "trait-implementations"
		| "synthetic-implementations"
		| "blanket-implementations"
		| "required-associated-types"
		| "provided-associated-types"
		| "provided-associated-consts"
		| "required-associated-consts"
		| "required-methods"
		| "provided-methods"
		| "dyn-compatibility"
		| "implementors"
		| "synthetic-implementors"
		| "implementations-list"
		| "trait-implementations-list"
		| "synthetic-implementations-list"
		| "blanket-implementations-list"
		| "deref-methods"
		| "layout"
		| "aliased-type"
		=> true,
		_ => false,
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

matches!. Let's find out in the next set of ideas I have as follow-up?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine. It can be tried as a follow-up PR.

let mut map = FxHashSet::default();
// This is the list of IDs used in JavaScript.
map.insert("help".into());
map.insert("settings".into());
map.insert("not-displayed".into());
map.insert("alternative-display".into());
map.insert("search".into());
map.insert("crate-search".into());
map.insert("crate-search-div".into());
map.insert("help");
map.insert("settings");
map.insert("not-displayed");
map.insert("alternative-display");
map.insert("search");
map.insert("crate-search");
map.insert("crate-search-div");
// This is the list of IDs used in HTML generated in Rust (including the ones
// used in tera template files).
map.insert("themeStyle".into());
map.insert("settings-menu".into());
map.insert("help-button".into());
map.insert("sidebar-button".into());
map.insert("main-content".into());
map.insert("toggle-all-docs".into());
map.insert("all-types".into());
map.insert("default-settings".into());
map.insert("sidebar-vars".into());
map.insert("copy-path".into());
map.insert("rustdoc-toc".into());
map.insert("rustdoc-modnav".into());
map.insert("themeStyle");
map.insert("settings-menu");
map.insert("help-button");
map.insert("sidebar-button");
map.insert("main-content");
map.insert("toggle-all-docs");
map.insert("all-types");
map.insert("default-settings");
map.insert("sidebar-vars");
map.insert("copy-path");
map.insert("rustdoc-toc");
map.insert("rustdoc-modnav");
// This is the list of IDs used by rustdoc sections (but still generated by
// rustdoc).
map.insert("fields".into());
map.insert("variants".into());
map.insert("implementors-list".into());
map.insert("synthetic-implementors-list".into());
map.insert("foreign-impls".into());
map.insert("implementations".into());
map.insert("trait-implementations".into());
map.insert("synthetic-implementations".into());
map.insert("blanket-implementations".into());
map.insert("required-associated-types".into());
map.insert("provided-associated-types".into());
map.insert("provided-associated-consts".into());
map.insert("required-associated-consts".into());
map.insert("required-methods".into());
map.insert("provided-methods".into());
map.insert("dyn-compatibility".into());
map.insert("implementors".into());
map.insert("synthetic-implementors".into());
map.insert("implementations-list".into());
map.insert("trait-implementations-list".into());
map.insert("synthetic-implementations-list".into());
map.insert("blanket-implementations-list".into());
map.insert("deref-methods".into());
map.insert("layout".into());
map.insert("aliased-type".into());
map.insert("fields");
map.insert("variants");
map.insert("implementors-list");
map.insert("synthetic-implementors-list");
map.insert("foreign-impls");
map.insert("implementations");
map.insert("trait-implementations");
map.insert("synthetic-implementations");
map.insert("blanket-implementations");
map.insert("required-associated-types");
map.insert("provided-associated-types");
map.insert("provided-associated-consts");
map.insert("required-associated-consts");
map.insert("required-methods");
map.insert("provided-methods");
map.insert("dyn-compatibility");
map.insert("implementors");
map.insert("synthetic-implementors");
map.insert("implementations-list");
map.insert("trait-implementations-list");
map.insert("synthetic-implementations-list");
map.insert("blanket-implementations-list");
map.insert("deref-methods");
map.insert("layout");
map.insert("aliased-type");
map
}

impl IdMap {
pub fn new() -> Self {
let mut id_map = IdMap { map: FxHashMap::default(), existing_footnotes: 0 };
id_map.init_map();
id_map
}

#[allow(rustc::potential_query_instability)]
fn init_map(&mut self) {
for key in DEFAULT_ID_MAP.get_or_init(init_id_map).iter() {
self.map.insert(key.clone(), 1);
}
IdMap { map: FxHashMap::default(), defined_ids: init_id_map(), existing_footnotes: 0 }
}

pub(crate) fn derive<S: AsRef<str> + ToString>(&mut self, candidate: S) -> String {
let id = match self.map.get_mut(candidate.as_ref()) {
None => candidate.to_string(),
None => {
let candidate = candidate.to_string();
if self.defined_ids.contains(candidate.as_str()) {
let id = format!("{}-{}", candidate, 1);
self.map.insert(candidate.into(), 2);
id
} else {
candidate
}
}
Some(a) => {
let id = format!("{}-{}", candidate.as_ref(), *a);
*a += 1;
Expand All @@ -1982,7 +1979,6 @@ impl IdMap {

pub(crate) fn clear(&mut self) {
self.map.clear();
self.init_map();
self.existing_footnotes = 0;
}
}
6 changes: 0 additions & 6 deletions src/librustdoc/html/render/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,6 @@ impl ContextInfo {
}
}

// `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
#[cfg(all(not(windows), target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Context<'_>, 192);
#[cfg(all(windows, target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Context<'_>, 200);

/// Shared mutable state used in [`Context`] and elsewhere.
pub(crate) struct SharedContext<'tcx> {
pub(crate) tcx: TyCtxt<'tcx>,
Expand Down