Skip to content

Don't share id_map and deref_id_map #82424

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.

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

Closed
wants to merge 7 commits into from
Closed
Changes from 1 commit
Commits
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
36 changes: 18 additions & 18 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ crate struct Context<'tcx> {
/// real location of an item. This is used to allow external links to
/// publicly reused items to redirect to the right location.
render_redirect_pages: bool,
/// The map used to ensure all generated 'id=' attributes are unique.
id_map: RefCell<IdMap>,
/// Tracks section IDs for `Deref` targets so they match in both the main
/// body and the sidebar.
deref_id_map: RefCell<FxHashMap<DefId, String>>,
Copy link
Member Author

Choose a reason for hiding this comment

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

I wasn't able to get rid of the RefCells here because the fields are used all over the place.

/// Shared mutable state.
///
/// Issue for improving the situation: [#82381][]
Expand All @@ -130,7 +135,7 @@ crate struct Context<'tcx> {

// `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
#[cfg(target_arch = "x86_64")]
rustc_data_structures::static_assert_size!(Context<'_>, 72);
rustc_data_structures::static_assert_size!(Context<'_>, 152);
Copy link
Member Author

Choose a reason for hiding this comment

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

Big size increase! Should I box the fields to reduce the size?


/// Shared mutable state used in [`Context`] and elsewhere.
crate struct SharedContext<'tcx> {
Expand Down Expand Up @@ -172,11 +177,6 @@ crate struct SharedContext<'tcx> {
crate edition: Edition,
codes: ErrorCodes,
playground: Option<markdown::Playground>,
/// The map used to ensure all generated 'id=' attributes are unique.
id_map: RefCell<IdMap>,
/// Tracks section IDs for `Deref` targets so they match in both the main
/// body and the sidebar.
deref_id_map: RefCell<FxHashMap<DefId, String>>,
all: RefCell<AllTypes>,
/// Storage for the errors produced while generating documentation so they
/// can be printed together at the end.
Expand Down Expand Up @@ -496,8 +496,6 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
edition,
codes: ErrorCodes::from(unstable_features.is_nightly_build()),
playground,
id_map: RefCell::new(id_map),
deref_id_map: RefCell::new(FxHashMap::default()),
all: RefCell::new(AllTypes::new()),
errors: receiver,
};
Expand Down Expand Up @@ -526,6 +524,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
current: Vec::new(),
dst,
render_redirect_pages: false,
id_map: RefCell::new(id_map),
deref_id_map: RefCell::new(FxHashMap::default()),
shared: Rc::new(scx),
cache: Rc::new(cache),
};
Expand Down Expand Up @@ -1535,7 +1535,7 @@ fn settings(root_path: &str, suffix: &str, themes: &[StylePath]) -> Result<Strin

impl Context<'_> {
fn derive_id(&self, id: String) -> String {
let mut map = self.shared.id_map.borrow_mut();
let mut map = self.id_map.borrow_mut();
map.derive(id)
}

Expand Down Expand Up @@ -1590,8 +1590,9 @@ impl Context<'_> {
};

{
self.shared.id_map.borrow_mut().reset();
self.shared.id_map.borrow_mut().populate(&INITIAL_IDS);
// FIXME: remove this once `Context` is no longer cloned
self.id_map.borrow_mut().reset();
self.id_map.borrow_mut().populate(&INITIAL_IDS);
}

if !self.render_redirect_pages {
Expand Down Expand Up @@ -1859,7 +1860,7 @@ fn render_markdown(
prefix: &str,
is_hidden: bool,
) {
let mut ids = cx.shared.id_map.borrow_mut();
let mut ids = cx.id_map.borrow_mut();
write!(
w,
"<div class=\"docblock{}\">{}{}</div>",
Expand Down Expand Up @@ -2337,7 +2338,7 @@ fn short_item_info(

if let Some(note) = note {
let note = note.as_str();
let mut ids = cx.shared.id_map.borrow_mut();
let mut ids = cx.id_map.borrow_mut();
let html = MarkdownHtml(
&note,
&mut ids,
Expand Down Expand Up @@ -2376,7 +2377,7 @@ fn short_item_info(
message.push_str(&format!(" ({})", feature));

if let Some(unstable_reason) = reason {
let mut ids = cx.shared.id_map.borrow_mut();
let mut ids = cx.id_map.borrow_mut();
message = format!(
"<details><summary>{}</summary>{}</details>",
message,
Expand Down Expand Up @@ -3531,8 +3532,7 @@ fn render_assoc_items(
type_.print(cx.cache())
)));
debug!("Adding {} to deref id map", type_.print(cx.cache()));
cx.shared
.deref_id_map
cx.deref_id_map
.borrow_mut()
.insert(type_.def_id_full(cx.cache()).unwrap(), id.clone());
write!(
Expand Down Expand Up @@ -3838,7 +3838,7 @@ fn render_impl(
}

if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) {
let mut ids = cx.shared.id_map.borrow_mut();
let mut ids = cx.id_map.borrow_mut();
write!(
w,
"<div class=\"docblock\">{}</div>",
Expand Down Expand Up @@ -4467,7 +4467,7 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &V
.flat_map(|i| get_methods(i.inner_impl(), true, &mut used_links, deref_mut, c))
.collect::<Vec<_>>();
if !ret.is_empty() {
let deref_id_map = cx.shared.deref_id_map.borrow();
let deref_id_map = cx.deref_id_map.borrow();
let id = deref_id_map
.get(&real_target.def_id_full(cx.cache()).unwrap())
.expect("Deref section without derived id");
Expand Down