Replace String with Symbol in Cache.paths et al.

This should hopefully improve memory usage and rustdoc run time.
It's also more consistent with the rest of rustc and rustdoc.

Author: camelid

src/librustdoc/clean/inline.rs

@@ -162,13 +162,11 @@ 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| {
-        // extern blocks have an empty name
-        let s = elem.data.to_string();
-        if !s.is_empty() { Some(s) } else { None }
-    });
+    // FIXME: use def_id_to_path instead
+    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/clean/utils.rs

@@ -536,3 +536,20 @@ pub(super) fn display_macro_source(
         }
     }
 }
+
+crate fn join_path_segments(segments: &[Symbol]) -> String {
+    join_path_segments_with_sep(segments, "::")
+}
+
+crate fn join_path_segments_with_sep(segments: &[Symbol], sep: &str) -> String {
+    let mut out = String::new();
+    // This can't use an iterator intersperse because then it would be borrowing from
+    // temporary SymbolStrs due to the lifetimes on SymbolStr's Deref impl.
+    for (i, s) in segments.iter().enumerate() {
+        if i > 0 {
+            out.push_str(sep);
+        }
+        out.push_str(&s.as_str())
+    }
+    out
+}

src/librustdoc/formats/cache.rs

@@ -5,7 +5,9 @@ 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::Symbol;
 
+use crate::clean::utils::join_path_segments;
 use crate::clean::{self, ExternalCrate, ItemId, PrimitiveType};
 use crate::core::DocContext;
 use crate::fold::DocFolder;
@@ -39,11 +41,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 +57,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 +94,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,
@@ -156,7 +158,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)
@@ -165,10 +167,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);
@@ -300,7 +301,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_path_segments(path),
                             desc,
                             parent,
                             parent_idx: None,
@@ -321,7 +322,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,8 +19,10 @@ 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::Symbol;
 use rustc_target::spec::abi::Abi;
 
+use crate::clean::utils::join_path_segments;
 use crate::clean::{self, utils::find_nearest_parent_module, ExternalCrate, ItemId, PrimitiveType};
 use crate::formats::item_type::ItemType;
 use crate::html::escape::Escape;
@@ -505,7 +507,7 @@ 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 {
@@ -517,7 +519,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] }
     }
 
@@ -547,7 +549,7 @@ 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),
@@ -566,7 +568,7 @@ crate fn href_with_root_path(
         }
     }
     debug!(?url_parts);
-    let last = &fqp.last().unwrap()[..];
+    let last = &*fqp.last().unwrap().as_str();
     match shortty {
         ItemType::Module => {
             url_parts.push("index.html");
@@ -579,25 +581,28 @@ crate fn href_with_root_path(
     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: &[Symbol], relative_to_fqp: &[String]) -> UrlPartsBuilder {
     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 {
+        if &*f.as_str() != 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()];
+            let mut builder: UrlPartsBuilder =
+                iter::repeat("..").take(dissimilar_part_count).collect();
+            builder.extend(fqp_module.iter().copied());
+            return builder;
         }
     }
     // 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()
+        fqp[relative_to_fqp.len()..fqp.len()].iter().copied().collect()
     // 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();
@@ -631,8 +636,8 @@ fn resolved_path<'cx>(
             if let Ok((_, _, fqp)) = href(did, cx) {
                 format!(
                     "{}::{}",
-                    fqp[..fqp.len() - 1].join("::"),
-                    anchor(did, fqp.last().unwrap(), cx)
+                    join_path_segments(&fqp[..fqp.len() - 1]),
+                    anchor(did, &fqp.last().unwrap().as_str(), cx)
                 )
             } else {
                 last.name.to_string()
@@ -743,7 +748,7 @@ crate fn anchor<'a, 'cx: 'a>(
                 short_ty,
                 url,
                 short_ty,
-                fqp.join("::"),
+                join_path_segments(&fqp),
                 text
             )
         } else {
@@ -961,7 +966,7 @@ fn fmt_type<'cx>(
                         url = url,
                         shortty = ItemType::AssocType,
                         name = name,
-                        path = path.join("::")
+                        path = join_path_segments(path),
                     )?;
                 }
                 _ => write!(f, "{}", name)?,

src/librustdoc/html/render/cache.rs

@@ -8,6 +8,7 @@ use serde::ser::{Serialize, SerializeStruct, Serializer};
 
 use crate::clean;
 use crate::clean::types::{FnDecl, FnRetTy, GenericBound, Generics, Type, WherePredicate};
+use crate::clean::utils::join_path_segments;
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
 use crate::html::markdown::short_markdown_summary;
@@ -38,7 +39,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_path_segments(&fqp[..fqp.len() - 1]),
                 desc,
                 parent: Some(did),
                 parent_idx: None,
@@ -90,7 +91,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt<
                     lastpathid += 1;
 
                     if let Some(&(ref fqp, short)) = paths.get(&defid) {
-                        crate_paths.push((short, fqp.last().unwrap().clone()));
+                        crate_paths.push((short, fqp.last().unwrap().to_string()));
                         Some(pathid)
                     } else {
                         None

src/librustdoc/html/render/context.rs

@@ -237,18 +237,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('/');
                         }
-                        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)),

src/librustdoc/html/render/mod.rs

@@ -63,6 +63,7 @@ use rustc_span::{
 use serde::ser::SerializeSeq;
 use serde::{Serialize, Serializer};
 
+use crate::clean::utils::join_path_segments;
 use crate::clean::{self, ItemId, RenderedLink, SelfTy};
 use crate::error::Error;
 use crate::formats::cache::Cache;
@@ -2524,7 +2525,7 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec<String> {
         let fqp = cache.exact_paths.get(&did).cloned().or_else(get_extern);
 
         if let Some(path) = fqp {
-            out.push(path.join("::"));
+            out.push(join_path_segments(&path));
         }
     };
 

src/librustdoc/html/render/print_item.rs

@@ -22,6 +22,7 @@ use super::{
     ImplRenderingParameters,
 };
 use crate::clean;
+use crate::clean::utils::join_path_segments_with_sep;
 use crate::formats::item_type::ItemType;
 use crate::formats::{AssocItemRender, Impl, RenderMode};
 use crate::html::escape::Escape;
@@ -874,7 +875,7 @@ fn item_trait(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, t: &clean::Tra
             cx.current.join("/")
         } else {
             let (ref path, _) = cache.external_paths[&it.def_id.expect_def_id()];
-            path[..path.len() - 1].join("/")
+            join_path_segments_with_sep(&path[..path.len() - 1], "/")
         },
         ty = it.type_(),
         name = *it.name.as_ref().unwrap()

src/librustdoc/html/render/write_shared.rs

@@ -569,7 +569,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.as_str());
         }
         cx.shared.ensure_dir(&mydst)?;
         mydst.push(&format!("{}.{}.js", remote_item_type, remote_path[remote_path.len() - 1]));

src/librustdoc/html/url_parts_builder.rs

@@ -1,3 +1,5 @@
+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`.
 ///
@@ -115,5 +117,22 @@ impl<'a> Extend<&'a str> for UrlPartsBuilder {
     }
 }
 
+impl FromIterator<Symbol> for UrlPartsBuilder {
+    fn from_iter<T: IntoIterator<Item = Symbol>>(iter: T) -> Self {
+        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<Symbol> for UrlPartsBuilder {
+    fn extend<T: IntoIterator<Item = Symbol>>(&mut self, iter: T) {
+        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;

src/librustdoc/json/mod.rs

@@ -121,7 +121,7 @@ impl 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,
@@ -231,7 +231,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),
                         },
                     )

src/librustdoc/visit_ast.rs

@@ -42,13 +42,9 @@ impl Module<'hir> {
 }
 
 // FIXME: Should this be replaced with tcx.def_path_str?
-fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec<String> {
-    let crate_name = tcx.crate_name(did.krate).to_string();
-    let relative = tcx.def_path(did).data.into_iter().filter_map(|elem| {
-        // extern blocks have an empty name
-        let s = elem.data.to_string();
-        if !s.is_empty() { Some(s) } else { None }
-    });
+fn def_id_to_path(tcx: TyCtxt<'_>, did: DefId) -> Vec<Symbol> {
+    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<DefId, Vec<String>>,
+    exact_paths: FxHashMap<DefId, Vec<Symbol>>,
 }
 
 impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {