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.

Sign up for GitHub

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

Jump to bottom

rustdoc: avoid many Symbol to String conversions. #91948

Merged
merged 7 commits into from
Jan 15, 2022
Merged

rustdoc: avoid many Symbol to String conversions. #91948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
10b1c9a
rustdoc: avoid many `Symbol` to `String` conversions.
nnethercote Dec 14, 2021
1e4637c
rustdoc: remove many unnecessary `.as_ref()` calls.
nnethercote Dec 14, 2021
6b19cf9
rustdoc: remove some unnecessary sigils.
nnethercote Dec 15, 2021
53f1bed
Use UrlPartsBuilder and remove `join_with_slash`
camelid Jan 10, 2022
8f59eb6
Estimate path length instead of hardcoding 64 bytes
camelid Jan 10, 2022
cef250d
Make `AVG_PART_LENGTH` a power of 2
camelid Jan 10, 2022
c7147e4
Document and test `UrlPartsBuilder::push_fmt`
camelid Jan 10, 2022
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
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ symbols! {
doc_spotlight,
doctest,
document_private_items,
dotdot: "..",
dotdot_in_tuple_patterns,
dotdoteq_in_patterns,
dreg,
Expand Down
9 changes: 3 additions & 6 deletions src/librustdoc/clean/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rustc_data_structures::thin_vec::ThinVec;
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::definitions::DefPathData;
use rustc_hir::Mutability;
use rustc_metadata::creader::{CStore, LoadedMacro};
use rustc_middle::ty::{self, TyCtxt};
Expand Down Expand Up @@ -164,12 +163,10 @@ crate fn load_attrs<'hir>(cx: &DocContext<'hir>, did: DefId) -> Attrs<'hir> {
/// These names are used later on by HTML rendering to generate things like
/// source links back to the original item.
crate fn record_extern_fqn(cx: &mut DocContext<'_>, did: DefId, kind: ItemType) {
let crate_name = cx.tcx.crate_name(did.krate).to_string();
let crate_name = cx.tcx.crate_name(did.krate);

let relative = cx.tcx.def_path(did).data.into_iter().filter_map(|elem| {
// Filter out extern blocks
(elem.data != DefPathData::ForeignMod).then(|| elem.data.to_string())
});
let relative =
cx.tcx.def_path(did).data.into_iter().filter_map(|elem| elem.data.get_opt_name());
let fqn = if let ItemType::Macro = kind {
// Check to see if it is a macro 2.0 or built-in macro
if matches!(
Expand Down
24 changes: 12 additions & 12 deletions src/librustdoc/formats/cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def_id::{CrateNum, DefId, CRATE_DEF_INDEX};
use rustc_middle::middle::privacy::AccessLevels;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::sym;
use rustc_span::{sym, Symbol};

use crate::clean::{self, types::ExternalLocation, ExternalCrate, ItemId, PrimitiveType};
use crate::core::DocContext;
use crate::fold::DocFolder;
use crate::formats::item_type::ItemType;
use crate::formats::Impl;
use crate::html::format::join_with_double_colon;
use crate::html::markdown::short_markdown_summary;
use crate::html::render::search_index::get_function_type_for_search;
use crate::html::render::IndexItem;
Expand Down Expand Up @@ -39,11 +40,11 @@ crate struct Cache {
/// URLs when a type is being linked to. External paths are not located in
/// this map because the `External` type itself has all the information
/// necessary.
crate paths: FxHashMap<DefId, (Vec<String>, ItemType)>,
crate paths: FxHashMap<DefId, (Vec<Symbol>, ItemType)>,

/// Similar to `paths`, but only holds external paths. This is only used for
/// generating explicit hyperlinks to other crates.
crate external_paths: FxHashMap<DefId, (Vec<String>, ItemType)>,
crate external_paths: FxHashMap<DefId, (Vec<Symbol>, ItemType)>,

/// Maps local `DefId`s of exported types to fully qualified paths.
/// Unlike 'paths', this mapping ignores any renames that occur
Expand All @@ -55,7 +56,7 @@ crate struct Cache {
/// to the path used if the corresponding type is inlined. By
/// doing this, we can detect duplicate impls on a trait page, and only display
/// the impl for the inlined type.
crate exact_paths: FxHashMap<DefId, Vec<String>>,
crate exact_paths: FxHashMap<DefId, Vec<Symbol>>,

/// This map contains information about all known traits of this crate.
/// Implementations of a crate should inherit the documentation of the
Expand Down Expand Up @@ -92,7 +93,7 @@ crate struct Cache {
crate masked_crates: FxHashSet<CrateNum>,

// Private fields only used when initially crawling a crate to build a cache
stack: Vec<String>,
stack: Vec<Symbol>,
parent_stack: Vec<DefId>,
parent_is_trait_impl: bool,
stripped_mod: bool,
Expand Down Expand Up @@ -155,7 +156,7 @@ impl Cache {
let dst = &render_options.output;
let location = e.location(extern_url, extern_url_takes_precedence, dst, tcx);
cx.cache.extern_locations.insert(e.crate_num, location);
cx.cache.external_paths.insert(e.def_id(), (vec![name.to_string()], ItemType::Module));
cx.cache.external_paths.insert(e.def_id(), (vec![name], ItemType::Module));
}

// FIXME: avoid this clone (requires implementing Default manually)
Expand All @@ -164,10 +165,9 @@ impl Cache {
let crate_name = tcx.crate_name(def_id.krate);
// Recall that we only allow primitive modules to be at the root-level of the crate.
// If that restriction is ever lifted, this will have to include the relative paths instead.
cx.cache.external_paths.insert(
def_id,
(vec![crate_name.to_string(), prim.as_sym().to_string()], ItemType::Primitive),
);
cx.cache
.external_paths
.insert(def_id, (vec![crate_name, prim.as_sym()], ItemType::Primitive));
}

krate = CacheBuilder { tcx, cache: &mut cx.cache }.fold_crate(krate);
Expand Down Expand Up @@ -299,7 +299,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
self.cache.search_index.push(IndexItem {
ty: item.type_(),
name: s.to_string(),
path: path.join("::"),
path: join_with_double_colon(path),
desc,
parent,
parent_idx: None,
Expand All @@ -320,7 +320,7 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
// Keep track of the fully qualified path for this item.
let pushed = match item.name {
Some(n) if !n.is_empty() => {
self.cache.stack.push(n.to_string());
self.cache.stack.push(n);
true
}
_ => false,
Expand Down
99 changes: 60 additions & 39 deletions src/librustdoc/html/format.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc_middle::ty;
use rustc_middle::ty::DefIdTree;
use rustc_middle::ty::TyCtxt;
use rustc_span::def_id::CRATE_DEF_INDEX;
use rustc_span::{sym, Symbol};
use rustc_target::spec::abi::Abi;

use crate::clean::{
Expand All @@ -29,6 +30,7 @@ use crate::formats::item_type::ItemType;
use crate::html::escape::Escape;
use crate::html::render::Context;

use super::url_parts_builder::estimate_item_path_byte_length;
use super::url_parts_builder::UrlPartsBuilder;

crate trait Print {
Expand Down Expand Up @@ -502,11 +504,22 @@ crate enum HrefError {
NotInExternalCache,
}

// Panics if `syms` is empty.
crate fn join_with_double_colon(syms: &[Symbol]) -> String {
let mut s = String::with_capacity(estimate_item_path_byte_length(syms.len()));
s.push_str(&syms[0].as_str());
for sym in &syms[1..] {
s.push_str("::");
s.push_str(&sym.as_str());
}
s
}

crate fn href_with_root_path(
did: DefId,
cx: &Context<'_>,
root_path: Option<&str>,
) -> Result<(String, ItemType, Vec<String>), HrefError> {
) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
let tcx = cx.tcx();
let def_kind = tcx.def_kind(did);
let did = match def_kind {
Expand All @@ -518,7 +531,7 @@ crate fn href_with_root_path(
};
let cache = cx.cache();
let relative_to = &cx.current;
fn to_module_fqp(shortty: ItemType, fqp: &[String]) -> &[String] {
fn to_module_fqp(shortty: ItemType, fqp: &[Symbol]) -> &[Symbol] {
if shortty == ItemType::Module { fqp } else { &fqp[..fqp.len() - 1] }
}

Expand All @@ -533,9 +546,9 @@ crate fn href_with_root_path(
let mut is_remote = false;
let (fqp, shortty, mut url_parts) = match cache.paths.get(&did) {
Some(&(ref fqp, shortty)) => (fqp, shortty, {
let module_fqp = to_module_fqp(shortty, fqp);
let module_fqp = to_module_fqp(shortty, fqp.as_slice());
debug!(?fqp, ?shortty, ?module_fqp);
href_relative_parts(module_fqp, relative_to)
href_relative_parts(module_fqp, relative_to).collect()
}),
None => {
if let Some(&(ref fqp, shortty)) = cache.external_paths.get(&did) {
Expand All @@ -548,10 +561,12 @@ crate fn href_with_root_path(
is_remote = true;
let s = s.trim_end_matches('/');
let mut builder = UrlPartsBuilder::singleton(s);
builder.extend(module_fqp.iter().map(String::as_str));
builder.extend(module_fqp.iter().copied());
builder
}
ExternalLocation::Local => href_relative_parts(module_fqp, relative_to),
ExternalLocation::Local => {
href_relative_parts(module_fqp, relative_to).collect()
}
ExternalLocation::Unknown => return Err(HrefError::DocumentationNotBuilt),
},
)
Expand All @@ -567,45 +582,50 @@ crate fn href_with_root_path(
}
}
debug!(?url_parts);
let last = &fqp.last().unwrap()[..];
match shortty {
ItemType::Module => {
url_parts.push("index.html");
}
_ => {
let filename = format!("{}.{}.html", shortty.as_str(), last);
url_parts.push(&filename);
let prefix = shortty.as_str();
let last = fqp.last().unwrap();
url_parts.push_fmt(format_args!("{}.{}.html", prefix, last));
}
}
Ok((url_parts.finish(), shortty, fqp.to_vec()))
}

crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<String>), HrefError> {
crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec<Symbol>), HrefError> {
href_with_root_path(did, cx, None)
}

/// Both paths should only be modules.
/// This is because modules get their own directories; that is, `std::vec` and `std::vec::Vec` will
/// both need `../iter/trait.Iterator.html` to get at the iterator trait.
crate fn href_relative_parts(fqp: &[String], relative_to_fqp: &[String]) -> UrlPartsBuilder {
crate fn href_relative_parts<'fqp>(
fqp: &'fqp [Symbol],
relative_to_fqp: &[Symbol],
) -> Box<dyn Iterator<Item = Symbol> + 'fqp> {
for (i, (f, r)) in fqp.iter().zip(relative_to_fqp.iter()).enumerate() {
// e.g. linking to std::iter from std::vec (`dissimilar_part_count` will be 1)
if f != r {
let dissimilar_part_count = relative_to_fqp.len() - i;
let fqp_module = fqp[i..fqp.len()].iter().map(String::as_str);
return iter::repeat("..").take(dissimilar_part_count).chain(fqp_module).collect();
let fqp_module = &fqp[i..fqp.len()];
return box iter::repeat(sym::dotdot)
.take(dissimilar_part_count)
.chain(fqp_module.iter().copied());
}
}
// e.g. linking to std::sync::atomic from std::sync
if relative_to_fqp.len() < fqp.len() {
fqp[relative_to_fqp.len()..fqp.len()].iter().map(String::as_str).collect()
box fqp[relative_to_fqp.len()..fqp.len()].iter().copied()
// e.g. linking to std::sync from std::sync::atomic
} else if fqp.len() < relative_to_fqp.len() {
let dissimilar_part_count = relative_to_fqp.len() - fqp.len();
iter::repeat("..").take(dissimilar_part_count).collect()
box iter::repeat(sym::dotdot).take(dissimilar_part_count)
// linking to the same module
} else {
UrlPartsBuilder::new()
box iter::empty()
}
}

Expand All @@ -632,14 +652,14 @@ fn resolved_path<'cx>(
if let Ok((_, _, fqp)) = href(did, cx) {
format!(
"{}::{}",
fqp[..fqp.len() - 1].join("::"),
anchor(did, fqp.last().unwrap(), cx)
join_with_double_colon(&fqp[..fqp.len() - 1]),
anchor(did, *fqp.last().unwrap(), cx)
)
} else {
last.name.to_string()
}
} else {
anchor(did, last.name.as_str(), cx).to_string()
anchor(did, last.name, cx).to_string()
};
write!(w, "{}{}", path, last.args.print(cx))?;
}
Expand Down Expand Up @@ -668,30 +688,31 @@ fn primitive_link(
needs_termination = true;
}
Some(&def_id) => {
let cname_sym;
let loc = match m.extern_locations[&def_id.krate] {
ExternalLocation::Remote(ref s) => {
cname_sym = ExternalCrate { crate_num: def_id.krate }.name(cx.tcx());
Some(vec![s.trim_end_matches('/'), cname_sym.as_str()])
let cname_sym = ExternalCrate { crate_num: def_id.krate }.name(cx.tcx());
let builder: UrlPartsBuilder =
[s.as_str().trim_end_matches('/'), cname_sym.as_str()]
.into_iter()
.collect();
Some(builder)
}
ExternalLocation::Local => {
cname_sym = ExternalCrate { crate_num: def_id.krate }.name(cx.tcx());
Some(if cx.current.first().map(|x| &x[..]) == Some(cname_sym.as_str()) {
iter::repeat("..").take(cx.current.len() - 1).collect()
let cname_sym = ExternalCrate { crate_num: def_id.krate }.name(cx.tcx());
Some(if cx.current.first() == Some(&cname_sym) {
iter::repeat(sym::dotdot).take(cx.current.len() - 1).collect()
} else {
let cname = iter::once(cname_sym.as_str());
iter::repeat("..").take(cx.current.len()).chain(cname).collect()
iter::repeat(sym::dotdot)
.take(cx.current.len())
.chain(iter::once(cname_sym))
.collect()
})
}
ExternalLocation::Unknown => None,
};
if let Some(loc) = loc {
write!(
f,
"<a class=\"primitive\" href=\"{}/primitive.{}.html\">",
loc.join("/"),
prim.as_sym()
)?;
if let Some(mut loc) = loc {
loc.push_fmt(format_args!("primitive.{}.html", prim.as_sym()));
write!(f, "<a class=\"primitive\" href=\"{}\">", loc.finish())?;
needs_termination = true;
}
}
Expand Down Expand Up @@ -730,7 +751,7 @@ fn tybounds<'a, 'tcx: 'a>(

crate fn anchor<'a, 'cx: 'a>(
did: DefId,
text: &'a str,
text: Symbol,
cx: &'cx Context<'_>,
) -> impl fmt::Display + 'a {
let parts = href(did, cx);
Expand All @@ -742,8 +763,8 @@ crate fn anchor<'a, 'cx: 'a>(
short_ty,
url,
short_ty,
fqp.join("::"),
text
join_with_double_colon(&fqp),
text.as_str()
)
} else {
write!(f, "{}", text)
Expand Down Expand Up @@ -960,7 +981,7 @@ fn fmt_type<'cx>(
url = url,
shortty = ItemType::AssocType,
name = name,
path = path.join("::")
path = join_with_double_colon(path),
)?;
}
_ => write!(f, "{}", name)?,
Expand Down Expand Up @@ -1270,7 +1291,7 @@ impl clean::Visibility {
debug!("path={:?}", path);
// modified from `resolved_path()` to work with `DefPathData`
let last_name = path.data.last().unwrap().data.get_opt_name().unwrap();
let anchor = anchor(vis_did, last_name.as_str(), cx).to_string();
let anchor = anchor(vis_did, last_name, cx).to_string();

let mut s = "pub(in ".to_owned();
for seg in &path.data[..path.data.len() - 1] {
Expand Down
Loading