From 8047340599763eeca43a0dcee1b1f6b65b6d4ecd Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 21 Nov 2024 15:16:36 +0100 Subject: [PATCH] Stop cloning `Context` so much --- src/librustdoc/formats/renderer.rs | 70 +++++++------ src/librustdoc/html/markdown.rs | 113 ++++++++++++--------- src/librustdoc/html/render/context.rs | 75 ++++++++------ src/librustdoc/html/render/print_item.rs | 2 +- src/librustdoc/html/render/write_shared.rs | 2 +- src/librustdoc/html/sources.rs | 4 +- src/librustdoc/json/mod.rs | 6 +- 7 files changed, 153 insertions(+), 119 deletions(-) diff --git a/src/librustdoc/formats/renderer.rs b/src/librustdoc/formats/renderer.rs index edd7d56b1798..f7ba5bff51bf 100644 --- a/src/librustdoc/formats/renderer.rs +++ b/src/librustdoc/formats/renderer.rs @@ -1,5 +1,5 @@ +use rustc_data_structures::profiling::SelfProfilerRef; use rustc_middle::ty::TyCtxt; -use tracing::debug; use crate::clean; use crate::config::RenderOptions; @@ -17,6 +17,7 @@ pub(crate) trait FormatRenderer<'tcx>: Sized { /// /// This is true for html, and false for json. See #80664 const RUN_ON_MODULE: bool; + type InfoType; /// Sets up any state required for the renderer. When this is called the cache has already been /// populated. @@ -28,7 +29,8 @@ pub(crate) trait FormatRenderer<'tcx>: Sized { ) -> Result<(Self, clean::Crate), Error>; /// Make a new renderer to render a child of the item currently being rendered. - fn make_child_renderer(&self) -> Self; + fn make_child_renderer(&mut self) -> Self::InfoType; + fn set_back_info(&mut self, _info: Self::InfoType); /// Renders a single non-module item. This means no recursive sub-item rendering is required. fn item(&mut self, item: clean::Item) -> Result<(), Error>; @@ -47,6 +49,40 @@ pub(crate) trait FormatRenderer<'tcx>: Sized { fn cache(&self) -> &Cache; } +fn run_format_inner<'tcx, T: FormatRenderer<'tcx>>( + cx: &mut T, + item: clean::Item, + prof: &SelfProfilerRef, +) -> Result<(), Error> { + if item.is_mod() && T::RUN_ON_MODULE { + // modules are special because they add a namespace. We also need to + // recurse into the items of the module as well. + let _timer = + prof.generic_activity_with_arg("render_mod_item", item.name.unwrap().to_string()); + + cx.mod_item_in(&item)?; + let (clean::StrippedItem(box clean::ModuleItem(module)) | clean::ModuleItem(module)) = + item.inner.kind + else { + unreachable!() + }; + for it in module.items { + let info = cx.make_child_renderer(); + run_format_inner(cx, it, prof)?; + cx.set_back_info(info); + } + + cx.mod_item_out()?; + // FIXME: checking `item.name.is_some()` is very implicit and leads to lots of special + // cases. Use an explicit match instead. + } else if let Some(item_name) = item.name + && !item.is_extern_crate() + { + prof.generic_activity_with_arg("render_item", item_name.as_str()).run(|| cx.item(item))?; + } + Ok(()) +} + /// Main method for rendering a crate. pub(crate) fn run_format<'tcx, T: FormatRenderer<'tcx>>( krate: clean::Crate, @@ -66,36 +102,8 @@ pub(crate) fn run_format<'tcx, T: FormatRenderer<'tcx>>( } // Render the crate documentation - let mut work = vec![(format_renderer.make_child_renderer(), krate.module)]; + run_format_inner(&mut format_renderer, krate.module, prof)?; - while let Some((mut cx, item)) = work.pop() { - if item.is_mod() && T::RUN_ON_MODULE { - // modules are special because they add a namespace. We also need to - // recurse into the items of the module as well. - let _timer = - prof.generic_activity_with_arg("render_mod_item", item.name.unwrap().to_string()); - - cx.mod_item_in(&item)?; - let (clean::StrippedItem(box clean::ModuleItem(module)) | clean::ModuleItem(module)) = - item.inner.kind - else { - unreachable!() - }; - for it in module.items { - debug!("Adding {:?} to worklist", it.name); - work.push((cx.make_child_renderer(), it)); - } - - cx.mod_item_out()?; - // FIXME: checking `item.name.is_some()` is very implicit and leads to lots of special - // cases. Use an explicit match instead. - } else if let Some(item_name) = item.name - && !item.is_extern_crate() - { - prof.generic_activity_with_arg("render_item", item_name.as_str()) - .run(|| cx.item(item))?; - } - } prof.verbose_generic_activity_with_arg("renderer_after_krate", T::descr()) .run(|| format_renderer.after_krate()) } diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index b6829d5457ba..0f210270f788 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -37,7 +37,7 @@ use std::sync::OnceLock; use pulldown_cmark::{ BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, TagEnd, html, }; -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::{Diag, DiagMessage}; use rustc_hir::def_id::LocalDefId; use rustc_middle::ty::TyCtxt; @@ -1887,65 +1887,74 @@ pub struct IdMap { } // The map is pre-initialized and cloned each time to avoid reinitializing it repeatedly. -static DEFAULT_ID_MAP: OnceLock, usize>> = OnceLock::new(); +static DEFAULT_ID_MAP: OnceLock>> = OnceLock::new(); -fn init_id_map() -> FxHashMap, usize> { - let mut map = FxHashMap::default(); +fn init_id_map() -> FxHashSet> { + let mut map = FxHashSet::default(); // This is the list of IDs used in JavaScript. - map.insert("help".into(), 1); - map.insert("settings".into(), 1); - map.insert("not-displayed".into(), 1); - map.insert("alternative-display".into(), 1); - map.insert("search".into(), 1); - map.insert("crate-search".into(), 1); - map.insert("crate-search-div".into(), 1); + 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()); // This is the list of IDs used in HTML generated in Rust (including the ones // used in tera template files). - map.insert("themeStyle".into(), 1); - map.insert("settings-menu".into(), 1); - map.insert("help-button".into(), 1); - map.insert("sidebar-button".into(), 1); - map.insert("main-content".into(), 1); - map.insert("toggle-all-docs".into(), 1); - map.insert("all-types".into(), 1); - map.insert("default-settings".into(), 1); - map.insert("sidebar-vars".into(), 1); - map.insert("copy-path".into(), 1); - map.insert("rustdoc-toc".into(), 1); - map.insert("rustdoc-modnav".into(), 1); + 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()); // This is the list of IDs used by rustdoc sections (but still generated by // rustdoc). - map.insert("fields".into(), 1); - map.insert("variants".into(), 1); - map.insert("implementors-list".into(), 1); - map.insert("synthetic-implementors-list".into(), 1); - map.insert("foreign-impls".into(), 1); - map.insert("implementations".into(), 1); - map.insert("trait-implementations".into(), 1); - map.insert("synthetic-implementations".into(), 1); - map.insert("blanket-implementations".into(), 1); - map.insert("required-associated-types".into(), 1); - map.insert("provided-associated-types".into(), 1); - map.insert("provided-associated-consts".into(), 1); - map.insert("required-associated-consts".into(), 1); - map.insert("required-methods".into(), 1); - map.insert("provided-methods".into(), 1); - map.insert("dyn-compatibility".into(), 1); - map.insert("implementors".into(), 1); - map.insert("synthetic-implementors".into(), 1); - map.insert("implementations-list".into(), 1); - map.insert("trait-implementations-list".into(), 1); - map.insert("synthetic-implementations-list".into(), 1); - map.insert("blanket-implementations-list".into(), 1); - map.insert("deref-methods".into(), 1); - map.insert("layout".into(), 1); - map.insert("aliased-type".into(), 1); + 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 } impl IdMap { pub fn new() -> Self { - IdMap { map: DEFAULT_ID_MAP.get_or_init(init_id_map).clone(), existing_footnotes: 0 } + 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); + } } pub(crate) fn derive + ToString>(&mut self, candidate: S) -> String { @@ -1970,4 +1979,10 @@ impl IdMap { closure(self, &mut existing_footnotes); self.existing_footnotes = existing_footnotes; } + + pub(crate) fn clear(&mut self) { + self.map.clear(); + self.init_map(); + self.existing_footnotes = 0; + } } diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 3a8144621747..0b87c1bb62ca 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -49,10 +49,6 @@ pub(crate) struct Context<'tcx> { /// The current destination folder of where HTML artifacts should be placed. /// This changes as the context descends into the module hierarchy. pub(crate) dst: PathBuf, - /// A flag, which when `true`, will render pages which redirect to the - /// real location of an item. This is used to allow external links to - /// publicly reused items to redirect to the right location. - pub(super) render_redirect_pages: bool, /// Tracks section IDs for `Deref` targets so they match in both the main /// body and the sidebar. pub(super) deref_id_map: DefIdMap, @@ -64,16 +60,33 @@ pub(crate) struct Context<'tcx> { /// /// [#82381]: https://github.com/rust-lang/rust/issues/82381 pub(crate) shared: Rc>, + /// Collection of all types with notable traits referenced in the current module. + pub(crate) types_with_notable_traits: FxIndexSet, + /// Contains information that needs to be saved and reset after rendering an item which is + /// not a module. + pub(crate) info: ContextInfo, +} + +#[derive(Clone, Copy)] +pub(crate) struct ContextInfo { + /// A flag, which when `true`, will render pages which redirect to the + /// real location of an item. This is used to allow external links to + /// publicly reused items to redirect to the right location. + pub(super) render_redirect_pages: bool, /// This flag indicates whether source links should be generated or not. If /// the source files are present in the html rendering, then this will be /// `true`. pub(crate) include_sources: bool, - /// Collection of all types with notable traits referenced in the current module. - pub(crate) types_with_notable_traits: FxIndexSet, /// Field used during rendering, to know if we're inside an inlined item. pub(crate) is_inside_inlined_module: bool, } +impl ContextInfo { + fn new(include_sources: bool) -> Self { + Self { render_redirect_pages: false, include_sources, is_inside_inlined_module: false } + } +} + // `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); @@ -174,14 +187,16 @@ impl<'tcx> Context<'tcx> { } fn render_item(&mut self, it: &clean::Item, is_module: bool) -> String { - let mut render_redirect_pages = self.render_redirect_pages; + let mut render_redirect_pages = self.info.render_redirect_pages; // If the item is stripped but inlined, links won't point to the item so no need to generate // a file for it. if it.is_stripped() && let Some(def_id) = it.def_id() && def_id.is_local() { - if self.is_inside_inlined_module || self.shared.cache.inlined_items.contains(&def_id) { + if self.info.is_inside_inlined_module + || self.shared.cache.inlined_items.contains(&def_id) + { // For now we're forced to generate a redirect page for stripped items until // `record_extern_fqn` correctly points to external items. render_redirect_pages = true; @@ -441,6 +456,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { } const RUN_ON_MODULE: bool = true; + type InfoType = ContextInfo; fn init( krate: clean::Crate, @@ -562,13 +578,11 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { let mut cx = Context { current: Vec::new(), dst, - render_redirect_pages: false, id_map, deref_id_map: Default::default(), shared: Rc::new(scx), - include_sources, types_with_notable_traits: FxIndexSet::default(), - is_inside_inlined_module: false, + info: ContextInfo::new(include_sources), }; if emit_crate { @@ -582,18 +596,15 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { Ok((cx, krate)) } - fn make_child_renderer(&self) -> Self { - Self { - current: self.current.clone(), - dst: self.dst.clone(), - render_redirect_pages: self.render_redirect_pages, - deref_id_map: Default::default(), - id_map: IdMap::new(), - shared: Rc::clone(&self.shared), - include_sources: self.include_sources, - types_with_notable_traits: FxIndexSet::default(), - is_inside_inlined_module: self.is_inside_inlined_module, - } + fn make_child_renderer(&mut self) -> Self::InfoType { + self.deref_id_map.clear(); + self.id_map.clear(); + self.types_with_notable_traits.clear(); + self.info + } + + fn set_back_info(&mut self, info: Self::InfoType) { + self.info = info; } fn after_krate(&mut self) -> Result<(), Error> { @@ -775,8 +786,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { // External crates will provide links to these structures, so // these modules are recursed into, but not rendered normally // (a flag on the context). - if !self.render_redirect_pages { - self.render_redirect_pages = item.is_stripped(); + if !self.info.render_redirect_pages { + self.info.render_redirect_pages = item.is_stripped(); } let item_name = item.name.unwrap(); self.dst.push(item_name.as_str()); @@ -793,19 +804,19 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { self.shared.fs.write(joint_dst, buf)?; } } - if !self.is_inside_inlined_module { + if !self.info.is_inside_inlined_module { if let Some(def_id) = item.def_id() && self.cache().inlined_items.contains(&def_id) { - self.is_inside_inlined_module = true; + self.info.is_inside_inlined_module = true; } } else if !self.cache().document_hidden && item.is_doc_hidden() { // We're not inside an inlined module anymore since this one cannot be re-exported. - self.is_inside_inlined_module = false; + self.info.is_inside_inlined_module = false; } // Render sidebar-items.js used throughout this module. - if !self.render_redirect_pages { + if !self.info.render_redirect_pages { let (clean::StrippedItem(box clean::ModuleItem(ref module)) | clean::ModuleItem(ref module)) = item.kind else { @@ -836,8 +847,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { // External crates will provide links to these structures, so // these modules are recursed into, but not rendered normally // (a flag on the context). - if !self.render_redirect_pages { - self.render_redirect_pages = item.is_stripped(); + if !self.info.render_redirect_pages { + self.info.render_redirect_pages = item.is_stripped(); } let buf = self.render_item(&item, false); @@ -850,7 +861,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { let joint_dst = self.dst.join(file_name); self.shared.fs.write(joint_dst, buf)?; - if !self.render_redirect_pages { + if !self.info.render_redirect_pages { self.shared.all.borrow_mut().append(full_path(self, &item), &item_type); } // If the item is a macro, redirect from the old macro URL (with !) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index dc205252507c..a86b7966c260 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -223,7 +223,7 @@ pub(super) fn print_item(cx: &mut Context<'_>, item: &clean::Item, buf: &mut Buf // this page, and this link will be auto-clicked. The `id` attribute is // used to find the link to auto-click. let src_href = - if cx.include_sources && !item.is_primitive() { cx.src_href(item) } else { None }; + if cx.info.include_sources && !item.is_primitive() { cx.src_href(item) } else { None }; let path_components = if item.is_primitive() || item.is_keyword() { vec![] diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 7c676469597d..1fb589d829e5 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -104,7 +104,7 @@ pub(crate) fn write_shared( &cx.shared.style_files, cx.shared.layout.css_file_extension.as_deref(), &cx.shared.resource_suffix, - cx.include_sources, + cx.info.include_sources, )?; match &opt.index_page { Some(index_page) if opt.enable_index_page => { diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 2fe9364c259c..71b110c943e2 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -124,7 +124,7 @@ struct SourceCollector<'a, 'tcx> { impl DocVisitor<'_> for SourceCollector<'_, '_> { fn visit_item(&mut self, item: &clean::Item) { - if !self.cx.include_sources { + if !self.cx.info.include_sources { return; } @@ -146,7 +146,7 @@ impl DocVisitor<'_> for SourceCollector<'_, '_> { // something like that), so just don't include sources for the // entire crate. The other option is maintaining this mapping on a // per-file basis, but that's probably not worth it... - self.cx.include_sources = match self.emit_source(&filename, file_span) { + self.cx.info.include_sources = match self.emit_source(&filename, file_span) { Ok(()) => true, Err(e) => { self.cx.shared.tcx.dcx().span_err( diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 560ed872ef3a..9e512e87afc4 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -137,6 +137,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { } const RUN_ON_MODULE: bool = false; + type InfoType = (); fn init( krate: clean::Crate, @@ -161,9 +162,8 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { )) } - fn make_child_renderer(&self) -> Self { - self.clone() - } + fn make_child_renderer(&mut self) -> Self::InfoType {} + fn set_back_info(&mut self, _info: Self::InfoType) {} /// Inserts an item into the index. This should be used rather than directly calling insert on /// the hashmap because certain items (traits and types) need to have their mappings for trait