-
Notifications
You must be signed in to change notification settings - Fork 13.4k
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
Closed
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
bea81a3
rustdoc: Move most shared fields to `SharedContext`
camelid 894fdf9
rustdoc: Replace `Arc` around `SharedContext` with `Rc`
camelid 12a886f
rustdoc: Make a bunch of fields private
camelid a0e2fe0
rustdoc: Add static size assertion for `Context`
camelid 9580b56
Add issue for removing shared mutable state
camelid 1701d66
Only run static assert on x86-64
camelid 3823a17
Don't share `id_map` and `deref_id_map`
camelid File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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>>, | ||
/// Shared mutable state. | ||
/// | ||
/// Issue for improving the situation: [#82381][] | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> { | ||
|
@@ -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. | ||
|
@@ -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, | ||
}; | ||
|
@@ -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), | ||
}; | ||
|
@@ -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) | ||
} | ||
|
||
|
@@ -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 { | ||
|
@@ -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>", | ||
|
@@ -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( | ||
¬e, | ||
&mut ids, | ||
|
@@ -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, | ||
|
@@ -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!( | ||
|
@@ -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>", | ||
|
@@ -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"); | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.