Auto merge of #91948 - nnethercote:rustdoc-more-Symbols, r=GuillaumeGomez

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.

r? `@GuillaumeGomez`

Author: bors

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,

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!(

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<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
@@ -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
@@ -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,
@@ -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,

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::{
@@ -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 {
@@ -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 {
@@ -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] }
     }
 
@@ -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) {
@@ -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),
                     },
                 )
@@ -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()
     }
 }
 
@@ -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))?;
     }
@@ -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;
                 }
             }
@@ -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);
@@ -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)
@@ -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)?,
@@ -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] {