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: reduce allocations when generating tooltips #108098

Merged
merged 1 commit into from
Feb 28, 2023
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
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 @@ -981,7 +981,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 @@ -1064,7 +1064,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 @@ -1111,7 +1111,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 @@ -1192,7 +1192,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