Auto merge of #142667 - yotamofek:pr/rustdoc/more-write-shared-perf, r=<try>

Avoid a few more allocations in `write_shared.rs`

Inspired by #141421 , avoids a few `Vec`, `PathBuf` and `String` allocations in `write_shared.rs`. I don't think these will show up on benchmarks, but are still worthwhile IMHO.
Also includes a few small cleanups.
r? nnethercote - if you'd like :)

Author: bors

src/librustdoc/html/render/sorted_template.rs

@@ -1,3 +1,4 @@
+use std::borrow::Cow;
 use std::collections::BTreeSet;
 use std::fmt::{self, Write as _};
 use std::marker::PhantomData;
@@ -12,8 +13,8 @@ use serde::{Deserialize, Serialize};
 #[derive(Debug, Clone)]
 pub(crate) struct SortedTemplate<F> {
     format: PhantomData<F>,
-    before: String,
-    after: String,
+    before: Cow<'static, str>,
+    after: Cow<'static, str>,
     fragments: BTreeSet<String>,
 }
 
@@ -38,14 +39,20 @@ impl<F> SortedTemplate<F> {
         if split.next().is_some() {
             return Err(Error("delimiter should appear at most once"));
         }
-        Ok(Self::from_before_after(before, after))
+        Ok(Self::from_before_after(before.to_string(), after.to_string()))
     }
 
     /// Template will insert fragments between `before` and `after`
-    pub(crate) fn from_before_after<S: ToString, T: ToString>(before: S, after: T) -> Self {
-        let before = before.to_string();
-        let after = after.to_string();
-        Self { format: PhantomData, before, after, fragments: Default::default() }
+    pub(crate) fn from_before_after(
+        before: impl Into<Cow<'static, str>>,
+        after: impl Into<Cow<'static, str>>,
+    ) -> Self {
+        Self {
+            format: PhantomData,
+            before: before.into(),
+            after: after.into(),
+            fragments: Default::default(),
+        }
     }
 }
 
@@ -102,8 +109,8 @@ impl<F: FileFormat> FromStr for SortedTemplate<F> {
         }
         Ok(Self {
             format: PhantomData,
-            before: before.to_string(),
-            after: s.to_string(),
+            before: before.to_string().into(),
+            after: s.to_string().into(),
             fragments,
         })
     }

src/librustdoc/html/render/write_shared.rs

@@ -13,6 +13,7 @@
 //!    --resource-suffix flag and are emitted when --emit-type is empty (default)
 //!    or contains "invocation-specific".
 
+use std::borrow::Cow;
 use std::cell::RefCell;
 use std::ffi::OsString;
 use std::fs::File;
@@ -284,7 +285,7 @@ enum CrateInfoVersion {
 #[derive(Serialize, Deserialize, Debug, Clone)]
 #[serde(transparent)]
 struct PartsAndLocations<P> {
-    parts: Vec<(PathBuf, P)>,
+    parts: Vec<(Cow<'static, Path>, P)>,
 }
 
 impl<P> Default for PartsAndLocations<P> {
@@ -294,12 +295,12 @@ impl<P> Default for PartsAndLocations<P> {
 }
 
 impl<T, U> PartsAndLocations<Part<T, U>> {
-    fn push(&mut self, path: PathBuf, item: U) {
-        self.parts.push((path, Part { _artifact: PhantomData, item }));
+    fn push(&mut self, path: impl Into<Cow<'static, Path>>, item: U) {
+        self.parts.push((path.into(), Part { _artifact: PhantomData, item }));
     }
 
     /// Singleton part, one file
-    fn with(path: PathBuf, part: U) -> Self {
+    fn with(path: impl Into<Cow<'static, Path>>, part: U) -> Self {
         let mut ret = Self::default();
         ret.push(path, part);
         ret
@@ -439,24 +440,20 @@ impl CratesIndexPart {
         let content =
             format!("<h1>List of all crates</h1><ul class=\"all-items\">{DELIMITER}</ul>");
         let template = layout::render(layout, &page, "", content, style_files);
-        match SortedTemplate::from_template(&template, DELIMITER) {
-            Ok(template) => template,
-            Err(e) => panic!(
-                "Object Replacement Character (U+FFFC) should not appear in the --index-page: {e}"
-            ),
-        }
+        SortedTemplate::from_template(&template, DELIMITER)
+            .expect("Object Replacement Character (U+FFFC) should not appear in the --index-page")
     }
 
     /// Might return parts that are duplicate with ones in prexisting index.html
     fn get(crate_name: &str, external_crates: &[String]) -> Result<PartsAndLocations<Self>, Error> {
         let mut ret = PartsAndLocations::default();
-        let path = PathBuf::from("index.html");
+        let path = Path::new("index.html");
         for crate_name in external_crates.iter().map(|s| s.as_str()).chain(once(crate_name)) {
             let part = format!(
                 "<li><a href=\"{trailing_slash}index.html\">{crate_name}</a></li>",
                 trailing_slash = ensure_trailing_slash(crate_name),
             );
-            ret.push(path.clone(), part);
+            ret.push(path, part);
         }
         Ok(ret)
     }
@@ -600,7 +597,6 @@ impl TypeAliasPart {
             cx,
         };
         DocVisitor::visit_crate(&mut type_impl_collector, krate);
-        let cx = type_impl_collector.cx;
         let aliased_types = type_impl_collector.aliased_types;
         for aliased_type in aliased_types.values() {
             let impls = aliased_type.impl_.values().filter_map(
@@ -612,7 +608,7 @@ impl TypeAliasPart {
                     for &(type_alias_fqp, type_alias_item) in type_aliases {
                         cx.id_map.borrow_mut().clear();
                         cx.deref_id_map.borrow_mut().clear();
-                        let type_alias_fqp = (*type_alias_fqp).iter().join("::");
+                        let type_alias_fqp = type_alias_fqp.iter().join("::");
                         if let Some(ret) = &mut ret {
                             ret.aliases.push(type_alias_fqp);
                         } else {
@@ -737,7 +733,7 @@ impl TraitAliasPart {
                 },
             };
 
-            let implementors = imps
+            let mut implementors = imps
                 .iter()
                 .filter_map(|imp| {
                     // If the trait and implementation are in the same crate, then
@@ -759,12 +755,12 @@ impl TraitAliasPart {
                         })
                     }
                 })
-                .collect::<Vec<_>>();
+                .peekable();
 
             // Only create a js file if we have impls to add to it. If the trait is
             // documented locally though we always create the file to avoid dead
             // links.
-            if implementors.is_empty() && !cache.paths.contains_key(&did) {
+            if implementors.peek().is_none() && !cache.paths.contains_key(&did) {
                 continue;
             }
 
@@ -775,11 +771,7 @@ impl TraitAliasPart {
             path.push(format!("{remote_item_type}.{}.js", remote_path[remote_path.len() - 1]));
 
             let part = OrderedJson::array_sorted(
-                implementors
-                    .iter()
-                    .map(OrderedJson::serialize)
-                    .collect::<Result<Vec<_>, _>>()
-                    .unwrap(),
+                implementors.map(|implementor| OrderedJson::serialize(implementor).unwrap()),
             );
             path_parts.push(path, OrderedJson::array_unsorted([crate_name_json, &part]));
         }
@@ -874,9 +866,8 @@ impl<'item> DocVisitor<'item> for TypeImplCollector<'_, '_, 'item> {
             let impl_ = cache
                 .impls
                 .get(&target_did)
-                .map(|v| &v[..])
-                .unwrap_or_default()
-                .iter()
+                .into_iter()
+                .flatten()
                 .map(|impl_| {
                     (impl_.impl_item.item_id, AliasedTypeImpl { impl_, type_aliases: Vec::new() })
                 })
@@ -891,14 +882,8 @@ impl<'item> DocVisitor<'item> for TypeImplCollector<'_, '_, 'item> {
         // Exclude impls that are directly on this type. They're already in the HTML.
         // Some inlining scenarios can cause there to be two versions of the same
         // impl: one on the type alias and one on the underlying target type.
-        let mut seen_impls: FxHashSet<ItemId> = cache
-            .impls
-            .get(&self_did)
-            .map(|s| &s[..])
-            .unwrap_or_default()
-            .iter()
-            .map(|i| i.impl_item.item_id)
-            .collect();
+        let mut seen_impls: FxHashSet<ItemId> =
+            cache.impls.get(&self_did).into_iter().flatten().map(|i| i.impl_item.item_id).collect();
         for (impl_item_id, aliased_type_impl) in &mut aliased_type.impl_ {
             // Only include this impl if it actually unifies with this alias.
             // Synthetic impls are not included; those are also included in the HTML.