Skip to content
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

rustdoc perf: clone clean::Item less #130857

Merged
merged 3 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 2 additions & 3 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -837,8 +837,7 @@ pub(crate) fn record_extern_trait(cx: &mut DocContext<'_>, did: DefId) {
}

{
if cx.external_traits.borrow().contains_key(&did) || cx.active_extern_traits.contains(&did)
{
if cx.external_traits.contains_key(&did) || cx.active_extern_traits.contains(&did) {
return;
}
}
Expand All @@ -850,6 +849,6 @@ pub(crate) fn record_extern_trait(cx: &mut DocContext<'_>, did: DefId) {
debug!("record_extern_trait: {did:?}");
let trait_ = build_external_trait(cx, did);

cx.external_traits.borrow_mut().insert(did, trait_);
cx.external_traits.insert(did, trait_);
cx.active_extern_traits.remove(&did);
}
4 changes: 1 addition & 3 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::borrow::Cow;
use std::cell::RefCell;
use std::hash::Hash;
use std::path::PathBuf;
use std::rc::Rc;
use std::sync::{Arc, OnceLock as OnceCell};
use std::{fmt, iter};

Expand Down Expand Up @@ -115,7 +113,7 @@ impl From<DefId> for ItemId {
pub(crate) struct Crate {
pub(crate) module: Item,
/// Only here so that they can be filtered through the rustdoc passes.
pub(crate) external_traits: Rc<RefCell<FxHashMap<DefId, Trait>>>,
pub(crate) external_traits: Box<FxHashMap<DefId, Trait>>,
}

impl Crate {
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ pub(crate) fn krate(cx: &mut DocContext<'_>) -> Crate {
}));
}

Crate { module, external_traits: cx.external_traits.clone() }
Crate { module, external_traits: Box::new(mem::take(&mut cx.external_traits)) }
}

pub(crate) fn clean_middle_generic_args<'tcx>(
Expand Down
6 changes: 2 additions & 4 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::cell::RefCell;
use std::rc::Rc;
use std::sync::atomic::AtomicBool;
use std::sync::{Arc, LazyLock};
use std::{io, mem};
Expand Down Expand Up @@ -41,7 +39,7 @@ pub(crate) struct DocContext<'tcx> {
/// Most of this logic is copied from rustc_lint::late.
pub(crate) param_env: ParamEnv<'tcx>,
/// Later on moved through `clean::Crate` into `cache`
pub(crate) external_traits: Rc<RefCell<FxHashMap<DefId, clean::Trait>>>,
pub(crate) external_traits: FxHashMap<DefId, clean::Trait>,
/// Used while populating `external_traits` to ensure we don't process the same trait twice at
/// the same time.
pub(crate) active_extern_traits: DefIdSet,
Expand Down Expand Up @@ -359,7 +357,7 @@ pub(crate) fn run_global_ctxt(
// Note that in case of `#![no_core]`, the trait is not available.
if let Some(sized_trait_did) = ctxt.tcx.lang_items().sized_trait() {
let sized_trait = build_external_trait(&mut ctxt, sized_trait_did);
ctxt.external_traits.borrow_mut().insert(sized_trait_did, sized_trait);
ctxt.external_traits.insert(sized_trait_did, sized_trait);
}

debug!("crate: {:?}", tcx.hir().krate());
Expand Down
11 changes: 7 additions & 4 deletions src/librustdoc/fold.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::mem;

use crate::clean::*;

pub(crate) fn strip_item(mut item: Item) -> Item {
Expand Down Expand Up @@ -116,10 +118,11 @@ pub(crate) trait DocFolder: Sized {
fn fold_crate(&mut self, mut c: Crate) -> Crate {
c.module = self.fold_item(c.module).unwrap();

let external_traits = { std::mem::take(&mut *c.external_traits.borrow_mut()) };
for (k, mut v) in external_traits {
v.items = v.items.into_iter().filter_map(|i| self.fold_item(i)).collect();
c.external_traits.borrow_mut().insert(k, v);
for trait_ in c.external_traits.values_mut() {
trait_.items = mem::take(&mut trait_.items)
.into_iter()
.filter_map(|i| self.fold_item(i))
.collect();
}

c
Expand Down
3 changes: 2 additions & 1 deletion src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ impl Cache {

// Crawl the crate to build various caches used for the output
debug!(?cx.cache.crate_version);
cx.cache.traits = krate.external_traits.take();
assert!(cx.external_traits.is_empty());
cx.cache.traits = mem::take(&mut krate.external_traits);

// Cache where all our extern crates are located
// FIXME: this part is specific to HTML so it'd be nice to remove it from the common code
Expand Down
18 changes: 9 additions & 9 deletions src/librustdoc/html/render/write_shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -824,9 +824,9 @@ impl Serialize for Implementor {
/// this visitor works to reverse that: `aliased_types` is a map
/// from target to the aliases that reference it, and each one
/// will generate one file.
struct TypeImplCollector<'cx, 'cache> {
struct TypeImplCollector<'cx, 'cache, 'item> {
/// Map from DefId-of-aliased-type to its data.
aliased_types: IndexMap<DefId, AliasedType<'cache>>,
aliased_types: IndexMap<DefId, AliasedType<'cache, 'item>>,
visited_aliases: FxHashSet<DefId>,
cache: &'cache Cache,
cx: &'cache mut Context<'cx>,
Expand All @@ -847,26 +847,26 @@ struct TypeImplCollector<'cx, 'cache> {
/// ]
/// )
/// ```
struct AliasedType<'cache> {
struct AliasedType<'cache, 'item> {
/// This is used to generate the actual filename of this aliased type.
target_fqp: &'cache [Symbol],
target_type: ItemType,
/// This is the data stored inside the file.
/// ItemId is used to deduplicate impls.
impl_: IndexMap<ItemId, AliasedTypeImpl<'cache>>,
impl_: IndexMap<ItemId, AliasedTypeImpl<'cache, 'item>>,
}

/// The `impl_` contains data that's used to figure out if an alias will work,
/// and to generate the HTML at the end.
///
/// The `type_aliases` list is built up with each type alias that matches.
struct AliasedTypeImpl<'cache> {
struct AliasedTypeImpl<'cache, 'item> {
impl_: &'cache Impl,
type_aliases: Vec<(&'cache [Symbol], Item)>,
type_aliases: Vec<(&'cache [Symbol], &'item Item)>,
}

impl<'cx, 'cache> DocVisitor for TypeImplCollector<'cx, 'cache> {
fn visit_item(&mut self, it: &Item) {
impl<'cx, 'cache, 'item> DocVisitor<'item> for TypeImplCollector<'cx, 'cache, 'item> {
fn visit_item(&mut self, it: &'item Item) {
self.visit_item_recur(it);
let cache = self.cache;
let ItemKind::TypeAliasItem(ref t) = it.kind else { return };
Expand Down Expand Up @@ -927,7 +927,7 @@ impl<'cx, 'cache> DocVisitor for TypeImplCollector<'cx, 'cache> {
continue;
}
// This impl was not found in the set of rejected impls
aliased_type_impl.type_aliases.push((&self_fqp[..], it.clone()));
aliased_type_impl.type_aliases.push((&self_fqp[..], it));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/html/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ impl LocalSourcesCollector<'_, '_> {
}
}

impl DocVisitor for LocalSourcesCollector<'_, '_> {
impl DocVisitor<'_> for LocalSourcesCollector<'_, '_> {
fn visit_item(&mut self, item: &clean::Item) {
self.add_local_source(item);

Expand All @@ -122,7 +122,7 @@ struct SourceCollector<'a, 'tcx> {
crate_name: &'a str,
}

impl DocVisitor for SourceCollector<'_, '_> {
impl DocVisitor<'_> for SourceCollector<'_, '_> {
fn visit_item(&mut self, item: &clean::Item) {
if !self.cx.include_sources {
return;
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/calculate_doc_coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ impl<'a, 'b> CoverageCalculator<'a, 'b> {
}
}

impl<'a, 'b> DocVisitor for CoverageCalculator<'a, 'b> {
impl<'a, 'b> DocVisitor<'_> for CoverageCalculator<'a, 'b> {
fn visit_item(&mut self, i: &clean::Item) {
if !i.item_id.is_local() {
// non-local items are skipped because they can be out of the users control,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/check_doc_test_visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub(crate) fn check_doc_test_visibility(krate: Crate, cx: &mut DocContext<'_>) -
krate
}

impl<'a, 'tcx> DocVisitor for DocTestVisibilityLinter<'a, 'tcx> {
impl<'a, 'tcx> DocVisitor<'_> for DocTestVisibilityLinter<'a, 'tcx> {
fn visit_item(&mut self, item: &Item) {
look_for_tests(self.cx, &item.doc_value(), item);

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ fn is_derive_trait_collision<T>(ns: &PerNS<Result<Vec<(Res, T)>, ResolutionFailu
}
}

impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> {
impl<'a, 'tcx> DocVisitor<'_> for LinkCollector<'a, 'tcx> {
fn visit_item(&mut self, item: &Item) {
self.resolve_links(item);
self.visit_item_recur(item)
Expand Down
6 changes: 4 additions & 2 deletions src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
panic!("collect-trait-impls can't run");
};

krate.external_traits.extend(cx.external_traits.drain());

krate
}

Expand All @@ -227,7 +229,7 @@ struct SyntheticImplCollector<'a, 'tcx> {
impls: Vec<Item>,
}

impl<'a, 'tcx> DocVisitor for SyntheticImplCollector<'a, 'tcx> {
impl<'a, 'tcx> DocVisitor<'_> for SyntheticImplCollector<'a, 'tcx> {
fn visit_item(&mut self, i: &Item) {
if i.is_struct() || i.is_enum() || i.is_union() {
// FIXME(eddyb) is this `doc(hidden)` check needed?
Expand All @@ -254,7 +256,7 @@ impl<'cache> ItemAndAliasCollector<'cache> {
}
}

impl<'cache> DocVisitor for ItemAndAliasCollector<'cache> {
impl<'cache> DocVisitor<'_> for ItemAndAliasCollector<'cache> {
fn visit_item(&mut self, i: &Item) {
self.items.insert(i.item_id);

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(crate) fn run_lints(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
krate
}

impl<'a, 'tcx> DocVisitor for Linter<'a, 'tcx> {
impl<'a, 'tcx> DocVisitor<'_> for Linter<'a, 'tcx> {
fn visit_item(&mut self, item: &Item) {
let Some(hir_id) = DocContext::as_local_hir_id(self.cx.tcx, item.item_id) else {
// If non-local, no need to check anything.
Expand Down
19 changes: 8 additions & 11 deletions src/librustdoc/visit.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::clean::*;

pub(crate) trait DocVisitor: Sized {
fn visit_item(&mut self, item: &Item) {
pub(crate) trait DocVisitor<'a>: Sized {
fn visit_item(&mut self, item: &'a Item) {
self.visit_item_recur(item)
}

/// don't override!
fn visit_inner_recur(&mut self, kind: &ItemKind) {
fn visit_inner_recur(&mut self, kind: &'a ItemKind) {
match kind {
StrippedItem(..) => unreachable!(),
ModuleItem(i) => {
Expand Down Expand Up @@ -47,25 +47,22 @@ pub(crate) trait DocVisitor: Sized {
}

/// don't override!
fn visit_item_recur(&mut self, item: &Item) {
fn visit_item_recur(&mut self, item: &'a Item) {
match &item.kind {
StrippedItem(i) => self.visit_inner_recur(&*i),
_ => self.visit_inner_recur(&item.kind),
}
}

fn visit_mod(&mut self, m: &Module) {
fn visit_mod(&mut self, m: &'a Module) {
m.items.iter().for_each(|i| self.visit_item(i))
}

fn visit_crate(&mut self, c: &Crate) {
fn visit_crate(&mut self, c: &'a Crate) {
self.visit_item(&c.module);

// FIXME: make this a simple by-ref for loop once external_traits is cleaned up
let external_traits = { std::mem::take(&mut *c.external_traits.borrow_mut()) };
for (k, v) in external_traits {
v.items.iter().for_each(|i| self.visit_item(i));
c.external_traits.borrow_mut().insert(k, v);
for trait_ in c.external_traits.values() {
trait_.items.iter().for_each(|i| self.visit_item(i));
}
}
}
Loading