1
Fork 0

rustdoc: reduce allocations when generating tooltips

An attempt to reduce the perf regression in
https://github.com/rust-lang/rust/pull/108052#issuecomment-1430631861
This commit is contained in:
Michael Howell 2023-02-15 14:44:31 -07:00
parent 0978711950
commit 49d995a4cf
7 changed files with 40 additions and 29 deletions

View file

@ -265,9 +265,9 @@ fn strip_generics_from_path_segment(segment: Vec<char>) -> Result<String, Malfor
} }
} }
pub fn strip_generics_from_path(path_str: &str) -> Result<String, MalformedGenerics> { pub fn strip_generics_from_path(path_str: &str) -> Result<Box<str>, MalformedGenerics> {
if !path_str.contains(['<', '>']) { if !path_str.contains(['<', '>']) {
return Ok(path_str.to_string()); return Ok(path_str.into());
} }
let mut stripped_segments = vec![]; let mut stripped_segments = vec![];
let mut path = path_str.chars().peekable(); let mut path = path_str.chars().peekable();
@ -322,7 +322,11 @@ pub fn strip_generics_from_path(path_str: &str) -> Result<String, MalformedGener
let stripped_path = stripped_segments.join("::"); let stripped_path = stripped_segments.join("::");
if !stripped_path.is_empty() { Ok(stripped_path) } else { Err(MalformedGenerics::MissingType) } if !stripped_path.is_empty() {
Ok(stripped_path.into())
} else {
Err(MalformedGenerics::MissingType)
}
} }
/// Returns whether the first doc-comment is an inner attribute. /// Returns whether the first doc-comment is an inner attribute.
@ -336,7 +340,7 @@ pub fn inner_docs(attrs: &[ast::Attribute]) -> bool {
/// Simplified version of the corresponding function in rustdoc. /// Simplified version of the corresponding function in rustdoc.
/// If the rustdoc version returns a successful result, this function must return the same result. /// If the rustdoc version returns a successful result, this function must return the same result.
/// Otherwise this function may return anything. /// Otherwise this function may return anything.
fn preprocess_link(link: &str) -> String { fn preprocess_link(link: &str) -> Box<str> {
let link = link.replace('`', ""); let link = link.replace('`', "");
let link = link.split('#').next().unwrap(); let link = link.split('#').next().unwrap();
let link = link.trim(); let link = link.trim();
@ -345,7 +349,7 @@ fn preprocess_link(link: &str) -> String {
let link = link.strip_suffix("{}").unwrap_or(link); let link = link.strip_suffix("{}").unwrap_or(link);
let link = link.strip_suffix("[]").unwrap_or(link); let link = link.strip_suffix("[]").unwrap_or(link);
let link = if link != "!" { link.strip_suffix('!').unwrap_or(link) } else { link }; let link = if link != "!" { link.strip_suffix('!').unwrap_or(link) } else { link };
strip_generics_from_path(link).unwrap_or_else(|_| link.to_string()) strip_generics_from_path(link).unwrap_or_else(|_| link.into())
} }
/// Keep inline and reference links `[]`, /// Keep inline and reference links `[]`,
@ -365,7 +369,7 @@ pub fn may_be_doc_link(link_type: LinkType) -> bool {
/// Simplified version of `preprocessed_markdown_links` from rustdoc. /// Simplified version of `preprocessed_markdown_links` from rustdoc.
/// Must return at least the same links as it, but may add some more links on top of that. /// Must return at least the same links as it, but may add some more links on top of that.
pub(crate) fn attrs_to_preprocessed_links(attrs: &[ast::Attribute]) -> Vec<String> { pub(crate) fn attrs_to_preprocessed_links(attrs: &[ast::Attribute]) -> Vec<Box<str>> {
let (doc_fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true); let (doc_fragments, _) = attrs_to_doc_fragments(attrs.iter().map(|attr| (attr, None)), true);
let doc = prepare_to_doc_link_resolution(&doc_fragments).into_values().next().unwrap(); let doc = prepare_to_doc_link_resolution(&doc_fragments).into_values().next().unwrap();

View file

@ -1017,12 +1017,12 @@ pub(crate) fn collapse_doc_fragments(doc_strings: &[DocFragment]) -> String {
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct ItemLink { pub(crate) struct ItemLink {
/// The original link written in the markdown /// The original link written in the markdown
pub(crate) link: String, pub(crate) link: Box<str>,
/// The link text displayed in the HTML. /// The link text displayed in the HTML.
/// ///
/// This may not be the same as `link` if there was a disambiguator /// This may not be the same as `link` if there was a disambiguator
/// in an intra-doc link (e.g. \[`fn@f`\]) /// in an intra-doc link (e.g. \[`fn@f`\])
pub(crate) link_text: String, pub(crate) link_text: Box<str>,
/// The `DefId` of the Item whose **HTML Page** contains the item being /// The `DefId` of the Item whose **HTML Page** contains the item being
/// linked to. This will be different to `item_id` on item's that don't /// linked to. This will be different to `item_id` on item's that don't
/// have their own page, such as struct fields and enum variants. /// have their own page, such as struct fields and enum variants.
@ -1035,9 +1035,9 @@ pub struct RenderedLink {
/// The text the link was original written as. /// The text the link was original written as.
/// ///
/// This could potentially include disambiguators and backticks. /// This could potentially include disambiguators and backticks.
pub(crate) original_text: String, pub(crate) original_text: Box<str>,
/// The text to display in the HTML /// The text to display in the HTML
pub(crate) new_text: String, pub(crate) new_text: Box<str>,
/// The URL to put in the `href` /// The URL to put in the `href`
pub(crate) href: String, pub(crate) href: String,
/// The tooltip. /// The tooltip.

View file

@ -772,14 +772,21 @@ pub(crate) fn link_tooltip(did: DefId, fragment: &Option<UrlFragment>, cx: &Cont
let Some((fqp, shortty)) = cache.paths.get(&did) let Some((fqp, shortty)) = cache.paths.get(&did)
.or_else(|| cache.external_paths.get(&did)) .or_else(|| cache.external_paths.get(&did))
else { return String::new() }; else { return String::new() };
let fqp = fqp.iter().map(|sym| sym.as_str()).join("::"); let mut buf = Buffer::new();
if let &Some(UrlFragment::Item(id)) = fragment { if let &Some(UrlFragment::Item(id)) = fragment {
let name = cx.tcx().item_name(id); write!(buf, "{} ", cx.tcx().def_descr(id));
let descr = cx.tcx().def_descr(id); for component in fqp {
format!("{descr} {fqp}::{name}") write!(buf, "{component}::");
} else { }
format!("{shortty} {fqp}") write!(buf, "{}", cx.tcx().item_name(id));
} else if !fqp.is_empty() {
let mut fqp_it = fqp.into_iter();
write!(buf, "{shortty} {}", fqp_it.next().unwrap());
for component in fqp_it {
write!(buf, "::{component}");
}
} }
buf.into_inner()
} }
/// Used to render a [`clean::Path`]. /// Used to render a [`clean::Path`].

View file

@ -981,7 +981,7 @@ impl Markdown<'_> {
let mut replacer = |broken_link: BrokenLink<'_>| { let mut replacer = |broken_link: BrokenLink<'_>| {
links links
.iter() .iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference) .find(|link| &*link.original_text == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into())) .map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
}; };
@ -1064,7 +1064,7 @@ impl MarkdownSummaryLine<'_> {
let mut replacer = |broken_link: BrokenLink<'_>| { let mut replacer = |broken_link: BrokenLink<'_>| {
links links
.iter() .iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference) .find(|link| &*link.original_text == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into())) .map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
}; };
@ -1111,7 +1111,7 @@ fn markdown_summary_with_limit(
let mut replacer = |broken_link: BrokenLink<'_>| { let mut replacer = |broken_link: BrokenLink<'_>| {
link_names link_names
.iter() .iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference) .find(|link| &*link.original_text == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into())) .map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
}; };
@ -1192,7 +1192,7 @@ pub(crate) fn plain_text_summary(md: &str, link_names: &[RenderedLink]) -> Strin
let mut replacer = |broken_link: BrokenLink<'_>| { let mut replacer = |broken_link: BrokenLink<'_>| {
link_names link_names
.iter() .iter()
.find(|link| link.original_text.as_str() == &*broken_link.reference) .find(|link| &*link.original_text == &*broken_link.reference)
.map(|link| (link.href.as_str().into(), link.tooltip.as_str().into())) .map(|link| (link.href.as_str().into(), link.tooltip.as_str().into()))
}; };

View file

@ -38,7 +38,7 @@ impl JsonRenderer<'_> {
Some(UrlFragment::UserWritten(_)) | None => *page_id, Some(UrlFragment::UserWritten(_)) | None => *page_id,
}; };
(link.clone(), id_from_item_default(id.into(), self.tcx)) (String::from(&**link), id_from_item_default(id.into(), self.tcx))
}) })
.collect(); .collect();
let docs = item.attrs.collapsed_doc_value(); let docs = item.attrs.collapsed_doc_value();

View file

@ -228,7 +228,7 @@ struct ResolutionInfo {
item_id: ItemId, item_id: ItemId,
module_id: DefId, module_id: DefId,
dis: Option<Disambiguator>, dis: Option<Disambiguator>,
path_str: String, path_str: Box<str>,
extra_fragment: Option<String>, extra_fragment: Option<String>,
} }
@ -849,10 +849,10 @@ impl PreprocessingError {
#[derive(Clone)] #[derive(Clone)]
struct PreprocessingInfo { struct PreprocessingInfo {
path_str: String, path_str: Box<str>,
disambiguator: Option<Disambiguator>, disambiguator: Option<Disambiguator>,
extra_fragment: Option<String>, extra_fragment: Option<String>,
link_text: String, link_text: Box<str>,
} }
// Not a typedef to avoid leaking several private structures from this module. // Not a typedef to avoid leaking several private structures from this module.
@ -937,7 +937,7 @@ fn preprocess_link(
path_str, path_str,
disambiguator, disambiguator,
extra_fragment: extra_fragment.map(|frag| frag.to_owned()), extra_fragment: extra_fragment.map(|frag| frag.to_owned()),
link_text: link_text.to_owned(), link_text: Box::<str>::from(link_text),
})) }))
} }
@ -993,7 +993,7 @@ impl LinkCollector<'_, '_> {
item_id: item.item_id, item_id: item.item_id,
module_id, module_id,
dis: disambiguator, dis: disambiguator,
path_str: path_str.to_owned(), path_str: path_str.clone(),
extra_fragment: extra_fragment.clone(), extra_fragment: extra_fragment.clone(),
}, },
diag_info.clone(), // this struct should really be Copy, but Range is not :( diag_info.clone(), // this struct should really be Copy, but Range is not :(
@ -1067,7 +1067,7 @@ impl LinkCollector<'_, '_> {
} }
res.def_id(self.cx.tcx).map(|page_id| ItemLink { res.def_id(self.cx.tcx).map(|page_id| ItemLink {
link: ori_link.link.clone(), link: Box::<str>::from(&*ori_link.link),
link_text: link_text.clone(), link_text: link_text.clone(),
page_id, page_id,
fragment, fragment,
@ -1091,7 +1091,7 @@ impl LinkCollector<'_, '_> {
let page_id = clean::register_res(self.cx, rustc_hir::def::Res::Def(kind, id)); let page_id = clean::register_res(self.cx, rustc_hir::def::Res::Def(kind, id));
Some(ItemLink { Some(ItemLink {
link: ori_link.link.clone(), link: Box::<str>::from(&*ori_link.link),
link_text: link_text.clone(), link_text: link_text.clone(),
page_id, page_id,
fragment, fragment,

View file

@ -113,7 +113,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
if let Some(link) = if let Some(link) =
link_names.iter().find(|link| *link.original_text == *broken_link.reference) link_names.iter().find(|link| *link.original_text == *broken_link.reference)
{ {
Some((link.href.as_str().into(), link.new_text.as_str().into())) Some((link.href.as_str().into(), link.new_text.to_string().into()))
} else if matches!( } else if matches!(
&broken_link.link_type, &broken_link.link_type,
LinkType::Reference | LinkType::ReferenceUnknown LinkType::Reference | LinkType::ReferenceUnknown