From 10b1c9aa8b28f50cf7332960d1fc1e9b1d120eb7 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 15 Dec 2021 06:18:18 +1100 Subject: [PATCH 1/7] rustdoc: avoid many `Symbol` to `String` conversions. Particularly when constructing file paths and fully qualified paths. This avoids a lot of allocations, speeding things up on almost all examples. --- compiler/rustc_span/src/symbol.rs | 1 + src/librustdoc/clean/inline.rs | 9 +- src/librustdoc/formats/cache.rs | 24 ++-- src/librustdoc/html/format.rs | 121 ++++++++++++++------- src/librustdoc/html/render/context.rs | 20 ++-- src/librustdoc/html/render/mod.rs | 7 +- src/librustdoc/html/render/print_item.rs | 27 ++--- src/librustdoc/html/render/search_index.rs | 10 +- src/librustdoc/html/render/write_shared.rs | 2 +- src/librustdoc/html/tests.rs | 48 ++++---- src/librustdoc/html/url_parts_builder.rs | 33 ++++++ src/librustdoc/json/mod.rs | 4 +- src/librustdoc/visit_ast.rs | 12 +- 13 files changed, 198 insertions(+), 120 deletions(-) diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 84cf8878af809..5134d916320ad 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -567,6 +567,7 @@ symbols! { doc_spotlight, doctest, document_private_items, + dotdot: "..", dotdot_in_tuple_patterns, dotdoteq_in_patterns, dreg, diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index b91ba5523e074..a2e612955b349 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -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}; @@ -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!( diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 6b9ccd37cfb37..a8fef4a317802 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -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; @@ -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, ItemType)>, + crate paths: FxHashMap, ItemType)>, /// Similar to `paths`, but only holds external paths. This is only used for /// generating explicit hyperlinks to other crates. - crate external_paths: FxHashMap, ItemType)>, + crate external_paths: FxHashMap, ItemType)>, /// Maps local `DefId`s of exported types to fully qualified paths. /// Unlike 'paths', this mapping ignores any renames that occur @@ -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>, + crate exact_paths: FxHashMap>, /// This map contains information about all known traits of this crate. /// Implementations of a crate should inherit the documentation of the @@ -92,7 +93,7 @@ crate struct Cache { crate masked_crates: FxHashSet, // Private fields only used when initially crawling a crate to build a cache - stack: Vec, + stack: Vec, parent_stack: Vec, parent_is_trait_impl: bool, stripped_mod: bool, @@ -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) @@ -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); @@ -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, @@ -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, diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 981eb9589e9f1..a4cdef9358b47 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -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::{ @@ -502,11 +503,45 @@ crate enum HrefError { NotInExternalCache, } +// This mostly works with sequences of symbols, but sometimes the first item +// comes from a string, and in that case we want to trim any trailing `/`. +// `syms` can be empty. +crate fn join_with_slash(first: Option<&str>, syms: &[Symbol]) -> String { + // 64 bytes covers 99.9%+ of cases. + let mut s = String::with_capacity(64); + if let Some(first) = first { + s.push_str(first.trim_end_matches('/')); + if !syms.is_empty() { + s.push('/'); + } + } + if !syms.is_empty() { + s.push_str(&syms[0].as_str()); + for sym in &syms[1..] { + s.push('/'); + s.push_str(&sym.as_str()); + } + } + s +} + +// Panics if `syms` is empty. +crate fn join_with_double_colon(syms: &[Symbol]) -> String { + // 64 bytes covers 99.9%+ of cases. + let mut s = String::with_capacity(64); + 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), HrefError> { +) -> Result<(String, ItemType, Vec), HrefError> { let tcx = cx.tcx(); let def_kind = tcx.def_kind(did); let did = match def_kind { @@ -518,7 +553,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] } } @@ -533,9 +568,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) { @@ -548,10 +583,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), }, ) @@ -567,45 +604,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), HrefError> { +crate fn href(did: DefId, cx: &Context<'_>) -> Result<(String, ItemType, Vec), 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 + '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() } } @@ -632,14 +674,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))?; } @@ -668,30 +710,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, - "", - loc.join("/"), - prim.as_sym() - )?; + if let Some(mut loc) = loc { + loc.push_fmt(format_args!("primitive.{}.html", prim.as_sym())); + write!(f, "", loc.finish())?; needs_termination = true; } } @@ -730,7 +773,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); @@ -742,8 +785,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) @@ -960,7 +1003,7 @@ fn fmt_type<'cx>( url = url, shortty = ItemType::AssocType, name = name, - path = path.join("::") + path = join_with_double_colon(path), )?; } _ => write!(f, "{}", name)?, @@ -1270,7 +1313,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] { diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 4f3455a92083a..865e14f694dcc 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -11,7 +11,7 @@ use rustc_middle::ty::TyCtxt; use rustc_session::Session; use rustc_span::edition::Edition; use rustc_span::source_map::FileName; -use rustc_span::symbol::sym; +use rustc_span::{sym, Symbol}; use super::print_item::{full_path, item_path, print_item}; use super::search_index::build_index; @@ -29,7 +29,7 @@ use crate::formats::cache::Cache; use crate::formats::item_type::ItemType; use crate::formats::FormatRenderer; use crate::html::escape::Escape; -use crate::html::format::Buffer; +use crate::html::format::{join_with_double_colon, Buffer}; use crate::html::markdown::{self, plain_text_summary, ErrorCodes, IdMap}; use crate::html::{layout, sources}; use crate::scrape_examples::AllCallLocations; @@ -45,7 +45,7 @@ use crate::try_err; crate struct Context<'tcx> { /// Current hierarchy of components leading down to what's currently being /// rendered - pub(crate) current: Vec, + pub(crate) current: Vec, /// The current destination folder of where HTML artifacts should be placed. /// This changes as the context descends into the module hierarchy. crate dst: PathBuf, @@ -176,7 +176,7 @@ impl<'tcx> Context<'tcx> { title.push_str(" in "); } // No need to include the namespace for primitive types and keywords - title.push_str(&self.current.join("::")); + title.push_str(&join_with_double_colon(&self.current)); }; title.push_str(" - Rust"); let tyname = it.type_(); @@ -225,18 +225,18 @@ impl<'tcx> Context<'tcx> { if let Some(&(ref names, ty)) = self.cache().paths.get(&it.def_id.expect_def_id()) { let mut path = String::new(); for name in &names[..names.len() - 1] { - path.push_str(name); + path.push_str(&name.as_str()); path.push('/'); } - path.push_str(&item_path(ty, names.last().unwrap())); + path.push_str(&item_path(ty, &names.last().unwrap().as_str())); match self.shared.redirections { Some(ref redirections) => { let mut current_path = String::new(); for name in &self.current { - current_path.push_str(name); + current_path.push_str(&name.as_str()); current_path.push('/'); } - current_path.push_str(&item_path(ty, names.last().unwrap())); + current_path.push_str(&item_path(ty, &names.last().unwrap().as_str())); redirections.borrow_mut().insert(current_path, path); } None => return layout::redirect(&format!("{}{}", self.root_path(), path)), @@ -634,8 +634,8 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { self.render_redirect_pages = item.is_stripped(); } let scx = &self.shared; - let item_name = item.name.as_ref().unwrap().to_string(); - self.dst.push(&item_name); + let item_name = item.name.unwrap(); + self.dst.push(&*item_name.as_str()); self.current.push(item_name); info!("Recursing into {}", self.dst.display()); diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index dda7490493197..8ba9a6dccacf0 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -69,8 +69,9 @@ use crate::formats::item_type::ItemType; use crate::formats::{AssocItemRender, Impl, RenderMode}; use crate::html::escape::Escape; use crate::html::format::{ - href, print_abi_with_space, print_constness_with_space, print_default_space, - print_generic_bounds, print_where_clause, Buffer, HrefError, PrintWithSpace, + href, join_with_double_colon, print_abi_with_space, print_constness_with_space, + print_default_space, print_generic_bounds, print_where_clause, Buffer, HrefError, + PrintWithSpace, }; use crate::html::highlight; use crate::html::markdown::{HeadingOffset, Markdown, MarkdownHtml, MarkdownSummaryLine}; @@ -2515,7 +2516,7 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec { let fqp = cache.exact_paths.get(&did).cloned().or_else(get_extern); if let Some(path) = fqp { - out.push(path.join("::")); + out.push(join_with_double_colon(&path)); } }; diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 5431bd8565b57..6f7ba8aff5d50 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -26,7 +26,8 @@ use crate::formats::item_type::ItemType; use crate::formats::{AssocItemRender, Impl, RenderMode}; use crate::html::escape::Escape; use crate::html::format::{ - print_abi_with_space, print_constness_with_space, print_where_clause, Buffer, PrintWithSpace, + join_with_double_colon, join_with_slash, print_abi_with_space, print_constness_with_space, + print_where_clause, Buffer, PrintWithSpace, }; use crate::html::highlight; use crate::html::layout::Page; @@ -40,9 +41,9 @@ const ITEM_TABLE_ROW_OPEN: &str = "
"; const ITEM_TABLE_ROW_CLOSE: &str = "
"; // A component in a `use` path, like `string` in std::string::ToString -struct PathComponent<'a> { +struct PathComponent { path: String, - name: &'a str, + name: Symbol, } #[derive(Template)] @@ -53,7 +54,7 @@ struct ItemVars<'a> { typ: &'a str, name: &'a str, item_type: &'a str, - path_components: Vec>, + path_components: Vec, stability_since_raw: &'a str, src_href: Option<&'a str>, } @@ -121,7 +122,7 @@ pub(super) fn print_item(cx: &Context<'_>, item: &clean::Item, buf: &mut Buffer, .take(amt) .map(|(i, component)| PathComponent { path: "../".repeat(cur.len() - i - 1), - name: component, + name: *component, }) .collect() }; @@ -304,22 +305,18 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl w.write_str(ITEM_TABLE_ROW_OPEN); match *src { - Some(ref src) => write!( + Some(src) => write!( w, "
{}extern crate {} as {};", myitem.visibility.print_with_space(myitem.def_id, cx), - anchor(myitem.def_id.expect_def_id(), src.as_str(), cx), + anchor(myitem.def_id.expect_def_id(), src, cx), myitem.name.as_ref().unwrap(), ), None => write!( w, "
{}extern crate {};", myitem.visibility.print_with_space(myitem.def_id, cx), - anchor( - myitem.def_id.expect_def_id(), - myitem.name.as_ref().unwrap().as_str(), - cx - ), + anchor(myitem.def_id.expect_def_id(), *myitem.name.as_ref().unwrap(), cx), ), } w.write_str("
"); @@ -864,10 +861,10 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra ", root_path = vec![".."; cx.current.len()].join("/"), path = if it.def_id.is_local() { - cx.current.join("/") + join_with_slash(None, &cx.current) } else { let (ref path, _) = cache.external_paths[&it.def_id.expect_def_id()]; - path[..path.len() - 1].join("/") + join_with_slash(None, &path[..path.len() - 1]) }, ty = it.type_(), name = *it.name.as_ref().unwrap() @@ -1410,7 +1407,7 @@ crate fn compare_names(mut lhs: &str, mut rhs: &str) -> Ordering { } pub(super) fn full_path(cx: &Context<'_>, item: &clean::Item) -> String { - let mut s = cx.current.join("::"); + let mut s = join_with_double_colon(&cx.current); s.push_str("::"); s.push_str(item.name.unwrap().as_str()); s diff --git a/src/librustdoc/html/render/search_index.rs b/src/librustdoc/html/render/search_index.rs index 0fbe090f2190a..87138b9571c2a 100644 --- a/src/librustdoc/html/render/search_index.rs +++ b/src/librustdoc/html/render/search_index.rs @@ -10,6 +10,7 @@ use crate::clean; use crate::clean::types::{FnRetTy, Function, GenericBound, Generics, Type, WherePredicate}; use crate::formats::cache::Cache; use crate::formats::item_type::ItemType; +use crate::html::format::join_with_double_colon; use crate::html::markdown::short_markdown_summary; use crate::html::render::{IndexItem, IndexItemFunctionType, RenderType, TypeWithKind}; @@ -28,7 +29,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< cache.search_index.push(IndexItem { ty: item.type_(), name: item.name.unwrap().to_string(), - path: fqp[..fqp.len() - 1].join("::"), + path: join_with_double_colon(&fqp[..fqp.len() - 1]), desc, parent: Some(did), parent_idx: None, @@ -102,7 +103,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< struct CrateData<'a> { doc: String, items: Vec<&'a IndexItem>, - paths: Vec<(ItemType, String)>, + paths: Vec<(ItemType, Symbol)>, // The String is alias name and the vec is the list of the elements with this alias. // // To be noted: the `usize` elements are indexes to `items`. @@ -154,7 +155,10 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< "f", &self.items.iter().map(|item| &item.search_type).collect::>(), )?; - crate_data.serialize_field("p", &self.paths)?; + crate_data.serialize_field( + "p", + &self.paths.iter().map(|(it, s)| (it, s.to_string())).collect::>(), + )?; if has_aliases { crate_data.serialize_field("a", &self.aliases)?; } diff --git a/src/librustdoc/html/render/write_shared.rs b/src/librustdoc/html/render/write_shared.rs index 2e763dbd8fe9e..e6900332a0429 100644 --- a/src/librustdoc/html/render/write_shared.rs +++ b/src/librustdoc/html/render/write_shared.rs @@ -562,7 +562,7 @@ pub(super) fn write_shared( let mut mydst = dst.clone(); for part in &remote_path[..remote_path.len() - 1] { - mydst.push(part); + mydst.push(part.to_string()); } cx.shared.ensure_dir(&mydst)?; mydst.push(&format!("{}.{}.js", remote_item_type, remote_path[remote_path.len() - 1])); diff --git a/src/librustdoc/html/tests.rs b/src/librustdoc/html/tests.rs index dee9f5e5038a8..437d3995e29fc 100644 --- a/src/librustdoc/html/tests.rs +++ b/src/librustdoc/html/tests.rs @@ -1,44 +1,50 @@ use crate::html::format::href_relative_parts; +use rustc_span::{sym, Symbol}; -fn assert_relative_path(expected: &str, relative_to_fqp: &[&str], fqp: &[&str]) { - let relative_to_fqp: Vec = relative_to_fqp.iter().copied().map(String::from).collect(); - let fqp: Vec = fqp.iter().copied().map(String::from).collect(); - assert_eq!(expected, href_relative_parts(&fqp, &relative_to_fqp).finish()); +fn assert_relative_path(expected: &[Symbol], relative_to_fqp: &[Symbol], fqp: &[Symbol]) { + // No `create_default_session_globals_then` call is needed here because all + // the symbols used are static, and no `Symbol::intern` calls occur. + assert_eq!(expected, href_relative_parts(&fqp, &relative_to_fqp).collect::>()); } #[test] fn href_relative_parts_basic() { - let relative_to_fqp = &["std", "vec"]; - let fqp = &["std", "iter"]; - assert_relative_path("../iter", relative_to_fqp, fqp); + let relative_to_fqp = &[sym::std, sym::vec]; + let fqp = &[sym::std, sym::iter]; + assert_relative_path(&[sym::dotdot, sym::iter], relative_to_fqp, fqp); } + #[test] fn href_relative_parts_parent_module() { - let relative_to_fqp = &["std", "vec"]; - let fqp = &["std"]; - assert_relative_path("..", relative_to_fqp, fqp); + let relative_to_fqp = &[sym::std, sym::vec]; + let fqp = &[sym::std]; + assert_relative_path(&[sym::dotdot], relative_to_fqp, fqp); } + #[test] fn href_relative_parts_different_crate() { - let relative_to_fqp = &["std", "vec"]; - let fqp = &["core", "iter"]; - assert_relative_path("../../core/iter", relative_to_fqp, fqp); + let relative_to_fqp = &[sym::std, sym::vec]; + let fqp = &[sym::core, sym::iter]; + assert_relative_path(&[sym::dotdot, sym::dotdot, sym::core, sym::iter], relative_to_fqp, fqp); } + #[test] fn href_relative_parts_same_module() { - let relative_to_fqp = &["std", "vec"]; - let fqp = &["std", "vec"]; - assert_relative_path("", relative_to_fqp, fqp); + let relative_to_fqp = &[sym::std, sym::vec]; + let fqp = &[sym::std, sym::vec]; + assert_relative_path(&[], relative_to_fqp, fqp); } + #[test] fn href_relative_parts_child_module() { - let relative_to_fqp = &["std"]; - let fqp = &["std", "vec"]; - assert_relative_path("vec", relative_to_fqp, fqp); + let relative_to_fqp = &[sym::std]; + let fqp = &[sym::std, sym::vec]; + assert_relative_path(&[sym::vec], relative_to_fqp, fqp); } + #[test] fn href_relative_parts_root() { let relative_to_fqp = &[]; - let fqp = &["std"]; - assert_relative_path("std", relative_to_fqp, fqp); + let fqp = &[sym::std]; + assert_relative_path(&[sym::std], relative_to_fqp, fqp); } diff --git a/src/librustdoc/html/url_parts_builder.rs b/src/librustdoc/html/url_parts_builder.rs index 918d5e6bd1b3a..5c1557078aa80 100644 --- a/src/librustdoc/html/url_parts_builder.rs +++ b/src/librustdoc/html/url_parts_builder.rs @@ -1,3 +1,7 @@ +use std::fmt::{self, Write}; + +use rustc_span::Symbol; + /// A builder that allows efficiently and easily constructing the part of a URL /// after the domain: `nightly/core/str/struct.Bytes.html`. /// @@ -10,6 +14,7 @@ crate struct UrlPartsBuilder { impl UrlPartsBuilder { /// Create an empty buffer. + #[allow(dead_code)] crate fn new() -> Self { Self { buf: String::new() } } @@ -62,6 +67,13 @@ impl UrlPartsBuilder { self.buf.push_str(part); } + crate fn push_fmt(&mut self, args: fmt::Arguments<'_>) { + if !self.buf.is_empty() { + self.buf.push('/'); + } + self.buf.write_fmt(args).unwrap() + } + /// Push a component onto the front of the buffer. /// /// # Examples @@ -115,5 +127,26 @@ impl<'a> Extend<&'a str> for UrlPartsBuilder { } } +impl FromIterator for UrlPartsBuilder { + fn from_iter>(iter: T) -> Self { + // This code has to be duplicated from the `&str` impl because of + // `Symbol::as_str`'s lifetimes. + let iter = iter.into_iter(); + let mut builder = Self::with_capacity_bytes(AVG_PART_LENGTH * iter.size_hint().0); + iter.for_each(|part| builder.push(part.as_str())); + builder + } +} + +impl Extend for UrlPartsBuilder { + fn extend>(&mut self, iter: T) { + // This code has to be duplicated from the `&str` impl because of + // `Symbol::as_str`'s lifetimes. + let iter = iter.into_iter(); + self.buf.reserve(AVG_PART_LENGTH * iter.size_hint().0); + iter.for_each(|part| self.push(part.as_str())); + } +} + #[cfg(test)] mod tests; diff --git a/src/librustdoc/json/mod.rs b/src/librustdoc/json/mod.rs index 88a5c0c5ca260..81fbfd9fdbd16 100644 --- a/src/librustdoc/json/mod.rs +++ b/src/librustdoc/json/mod.rs @@ -120,7 +120,7 @@ impl<'tcx> JsonRenderer<'tcx> { }) .0 .last() - .map(Clone::clone), + .map(|s| s.to_string()), visibility: types::Visibility::Public, inner: types::ItemEnum::Trait(trait_item.clone().into_tcx(self.tcx)), span: None, @@ -230,7 +230,7 @@ impl<'tcx> FormatRenderer<'tcx> for JsonRenderer<'tcx> { from_item_id(k.into()), types::ItemSummary { crate_id: k.krate.as_u32(), - path, + path: path.iter().map(|s| s.to_string()).collect(), kind: kind.into_tcx(self.tcx), }, ) diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 6f1736afc3bdc..90cb5d586c211 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -5,7 +5,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; 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::Node; use rustc_hir::CRATE_HIR_ID; use rustc_middle::middle::privacy::AccessLevel; @@ -43,12 +42,9 @@ impl Module<'_> { } // FIXME: Should this be replaced with tcx.def_path_str? -fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec { - let crate_name = tcx.crate_name(did.krate).to_string(); - let relative = tcx.def_path(did).data.into_iter().filter_map(|elem| { - // Filter out extern blocks - (elem.data != DefPathData::ForeignMod).then(|| elem.data.to_string()) - }); +fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec { + let crate_name = tcx.crate_name(did.krate); + let relative = tcx.def_path(did).data.into_iter().filter_map(|elem| elem.data.get_opt_name()); std::iter::once(crate_name).chain(relative).collect() } @@ -71,7 +67,7 @@ crate struct RustdocVisitor<'a, 'tcx> { inlining: bool, /// Are the current module and all of its parents public? inside_public_path: bool, - exact_paths: FxHashMap>, + exact_paths: FxHashMap>, } impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { From 1e4637cf4d7cfb527cf7ec6e44dfb36e1e008985 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 15 Dec 2021 10:01:08 +1100 Subject: [PATCH 2/7] rustdoc: remove many unnecessary `.as_ref()` calls. --- src/librustdoc/html/render/print_item.rs | 67 ++++++++++-------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 6f7ba8aff5d50..dacaeac78f2b9 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -263,7 +263,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl indices.dedup_by_key(|i| { ( items[*i].def_id, - if items[*i].name.as_ref().is_some() { Some(full_path(cx, &items[*i])) } else { None }, + if items[*i].name.is_some() { Some(full_path(cx, &items[*i])) } else { None }, items[*i].type_(), if items[*i].is_import() { *i } else { 0 }, ) @@ -310,13 +310,13 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl "
{}extern crate {} as {};", myitem.visibility.print_with_space(myitem.def_id, cx), anchor(myitem.def_id.expect_def_id(), src, cx), - myitem.name.as_ref().unwrap(), + myitem.name.unwrap(), ), None => write!( w, "
{}extern crate {};", myitem.visibility.print_with_space(myitem.def_id, cx), - anchor(myitem.def_id.expect_def_id(), *myitem.name.as_ref().unwrap(), cx), + anchor(myitem.def_id.expect_def_id(), myitem.name.unwrap(), cx), ), } w.write_str("
"); @@ -388,7 +388,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl {stab_tags}\
\
{docs}
", - name = *myitem.name.as_ref().unwrap(), + name = myitem.name.unwrap(), stab_tags = extra_info_tags(myitem, item, cx.tcx()), docs = MarkdownSummaryLine(&doc_value, &myitem.links(cx)).into_string(), class = myitem.type_(), @@ -460,7 +460,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean:: let asyncness = f.header.asyncness.print_with_space(); let unsafety = f.header.unsafety.print_with_space(); let abi = print_abi_with_space(f.header.abi).to_string(); - let name = it.name.as_ref().unwrap(); + let name = it.name.unwrap(); let generics_len = format!("{:#}", f.generics.print(cx)).len(); let header_len = "fn ".len() @@ -516,7 +516,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra it.visibility.print_with_space(it.def_id, cx), t.unsafety.print_with_space(), if t.is_auto { "auto " } else { "" }, - it.name.as_ref().unwrap(), + it.name.unwrap(), t.generics.print(cx), bounds ); @@ -657,7 +657,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra } fn trait_item(w: &mut Buffer, cx: &Context<'_>, m: &clean::Item, t: &clean::Item) { - let name = m.name.as_ref().unwrap(); + let name = m.name.unwrap(); info!("Documenting {} on {:?}", name, t.name); let item_type = m.type_(); let id = cx.derive_id(format!("{}.{}", item_type, name)); @@ -867,7 +867,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra join_with_slash(None, &path[..path.len() - 1]) }, ty = it.type_(), - name = *it.name.as_ref().unwrap() + name = it.name.unwrap() ); } @@ -878,7 +878,7 @@ fn item_trait_alias(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clea write!( w, "trait {}{}{} = {};", - it.name.as_ref().unwrap(), + it.name.unwrap(), t.generics.print(cx), print_where_clause(&t.generics, cx, 0, true), bounds(&t.bounds, true, cx) @@ -902,7 +902,7 @@ fn item_opaque_ty(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean: write!( w, "type {}{}{where_clause} = impl {bounds};", - it.name.as_ref().unwrap(), + it.name.unwrap(), t.generics.print(cx), where_clause = print_where_clause(&t.generics, cx, 0, true), bounds = bounds(&t.bounds, false, cx), @@ -941,7 +941,7 @@ fn item_typedef( write!( w, "type {}{}{where_clause} = {type_};", - it.name.as_ref().unwrap(), + it.name.unwrap(), t.generics.print(cx), where_clause = print_where_clause(&t.generics, cx, 0, true), type_ = t.type_.print(cx), @@ -994,7 +994,7 @@ fn item_union(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::Uni Fields" ); for (field, ty) in fields { - let name = field.name.as_ref().expect("union field name"); + let name = field.name.expect("union field name"); let id = format!("{}.{}", ItemType::StructField, name); write!( w, @@ -1039,7 +1039,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum w, "{}enum {}{}{}", it.visibility.print_with_space(it.def_id, cx), - it.name.as_ref().unwrap(), + it.name.unwrap(), e.generics.print(cx), print_where_clause(&e.generics, cx, 0, true), ); @@ -1054,7 +1054,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum } for v in &e.variants { w.write_str(" "); - let name = v.name.as_ref().unwrap(); + let name = v.name.unwrap(); match *v.kind { clean::VariantItem(ref var) => match var { clean::Variant::CLike => write!(w, "{}", name), @@ -1103,15 +1103,14 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum ); document_non_exhaustive(w, it); for variant in &e.variants { - let id = - cx.derive_id(format!("{}.{}", ItemType::Variant, variant.name.as_ref().unwrap())); + let id = cx.derive_id(format!("{}.{}", ItemType::Variant, variant.name.unwrap())); write!( w, "

\ \ {name}", id = id, - name = variant.name.as_ref().unwrap() + name = variant.name.unwrap() ); if let clean::VariantItem(clean::Variant::Tuple(ref s)) = *variant.kind { w.write_str("("); @@ -1137,11 +1136,8 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum }; if let Some((heading, fields)) = heading_and_fields { - let variant_id = cx.derive_id(format!( - "{}.{}.fields", - ItemType::Variant, - variant.name.as_ref().unwrap() - )); + let variant_id = + cx.derive_id(format!("{}.{}.fields", ItemType::Variant, variant.name.unwrap())); write!(w, "
", id = variant_id); write!(w, "

{heading}

", heading = heading); document_non_exhaustive(w, variant); @@ -1151,8 +1147,8 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum clean::StructFieldItem(ref ty) => { let id = cx.derive_id(format!( "variant.{}.field.{}", - variant.name.as_ref().unwrap(), - field.name.as_ref().unwrap() + variant.name.unwrap(), + field.name.unwrap() )); write!( w, @@ -1162,7 +1158,7 @@ fn item_enum(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, e: &clean::Enum {f}: {t}\ ", id = id, - f = field.name.as_ref().unwrap(), + f = field.name.unwrap(), t = ty.print(cx) ); document(w, cx, field, Some(variant), HeadingOffset::H5); @@ -1201,7 +1197,7 @@ fn item_macro(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Mac fn item_proc_macro(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, m: &clean::ProcMacro) { wrap_into_docblock(w, |w| { - let name = it.name.as_ref().expect("proc-macros always have names"); + let name = it.name.expect("proc-macros always have names"); match m.kind { MacroKind::Bang => { wrap_item(w, "macro", |w| { @@ -1245,7 +1241,7 @@ fn item_constant(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, c: &clean:: w, "{vis}const {name}: {typ}", vis = it.visibility.print_with_space(it.def_id, cx), - name = it.name.as_ref().unwrap(), + name = it.name.unwrap(), typ = c.type_.print(cx), ); @@ -1338,7 +1334,7 @@ fn item_static(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, s: &clean::St "{vis}static {mutability}{name}: {typ}", vis = it.visibility.print_with_space(it.def_id, cx), mutability = s.mutability.print_with_space(), - name = it.name.as_ref().unwrap(), + name = it.name.unwrap(), typ = s.type_.print(cx) ); }); @@ -1355,7 +1351,7 @@ fn item_foreign_type(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item) { w, " {}type {};\n}}", it.visibility.print_with_space(it.def_id, cx), - it.name.as_ref().unwrap(), + it.name.unwrap(), ); }); }); @@ -1523,12 +1519,7 @@ fn render_union( tab: &str, cx: &Context<'_>, ) { - write!( - w, - "{}union {}", - it.visibility.print_with_space(it.def_id, cx), - it.name.as_ref().unwrap() - ); + write!(w, "{}union {}", it.visibility.print_with_space(it.def_id, cx), it.name.unwrap()); if let Some(g) = g { write!(w, "{}", g.print(cx)); write!(w, "{}", print_where_clause(g, cx, 0, true)); @@ -1548,7 +1539,7 @@ fn render_union( w, " {}{}: {},\n{}", field.visibility.print_with_space(field.def_id, cx), - field.name.as_ref().unwrap(), + field.name.unwrap(), ty.print(cx), tab ); @@ -1579,7 +1570,7 @@ fn render_struct( "{}{}{}", it.visibility.print_with_space(it.def_id, cx), if structhead { "struct " } else { "" }, - it.name.as_ref().unwrap() + it.name.unwrap() ); if let Some(g) = g { write!(w, "{}", g.print(cx)) @@ -1604,7 +1595,7 @@ fn render_struct( "\n{} {}{}: {},", tab, field.visibility.print_with_space(field.def_id, cx), - field.name.as_ref().unwrap(), + field.name.unwrap(), ty.print(cx), ); } From 6b19cf9f7438c1ef715e5fb9a4dff508f8e74dae Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 15 Dec 2021 11:14:21 +1100 Subject: [PATCH 3/7] rustdoc: remove some unnecessary sigils. --- src/librustdoc/html/format.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index a4cdef9358b47..b6cb0fbc76a19 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -786,7 +786,7 @@ crate fn anchor<'a, 'cx: 'a>( url, short_ty, join_with_double_colon(&fqp), - &*text.as_str() + text.as_str() ) } else { write!(f, "{}", text) From 53f1bed83ab3124ddc26c8e1e28666ae8a0f1842 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 10 Jan 2022 12:18:26 -0800 Subject: [PATCH 4/7] Use UrlPartsBuilder and remove `join_with_slash` --- src/librustdoc/html/format.rs | 22 ----------------- src/librustdoc/html/render/print_item.rs | 30 +++++++++++++----------- 2 files changed, 16 insertions(+), 36 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index b6cb0fbc76a19..342c15855d214 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -503,28 +503,6 @@ crate enum HrefError { NotInExternalCache, } -// This mostly works with sequences of symbols, but sometimes the first item -// comes from a string, and in that case we want to trim any trailing `/`. -// `syms` can be empty. -crate fn join_with_slash(first: Option<&str>, syms: &[Symbol]) -> String { - // 64 bytes covers 99.9%+ of cases. - let mut s = String::with_capacity(64); - if let Some(first) = first { - s.push_str(first.trim_end_matches('/')); - if !syms.is_empty() { - s.push('/'); - } - } - if !syms.is_empty() { - s.push_str(&syms[0].as_str()); - for sym in &syms[1..] { - s.push('/'); - s.push_str(&sym.as_str()); - } - } - s -} - // Panics if `syms` is empty. crate fn join_with_double_colon(syms: &[Symbol]) -> String { // 64 bytes covers 99.9%+ of cases. diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index dacaeac78f2b9..eda637acfc5b9 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -26,12 +26,13 @@ use crate::formats::item_type::ItemType; use crate::formats::{AssocItemRender, Impl, RenderMode}; use crate::html::escape::Escape; use crate::html::format::{ - join_with_double_colon, join_with_slash, print_abi_with_space, print_constness_with_space, - print_where_clause, Buffer, PrintWithSpace, + join_with_double_colon, print_abi_with_space, print_constness_with_space, print_where_clause, + Buffer, PrintWithSpace, }; use crate::html::highlight; use crate::html::layout::Page; use crate::html::markdown::{HeadingOffset, MarkdownSummaryLine}; +use crate::html::url_parts_builder::UrlPartsBuilder; use askama::Template; @@ -854,20 +855,21 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra } } + let mut js_src_path: UrlPartsBuilder = std::iter::repeat("..") + .take(cx.current.len()) + .chain(std::iter::once("implementors")) + .collect(); + if it.def_id.is_local() { + js_src_path.extend(cx.current.iter().copied()); + } else { + let (ref path, _) = cache.external_paths[&it.def_id.expect_def_id()]; + js_src_path.extend(path[..path.len() - 1].iter().copied()); + } + js_src_path.push_fmt(format_args!("{}.{}.js", it.type_(), it.name.unwrap())); write!( w, - "", - root_path = vec![".."; cx.current.len()].join("/"), - path = if it.def_id.is_local() { - join_with_slash(None, &cx.current) - } else { - let (ref path, _) = cache.external_paths[&it.def_id.expect_def_id()]; - join_with_slash(None, &path[..path.len() - 1]) - }, - ty = it.type_(), - name = it.name.unwrap() + "", + src = js_src_path.finish(), ); } From 8f59eb6da014cb146c1477ab0ddf35729c88e99f Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 10 Jan 2022 12:23:58 -0800 Subject: [PATCH 5/7] Estimate path length instead of hardcoding 64 bytes --- src/librustdoc/html/format.rs | 4 ++-- src/librustdoc/html/url_parts_builder.rs | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 342c15855d214..8571a6a137f51 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -30,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 { @@ -505,8 +506,7 @@ crate enum HrefError { // Panics if `syms` is empty. crate fn join_with_double_colon(syms: &[Symbol]) -> String { - // 64 bytes covers 99.9%+ of cases. - let mut s = String::with_capacity(64); + 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("::"); diff --git a/src/librustdoc/html/url_parts_builder.rs b/src/librustdoc/html/url_parts_builder.rs index 5c1557078aa80..26cebe8c72bea 100644 --- a/src/librustdoc/html/url_parts_builder.rs +++ b/src/librustdoc/html/url_parts_builder.rs @@ -110,6 +110,14 @@ impl UrlPartsBuilder { /// This is intentionally on the lower end to avoid overallocating. const AVG_PART_LENGTH: usize = 5; +/// Estimate the number of bytes in an item's path, based on how many segments it has. +/// +/// **Note:** This is only to be used with, e.g., [`String::with_capacity()`]; +/// the return value is just a rough estimate. +crate const fn estimate_item_path_byte_length(segment_count: usize) -> usize { + AVG_PART_LENGTH * segment_count +} + impl<'a> FromIterator<&'a str> for UrlPartsBuilder { fn from_iter>(iter: T) -> Self { let iter = iter.into_iter(); From cef250d90bbf65af7ec3c8ff5865eaa12a5f4a21 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 10 Jan 2022 12:24:20 -0800 Subject: [PATCH 6/7] Make `AVG_PART_LENGTH` a power of 2 I seem to recall that in general, it's best to request an allocation with a size that's a power of 2. The low estimate of 5 was probably a little too low as well. --- src/librustdoc/html/url_parts_builder.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/url_parts_builder.rs b/src/librustdoc/html/url_parts_builder.rs index 26cebe8c72bea..66c879b83925c 100644 --- a/src/librustdoc/html/url_parts_builder.rs +++ b/src/librustdoc/html/url_parts_builder.rs @@ -105,10 +105,17 @@ impl UrlPartsBuilder { /// This is just a guess at the average length of a URL part, /// used for [`String::with_capacity`] calls in the [`FromIterator`] -/// and [`Extend`] impls. +/// and [`Extend`] impls, and for [estimating item path lengths]. /// -/// This is intentionally on the lower end to avoid overallocating. -const AVG_PART_LENGTH: usize = 5; +/// The value `8` was chosen for two main reasons: +/// +/// * It seems like a good guess for the average part length. +/// * jemalloc's size classes are all multiples of eight, +/// which means that the amount of memory it allocates will often match +/// the amount requested, avoiding wasted bytes. +/// +/// [estimating item path lengths]: estimate_item_path_byte_length +const AVG_PART_LENGTH: usize = 8; /// Estimate the number of bytes in an item's path, based on how many segments it has. /// From c7147e4e1acd49ea01e6d67a85c270946554783a Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 10 Jan 2022 15:33:20 -0800 Subject: [PATCH 7/7] Document and test `UrlPartsBuilder::push_fmt` --- src/librustdoc/html/url_parts_builder.rs | 13 +++++++++++++ src/librustdoc/html/url_parts_builder/tests.rs | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/librustdoc/html/url_parts_builder.rs b/src/librustdoc/html/url_parts_builder.rs index 66c879b83925c..2bb78aa7dc9c0 100644 --- a/src/librustdoc/html/url_parts_builder.rs +++ b/src/librustdoc/html/url_parts_builder.rs @@ -67,6 +67,19 @@ impl UrlPartsBuilder { self.buf.push_str(part); } + /// Push a component onto the buffer, using [`format!`]'s formatting syntax. + /// + /// # Examples + /// + /// Basic usage (equivalent to the example for [`UrlPartsBuilder::push`]): + /// + /// ```ignore (private-type) + /// let mut builder = UrlPartsBuilder::new(); + /// builder.push("core"); + /// builder.push("str"); + /// builder.push_fmt(format_args!("{}.{}.html", "struct", "Bytes")); + /// assert_eq!(builder.finish(), "core/str/struct.Bytes.html"); + /// ``` crate fn push_fmt(&mut self, args: fmt::Arguments<'_>) { if !self.buf.is_empty() { self.buf.push('/'); diff --git a/src/librustdoc/html/url_parts_builder/tests.rs b/src/librustdoc/html/url_parts_builder/tests.rs index 43338c95010a0..636e1ab55279f 100644 --- a/src/librustdoc/html/url_parts_builder/tests.rs +++ b/src/librustdoc/html/url_parts_builder/tests.rs @@ -40,6 +40,16 @@ fn push_front_non_empty() { t(builder, "nightly/core/str/struct.Bytes.html"); } +#[test] +fn push_fmt() { + let mut builder = UrlPartsBuilder::new(); + builder.push_fmt(format_args!("{}", "core")); + builder.push("str"); + builder.push_front("nightly"); + builder.push_fmt(format_args!("{}.{}.html", "struct", "Bytes")); + t(builder, "nightly/core/str/struct.Bytes.html"); +} + #[test] fn collect() { t(["core", "str"].into_iter().collect(), "core/str");