Skip to content

Commit

Permalink
Auto merge of #108098 - notriddle:notriddle/rustdoc-tooltip-alloc, r=…
Browse files Browse the repository at this point in the history
…GuillaumeGomez

rustdoc: reduce allocations when generating tooltips

An attempt to reduce the perf regression in
#108052 (comment)
  • Loading branch information
bors committed Feb 28, 2023
2 parents fd1f1fa + 49d995a commit 5157d93
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 29 deletions.
16 changes: 10 additions & 6 deletions compiler/rustc_resolve/src/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(['<', '>']) {
return Ok(path_str.to_string());
return Ok(path_str.into());
}
let mut stripped_segments = vec![];
let mut path = path_str.chars().peekable();
Expand Down Expand Up @@ -322,7 +322,11 @@ pub fn strip_generics_from_path(path_str: &str) -> Result<String, MalformedGener

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.
Expand All @@ -336,7 +340,7 @@ pub fn inner_docs(attrs: &[ast::Attribute]) -> bool {
/// Simplified version of the corresponding function in rustdoc.
/// If the rustdoc version returns a successful result, this function must return the same result.
/// 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.split('#').next().unwrap();
let link = link.trim();
Expand All @@ -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 = 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 `[]`,
Expand All @@ -365,7 +369,7 @@ pub fn may_be_doc_link(link_type: LinkType) -> bool {

/// 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.
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 = prepare_to_doc_link_resolution(&doc_fragments).into_values().next().unwrap();

Expand Down
8 changes: 4 additions & 4 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1017,12 +1017,12 @@ pub(crate) fn collapse_doc_fragments(doc_strings: &[DocFragment]) -> String {
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct ItemLink {
/// The original link written in the markdown
pub(crate) link: String,
pub(crate) link: Box<str>,
/// The link text displayed in the HTML.
///
/// This may not be the same as `link` if there was a disambiguator
/// 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
/// 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.
Expand All @@ -1035,9 +1035,9 @@ pub struct RenderedLink {
/// The text the link was original written as.
///
/// 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
pub(crate) new_text: String,
pub(crate) new_text: Box<str>,
/// The URL to put in the `href`
pub(crate) href: String,
/// The tooltip.
Expand Down
19 changes: 13 additions & 6 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -772,14 +772,21 @@ pub(crate) fn link_tooltip(did: DefId, fragment: &Option<UrlFragment>, cx: &Cont
let Some((fqp, shortty)) = cache.paths.get(&did)
.or_else(|| cache.external_paths.get(&did))
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 {
let name = cx.tcx().item_name(id);
let descr = cx.tcx().def_descr(id);
format!("{descr} {fqp}::{name}")
} else {
format!("{shortty} {fqp}")
write!(buf, "{} ", cx.tcx().def_descr(id));
for component in fqp {
write!(buf, "{component}::");
}
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`].
Expand Down
8 changes: 4 additions & 4 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -978,7 +978,7 @@ impl Markdown<'_> {
let mut replacer = |broken_link: BrokenLink<'_>| {
links
.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()))
};

Expand Down Expand Up @@ -1061,7 +1061,7 @@ impl MarkdownSummaryLine<'_> {
let mut replacer = |broken_link: BrokenLink<'_>| {
links
.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()))
};

Expand Down Expand Up @@ -1108,7 +1108,7 @@ fn markdown_summary_with_limit(
let mut replacer = |broken_link: BrokenLink<'_>| {
link_names
.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()))
};

Expand Down Expand Up @@ -1189,7 +1189,7 @@ pub(crate) fn plain_text_summary(md: &str, link_names: &[RenderedLink]) -> Strin
let mut replacer = |broken_link: BrokenLink<'_>| {
link_names
.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()))
};

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/json/conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl JsonRenderer<'_> {
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();
let docs = item.attrs.collapsed_doc_value();
Expand Down
14 changes: 7 additions & 7 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ struct ResolutionInfo {
item_id: ItemId,
module_id: DefId,
dis: Option<Disambiguator>,
path_str: String,
path_str: Box<str>,
extra_fragment: Option<String>,
}

Expand Down Expand Up @@ -849,10 +849,10 @@ impl PreprocessingError {

#[derive(Clone)]
struct PreprocessingInfo {
path_str: String,
path_str: Box<str>,
disambiguator: Option<Disambiguator>,
extra_fragment: Option<String>,
link_text: String,
link_text: Box<str>,
}

// Not a typedef to avoid leaking several private structures from this module.
Expand Down Expand Up @@ -937,7 +937,7 @@ fn preprocess_link(
path_str,
disambiguator,
extra_fragment: extra_fragment.map(|frag| frag.to_owned()),
link_text: link_text.to_owned(),
link_text: Box::<str>::from(link_text),
}))
}

Expand Down Expand Up @@ -993,7 +993,7 @@ impl LinkCollector<'_, '_> {
item_id: item.item_id,
module_id,
dis: disambiguator,
path_str: path_str.to_owned(),
path_str: path_str.clone(),
extra_fragment: extra_fragment.clone(),
},
diag_info.clone(), // this struct should really be Copy, but Range is not :(
Expand Down Expand Up @@ -1067,7 +1067,7 @@ impl LinkCollector<'_, '_> {
}

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(),
page_id,
fragment,
Expand All @@ -1091,7 +1091,7 @@ impl LinkCollector<'_, '_> {

let page_id = clean::register_res(self.cx, rustc_hir::def::Res::Def(kind, id));
Some(ItemLink {
link: ori_link.link.clone(),
link: Box::<str>::from(&*ori_link.link),
link_text: link_text.clone(),
page_id,
fragment,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/lint/html_tags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ pub(crate) fn visit_item(cx: &DocContext<'_>, item: &Item) {
if let Some(link) =
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!(
&broken_link.link_type,
LinkType::Reference | LinkType::ReferenceUnknown
Expand Down

0 comments on commit 5157d93

Please sign in to comment.