Skip to content

Commit c09d9d3

Browse files
committed
Don't unnecessarily clone some fields in Context
There was no need to clone `id_map` because it was reset before each item was rendered. `deref_id_map` was not reset, but it was keyed by `DefId` and thus was unlikely to have collisions (at least for now). Now we just clone the fields that need to be cloned, and instead create fresh versions of the others.
1 parent ff39c46 commit c09d9d3

File tree

5 files changed

+28
-22
lines changed

5 files changed

+28
-22
lines changed

src/librustdoc/formats/renderer.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::formats::cache::Cache;
99
/// Allows for different backends to rustdoc to be used with the `run_format()` function. Each
1010
/// backend renderer has hooks for initialization, documenting an item, entering and exiting a
1111
/// module, and cleanup/finalizing output.
12-
crate trait FormatRenderer<'tcx>: Clone {
12+
crate trait FormatRenderer<'tcx>: Sized {
1313
/// Gives a description of the renderer. Used for performance profiling.
1414
fn descr() -> &'static str;
1515

@@ -23,6 +23,9 @@ crate trait FormatRenderer<'tcx>: Clone {
2323
tcx: TyCtxt<'tcx>,
2424
) -> Result<(Self, clean::Crate), Error>;
2525

26+
/// Make a new renderer to render a child of the item currently being rendered.
27+
fn make_child_renderer(&self) -> Self;
28+
2629
/// Renders a single non-module item. This means no recursive sub-item rendering is required.
2730
fn item(&mut self, item: clean::Item) -> Result<(), Error>;
2831

@@ -67,7 +70,7 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>(
6770
item.name = Some(krate.name);
6871

6972
// Render the crate documentation
70-
let mut work = vec![(format_renderer.clone(), item)];
73+
let mut work = vec![(format_renderer.make_child_renderer(), item)];
7174

7275
let unknown = rustc_span::Symbol::intern("<unknown item>");
7376
while let Some((mut cx, item)) = work.pop() {
@@ -87,7 +90,7 @@ crate fn run_format<'tcx, T: FormatRenderer<'tcx>>(
8790
};
8891
for it in module.items {
8992
debug!("Adding {:?} to worklist", it.name);
90-
work.push((cx.clone(), it));
93+
work.push((cx.make_child_renderer(), it));
9194
}
9295

9396
cx.mod_item_out(&name)?;

src/librustdoc/html/markdown.rs

-4
Original file line numberDiff line numberDiff line change
@@ -1373,10 +1373,6 @@ impl IdMap {
13731373
}
13741374
}
13751375

1376-
crate fn reset(&mut self) {
1377-
self.map = init_id_map();
1378-
}
1379-
13801376
crate fn derive<S: AsRef<str> + ToString>(&mut self, candidate: S) -> String {
13811377
let id = match self.map.get_mut(candidate.as_ref()) {
13821378
None => candidate.to_string(),

src/librustdoc/html/markdown/tests.rs

+3-9
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,9 @@ fn test_unique_id() {
3838
"assoc_type.Item-1",
3939
];
4040

41-
let map = RefCell::new(IdMap::new());
42-
let test = || {
43-
let mut map = map.borrow_mut();
44-
let actual: Vec<String> = input.iter().map(|s| map.derive(s.to_string())).collect();
45-
assert_eq!(&actual[..], expected);
46-
};
47-
test();
48-
map.borrow_mut().reset();
49-
test();
41+
let mut map = IdMap::new();
42+
let actual: Vec<String> = input.iter().map(|s| map.derive(s.to_string())).collect();
43+
assert_eq!(&actual[..], expected);
5044
}
5145

5246
#[test]

src/librustdoc/html/render/context.rs

+15-6
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ use crate::html::{layout, sources};
4040
/// It is intended that this context is a lightweight object which can be fairly
4141
/// easily cloned because it is cloned per work-job (about once per item in the
4242
/// rustdoc tree).
43-
#[derive(Clone)]
4443
crate struct Context<'tcx> {
4544
/// Current hierarchy of components leading down to what's currently being
4645
/// rendered
@@ -157,11 +156,6 @@ impl<'tcx> Context<'tcx> {
157156
static_extra_scripts: &[],
158157
};
159158

160-
{
161-
self.id_map.borrow_mut().reset();
162-
self.id_map.borrow_mut().populate(&INITIAL_IDS);
163-
}
164-
165159
if !self.render_redirect_pages {
166160
layout::render(
167161
&self.shared.layout,
@@ -436,6 +430,21 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
436430
Ok((cx, krate))
437431
}
438432

433+
fn make_child_renderer(&self) -> Self {
434+
let mut id_map = IdMap::new();
435+
id_map.populate(&INITIAL_IDS);
436+
437+
Self {
438+
current: self.current.clone(),
439+
dst: self.dst.clone(),
440+
render_redirect_pages: self.render_redirect_pages,
441+
id_map: Box::new(RefCell::new(id_map)),
442+
deref_id_map: Box::new(RefCell::new(FxHashMap::default())),
443+
shared: Rc::clone(&self.shared),
444+
cache: Rc::clone(&self.cache),
445+
}
446+
}
447+
439448
fn after_krate(
440449
&mut self,
441450
krate: &clean::Crate,

src/librustdoc/json/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> {
149149
))
150150
}
151151

152+
fn make_child_renderer(&self) -> Self {
153+
self.clone()
154+
}
155+
152156
/// Inserts an item into the index. This should be used rather than directly calling insert on
153157
/// the hashmap because certain items (traits and types) need to have their mappings for trait
154158
/// implementations filled out before they're inserted.

0 commit comments

Comments
 (0)