Skip to content

Commit 0d6f0cd

Browse files
committed
Auto merge of rust-lang#133345 - GuillaumeGomez:stop-cloning-context, r=<try>
Stop cloning `Context` so much This is a first step for rust-lang#82381. It's already big enough so I'll continue in a follow-up once this PR is merged. Next step will be to get rid of `SharedContext` by inlining it directly into `Context`. cc `@camelid` r? `@notriddle`
2 parents 6e1c115 + 74d6adf commit 0d6f0cd

File tree

7 files changed

+96
-81
lines changed

7 files changed

+96
-81
lines changed

src/librustdoc/formats/renderer.rs

+40-32
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1+
use rustc_data_structures::profiling::SelfProfilerRef;
12
use rustc_middle::ty::TyCtxt;
2-
use tracing::debug;
33

44
use crate::clean;
55
use crate::config::RenderOptions;
@@ -17,6 +17,7 @@ pub(crate) trait FormatRenderer<'tcx>: Sized {
1717
///
1818
/// This is true for html, and false for json. See #80664
1919
const RUN_ON_MODULE: bool;
20+
type InfoType;
2021

2122
/// Sets up any state required for the renderer. When this is called the cache has already been
2223
/// populated.
@@ -28,7 +29,8 @@ pub(crate) trait FormatRenderer<'tcx>: Sized {
2829
) -> Result<(Self, clean::Crate), Error>;
2930

3031
/// Make a new renderer to render a child of the item currently being rendered.
31-
fn make_child_renderer(&self) -> Self;
32+
fn make_child_renderer(&mut self) -> Self::InfoType;
33+
fn set_back_info(&mut self, _info: Self::InfoType);
3234

3335
/// Renders a single non-module item. This means no recursive sub-item rendering is required.
3436
fn item(&mut self, item: clean::Item) -> Result<(), Error>;
@@ -47,6 +49,40 @@ pub(crate) trait FormatRenderer<'tcx>: Sized {
4749
fn cache(&self) -> &Cache;
4850
}
4951

52+
fn run_format_inner<'tcx, T: FormatRenderer<'tcx>>(
53+
cx: &mut T,
54+
item: clean::Item,
55+
prof: &SelfProfilerRef,
56+
) -> Result<(), Error> {
57+
if item.is_mod() && T::RUN_ON_MODULE {
58+
// modules are special because they add a namespace. We also need to
59+
// recurse into the items of the module as well.
60+
let _timer =
61+
prof.generic_activity_with_arg("render_mod_item", item.name.unwrap().to_string());
62+
63+
cx.mod_item_in(&item)?;
64+
let (clean::StrippedItem(box clean::ModuleItem(module)) | clean::ModuleItem(module)) =
65+
item.inner.kind
66+
else {
67+
unreachable!()
68+
};
69+
for it in module.items {
70+
let info = cx.make_child_renderer();
71+
run_format_inner(cx, it, prof)?;
72+
cx.set_back_info(info);
73+
}
74+
75+
cx.mod_item_out()?;
76+
// FIXME: checking `item.name.is_some()` is very implicit and leads to lots of special
77+
// cases. Use an explicit match instead.
78+
} else if let Some(item_name) = item.name
79+
&& !item.is_extern_crate()
80+
{
81+
prof.generic_activity_with_arg("render_item", item_name.as_str()).run(|| cx.item(item))?;
82+
}
83+
Ok(())
84+
}
85+
5086
/// Main method for rendering a crate.
5187
pub(crate) fn run_format<'tcx, T: FormatRenderer<'tcx>>(
5288
krate: clean::Crate,
@@ -66,36 +102,8 @@ pub(crate) fn run_format<'tcx, T: FormatRenderer<'tcx>>(
66102
}
67103

68104
// Render the crate documentation
69-
let mut work = vec![(format_renderer.make_child_renderer(), krate.module)];
70-
71-
while let Some((mut cx, item)) = work.pop() {
72-
if item.is_mod() && T::RUN_ON_MODULE {
73-
// modules are special because they add a namespace. We also need to
74-
// recurse into the items of the module as well.
75-
let _timer =
76-
prof.generic_activity_with_arg("render_mod_item", item.name.unwrap().to_string());
77-
78-
cx.mod_item_in(&item)?;
79-
let (clean::StrippedItem(box clean::ModuleItem(module)) | clean::ModuleItem(module)) =
80-
item.inner.kind
81-
else {
82-
unreachable!()
83-
};
84-
for it in module.items {
85-
debug!("Adding {:?} to worklist", it.name);
86-
work.push((cx.make_child_renderer(), it));
87-
}
88-
89-
cx.mod_item_out()?;
90-
// FIXME: checking `item.name.is_some()` is very implicit and leads to lots of special
91-
// cases. Use an explicit match instead.
92-
} else if let Some(item_name) = item.name
93-
&& !item.is_extern_crate()
94-
{
95-
prof.generic_activity_with_arg("render_item", item_name.as_str())
96-
.run(|| cx.item(item))?;
97-
}
98-
}
105+
run_format_inner(&mut format_renderer, krate.module, prof)?;
106+
99107
prof.verbose_generic_activity_with_arg("renderer_after_krate", T::descr())
100108
.run(|| format_renderer.after_krate())
101109
}

src/librustdoc/html/markdown.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ use std::iter::Peekable;
3232
use std::ops::{ControlFlow, Range};
3333
use std::path::PathBuf;
3434
use std::str::{self, CharIndices};
35-
use std::sync::OnceLock;
3635

3736
use pulldown_cmark::{
3837
BrokenLink, CodeBlockKind, CowStr, Event, LinkType, Options, Parser, Tag, TagEnd, html,
@@ -1886,12 +1885,10 @@ pub(crate) fn rust_code_blocks(md: &str, extra_info: &ExtraInfo<'_>) -> Vec<Rust
18861885
#[derive(Clone, Default, Debug)]
18871886
pub struct IdMap {
18881887
map: FxHashMap<Cow<'static, str>, usize>,
1888+
defined_ids: FxHashMap<Cow<'static, str>, usize>,
18891889
existing_footnotes: usize,
18901890
}
18911891

1892-
// The map is pre-initialized and cloned each time to avoid reinitializing it repeatedly.
1893-
static DEFAULT_ID_MAP: OnceLock<FxHashMap<Cow<'static, str>, usize>> = OnceLock::new();
1894-
18951892
fn init_id_map() -> FxHashMap<Cow<'static, str>, usize> {
18961893
let mut map = FxHashMap::default();
18971894
// This is the list of IDs used in JavaScript.
@@ -1948,7 +1945,7 @@ fn init_id_map() -> FxHashMap<Cow<'static, str>, usize> {
19481945

19491946
impl IdMap {
19501947
pub fn new() -> Self {
1951-
IdMap { map: DEFAULT_ID_MAP.get_or_init(init_id_map).clone(), existing_footnotes: 0 }
1948+
IdMap { map: FxHashMap::default(), defined_ids: init_id_map(), existing_footnotes: 0 }
19521949
}
19531950

19541951
pub(crate) fn derive<S: AsRef<str> + ToString>(&mut self, candidate: S) -> String {
@@ -1973,4 +1970,9 @@ impl IdMap {
19731970
closure(self, &mut existing_footnotes);
19741971
self.existing_footnotes = existing_footnotes;
19751972
}
1973+
1974+
pub(crate) fn clear(&mut self) {
1975+
self.map = self.defined_ids.clone();
1976+
self.existing_footnotes = 0;
1977+
}
19761978
}

src/librustdoc/html/render/context.rs

+42-37
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,6 @@ pub(crate) struct Context<'tcx> {
4949
/// The current destination folder of where HTML artifacts should be placed.
5050
/// This changes as the context descends into the module hierarchy.
5151
pub(crate) dst: PathBuf,
52-
/// A flag, which when `true`, will render pages which redirect to the
53-
/// real location of an item. This is used to allow external links to
54-
/// publicly reused items to redirect to the right location.
55-
pub(super) render_redirect_pages: bool,
5652
/// Tracks section IDs for `Deref` targets so they match in both the main
5753
/// body and the sidebar.
5854
pub(super) deref_id_map: DefIdMap<String>,
@@ -64,21 +60,32 @@ pub(crate) struct Context<'tcx> {
6460
///
6561
/// [#82381]: https://github.com/rust-lang/rust/issues/82381
6662
pub(crate) shared: Rc<SharedContext<'tcx>>,
63+
/// Collection of all types with notable traits referenced in the current module.
64+
pub(crate) types_with_notable_traits: FxIndexSet<clean::Type>,
65+
/// Contains information that needs to be saved and reset after rendering an item which is
66+
/// not a module.
67+
pub(crate) info: ContextInfo,
68+
}
69+
70+
#[derive(Clone, Copy)]
71+
pub(crate) struct ContextInfo {
72+
/// A flag, which when `true`, will render pages which redirect to the
73+
/// real location of an item. This is used to allow external links to
74+
/// publicly reused items to redirect to the right location.
75+
pub(super) render_redirect_pages: bool,
6776
/// This flag indicates whether source links should be generated or not. If
6877
/// the source files are present in the html rendering, then this will be
6978
/// `true`.
7079
pub(crate) include_sources: bool,
71-
/// Collection of all types with notable traits referenced in the current module.
72-
pub(crate) types_with_notable_traits: FxIndexSet<clean::Type>,
7380
/// Field used during rendering, to know if we're inside an inlined item.
7481
pub(crate) is_inside_inlined_module: bool,
7582
}
7683

77-
// `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
78-
#[cfg(all(not(windows), target_pointer_width = "64"))]
79-
rustc_data_structures::static_assert_size!(Context<'_>, 192);
80-
#[cfg(all(windows, target_pointer_width = "64"))]
81-
rustc_data_structures::static_assert_size!(Context<'_>, 200);
84+
impl ContextInfo {
85+
fn new(include_sources: bool) -> Self {
86+
Self { render_redirect_pages: false, include_sources, is_inside_inlined_module: false }
87+
}
88+
}
8289

8390
/// Shared mutable state used in [`Context`] and elsewhere.
8491
pub(crate) struct SharedContext<'tcx> {
@@ -174,14 +181,16 @@ impl<'tcx> Context<'tcx> {
174181
}
175182

176183
fn render_item(&mut self, it: &clean::Item, is_module: bool) -> String {
177-
let mut render_redirect_pages = self.render_redirect_pages;
184+
let mut render_redirect_pages = self.info.render_redirect_pages;
178185
// If the item is stripped but inlined, links won't point to the item so no need to generate
179186
// a file for it.
180187
if it.is_stripped()
181188
&& let Some(def_id) = it.def_id()
182189
&& def_id.is_local()
183190
{
184-
if self.is_inside_inlined_module || self.shared.cache.inlined_items.contains(&def_id) {
191+
if self.info.is_inside_inlined_module
192+
|| self.shared.cache.inlined_items.contains(&def_id)
193+
{
185194
// For now we're forced to generate a redirect page for stripped items until
186195
// `record_extern_fqn` correctly points to external items.
187196
render_redirect_pages = true;
@@ -441,6 +450,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
441450
}
442451

443452
const RUN_ON_MODULE: bool = true;
453+
type InfoType = ContextInfo;
444454

445455
fn init(
446456
krate: clean::Crate,
@@ -562,13 +572,11 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
562572
let mut cx = Context {
563573
current: Vec::new(),
564574
dst,
565-
render_redirect_pages: false,
566575
id_map,
567576
deref_id_map: Default::default(),
568577
shared: Rc::new(scx),
569-
include_sources,
570578
types_with_notable_traits: FxIndexSet::default(),
571-
is_inside_inlined_module: false,
579+
info: ContextInfo::new(include_sources),
572580
};
573581

574582
if emit_crate {
@@ -582,18 +590,15 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
582590
Ok((cx, krate))
583591
}
584592

585-
fn make_child_renderer(&self) -> Self {
586-
Self {
587-
current: self.current.clone(),
588-
dst: self.dst.clone(),
589-
render_redirect_pages: self.render_redirect_pages,
590-
deref_id_map: Default::default(),
591-
id_map: IdMap::new(),
592-
shared: Rc::clone(&self.shared),
593-
include_sources: self.include_sources,
594-
types_with_notable_traits: FxIndexSet::default(),
595-
is_inside_inlined_module: self.is_inside_inlined_module,
596-
}
593+
fn make_child_renderer(&mut self) -> Self::InfoType {
594+
self.deref_id_map.clear();
595+
self.id_map.clear();
596+
self.types_with_notable_traits.clear();
597+
self.info
598+
}
599+
600+
fn set_back_info(&mut self, info: Self::InfoType) {
601+
self.info = info;
597602
}
598603

599604
fn after_krate(&mut self) -> Result<(), Error> {
@@ -775,8 +780,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
775780
// External crates will provide links to these structures, so
776781
// these modules are recursed into, but not rendered normally
777782
// (a flag on the context).
778-
if !self.render_redirect_pages {
779-
self.render_redirect_pages = item.is_stripped();
783+
if !self.info.render_redirect_pages {
784+
self.info.render_redirect_pages = item.is_stripped();
780785
}
781786
let item_name = item.name.unwrap();
782787
self.dst.push(&*item_name.as_str());
@@ -793,19 +798,19 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
793798
self.shared.fs.write(joint_dst, buf)?;
794799
}
795800
}
796-
if !self.is_inside_inlined_module {
801+
if !self.info.is_inside_inlined_module {
797802
if let Some(def_id) = item.def_id()
798803
&& self.cache().inlined_items.contains(&def_id)
799804
{
800-
self.is_inside_inlined_module = true;
805+
self.info.is_inside_inlined_module = true;
801806
}
802807
} else if !self.cache().document_hidden && item.is_doc_hidden() {
803808
// We're not inside an inlined module anymore since this one cannot be re-exported.
804-
self.is_inside_inlined_module = false;
809+
self.info.is_inside_inlined_module = false;
805810
}
806811

807812
// Render sidebar-items.js used throughout this module.
808-
if !self.render_redirect_pages {
813+
if !self.info.render_redirect_pages {
809814
let (clean::StrippedItem(box clean::ModuleItem(ref module))
810815
| clean::ModuleItem(ref module)) = item.kind
811816
else {
@@ -836,8 +841,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
836841
// External crates will provide links to these structures, so
837842
// these modules are recursed into, but not rendered normally
838843
// (a flag on the context).
839-
if !self.render_redirect_pages {
840-
self.render_redirect_pages = item.is_stripped();
844+
if !self.info.render_redirect_pages {
845+
self.info.render_redirect_pages = item.is_stripped();
841846
}
842847

843848
let buf = self.render_item(&item, false);
@@ -850,7 +855,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
850855
let joint_dst = self.dst.join(file_name);
851856
self.shared.fs.write(joint_dst, buf)?;
852857

853-
if !self.render_redirect_pages {
858+
if !self.info.render_redirect_pages {
854859
self.shared.all.borrow_mut().append(full_path(self, &item), &item_type);
855860
}
856861
// If the item is a macro, redirect from the old macro URL (with !)

src/librustdoc/html/render/print_item.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ pub(super) fn print_item(cx: &mut Context<'_>, item: &clean::Item, buf: &mut Buf
223223
// this page, and this link will be auto-clicked. The `id` attribute is
224224
// used to find the link to auto-click.
225225
let src_href =
226-
if cx.include_sources && !item.is_primitive() { cx.src_href(item) } else { None };
226+
if cx.info.include_sources && !item.is_primitive() { cx.src_href(item) } else { None };
227227

228228
let path_components = if item.is_primitive() || item.is_keyword() {
229229
vec![]

src/librustdoc/html/render/write_shared.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ pub(crate) fn write_shared(
104104
&cx.shared.style_files,
105105
cx.shared.layout.css_file_extension.as_deref(),
106106
&cx.shared.resource_suffix,
107-
cx.include_sources,
107+
cx.info.include_sources,
108108
)?;
109109
match &opt.index_page {
110110
Some(index_page) if opt.enable_index_page => {

src/librustdoc/html/sources.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ struct SourceCollector<'a, 'tcx> {
124124

125125
impl DocVisitor<'_> for SourceCollector<'_, '_> {
126126
fn visit_item(&mut self, item: &clean::Item) {
127-
if !self.cx.include_sources {
127+
if !self.cx.info.include_sources {
128128
return;
129129
}
130130

@@ -146,7 +146,7 @@ impl DocVisitor<'_> for SourceCollector<'_, '_> {
146146
// something like that), so just don't include sources for the
147147
// entire crate. The other option is maintaining this mapping on a
148148
// per-file basis, but that's probably not worth it...
149-
self.cx.include_sources = match self.emit_source(&filename, file_span) {
149+
self.cx.info.include_sources = match self.emit_source(&filename, file_span) {
150150
Ok(()) => true,
151151
Err(e) => {
152152
self.cx.shared.tcx.dcx().span_err(

src/librustdoc/json/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
137137
}
138138

139139
const RUN_ON_MODULE: bool = false;
140+
type InfoType = ();
140141

141142
fn init(
142143
krate: clean::Crate,
@@ -161,9 +162,8 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
161162
))
162163
}
163164

164-
fn make_child_renderer(&self) -> Self {
165-
self.clone()
166-
}
165+
fn make_child_renderer(&mut self) -> Self::InfoType {}
166+
fn set_back_info(&mut self, _info: Self::InfoType) {}
167167

168168
/// Inserts an item into the index. This should be used rather than directly calling insert on
169169
/// the hashmap because certain items (traits and types) need to have their mappings for trait

0 commit comments

Comments
 (0)