From 4b806878549990d2ad2aa3c265751d3d89947cdf Mon Sep 17 00:00:00 2001 From: bors Date: Sat, 30 Jan 2021 01:02:18 +0000 Subject: [PATCH] rustdoc tweaking * Reuse memory * simplify `next_def_id`, avoid multiple hashing and unnecessary lookups * remove `all_fake_def_ids`, use the global map instead (probably not a good step toward parallelization, though...) * convert `add_deref_target` to iterative implementation * use `ArrayVec` where we know the max number of elements * minor touchups here and there * avoid building temporary vectors that get appended to other vectors At most places I may or may not be doing the compiler's job is this PR. --- Cargo.lock | 1 + src/librustdoc/Cargo.toml | 1 + src/librustdoc/clean/inline.rs | 25 +- src/librustdoc/clean/types.rs | 28 +- src/librustdoc/clean/utils.rs | 18 +- src/librustdoc/config.rs | 2 +- src/librustdoc/core.rs | 58 +- src/librustdoc/formats/item_type.rs | 2 +- src/librustdoc/html/escape.rs | 31 +- src/librustdoc/html/highlight.rs | 46 +- src/librustdoc/html/highlight/tests.rs | 9 +- src/librustdoc/html/layout.rs | 26 +- src/librustdoc/html/markdown.rs | 29 +- src/librustdoc/html/render/mod.rs | 699 ++++++++++--------- src/librustdoc/html/sources.rs | 14 +- src/librustdoc/passes/collect_trait_impls.rs | 44 +- 16 files changed, 521 insertions(+), 512 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a4ba7704426e8..9149ffeded952 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4386,6 +4386,7 @@ dependencies = [ name = "rustdoc" version = "0.0.0" dependencies = [ + "arrayvec", "expect-test", "itertools 0.9.0", "minifier", diff --git a/src/librustdoc/Cargo.toml b/src/librustdoc/Cargo.toml index db64b31f31cfc..b6965898b4e44 100644 --- a/src/librustdoc/Cargo.toml +++ b/src/librustdoc/Cargo.toml @@ -8,6 +8,7 @@ edition = "2018" path = "lib.rs" [dependencies] +arrayvec = { version = "0.5.1", default-features = false } pulldown-cmark = { version = "0.8", default-features = false } minifier = "0.0.33" rayon = { version = "0.3.0", package = "rustc-rayon" } diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs index 1f9e7f8ae5cd4..2588c00f2cffd 100644 --- a/src/librustdoc/clean/inline.rs +++ b/src/librustdoc/clean/inline.rs @@ -56,7 +56,7 @@ crate fn try_inline( let kind = match res { Res::Def(DefKind::Trait, did) => { record_extern_fqn(cx, did, clean::TypeKind::Trait); - ret.extend(build_impls(cx, Some(parent_module), did, attrs)); + build_impls(cx, Some(parent_module), did, attrs, &mut ret); clean::TraitItem(build_external_trait(cx, did)) } Res::Def(DefKind::Fn, did) => { @@ -65,27 +65,27 @@ crate fn try_inline( } Res::Def(DefKind::Struct, did) => { record_extern_fqn(cx, did, clean::TypeKind::Struct); - ret.extend(build_impls(cx, Some(parent_module), did, attrs)); + build_impls(cx, Some(parent_module), did, attrs, &mut ret); clean::StructItem(build_struct(cx, did)) } Res::Def(DefKind::Union, did) => { record_extern_fqn(cx, did, clean::TypeKind::Union); - ret.extend(build_impls(cx, Some(parent_module), did, attrs)); + build_impls(cx, Some(parent_module), did, attrs, &mut ret); clean::UnionItem(build_union(cx, did)) } Res::Def(DefKind::TyAlias, did) => { record_extern_fqn(cx, did, clean::TypeKind::Typedef); - ret.extend(build_impls(cx, Some(parent_module), did, attrs)); + build_impls(cx, Some(parent_module), did, attrs, &mut ret); clean::TypedefItem(build_type_alias(cx, did), false) } Res::Def(DefKind::Enum, did) => { record_extern_fqn(cx, did, clean::TypeKind::Enum); - ret.extend(build_impls(cx, Some(parent_module), did, attrs)); + build_impls(cx, Some(parent_module), did, attrs, &mut ret); clean::EnumItem(build_enum(cx, did)) } Res::Def(DefKind::ForeignTy, did) => { record_extern_fqn(cx, did, clean::TypeKind::Foreign); - ret.extend(build_impls(cx, Some(parent_module), did, attrs)); + build_impls(cx, Some(parent_module), did, attrs, &mut ret); clean::ForeignTypeItem } // Never inline enum variants but leave them shown as re-exports. @@ -133,10 +133,7 @@ crate fn try_inline_glob( res: Res, visited: &mut FxHashSet, ) -> Option> { - if res == Res::Err { - return None; - } - let did = res.def_id(); + let did = res.opt_def_id()?; if did.is_local() { return None; } @@ -280,16 +277,14 @@ crate fn build_impls( parent_module: Option, did: DefId, attrs: Option>, -) -> Vec { + ret: &mut Vec, +) { let tcx = cx.tcx; - let mut impls = Vec::new(); // for each implementation of an item represented by `did`, build the clean::Item for that impl for &did in tcx.inherent_impls(did).iter() { - build_impl(cx, parent_module, did, attrs, &mut impls); + build_impl(cx, parent_module, did, attrs, ret); } - - impls } /// `parent_module` refers to the parent of the re-export, not the original item diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index bebb746c92d2f..18683f6809f44 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -8,6 +8,7 @@ use std::rc::Rc; use std::sync::Arc; use std::{slice, vec}; +use arrayvec::ArrayVec; use rustc_ast::attr; use rustc_ast::util::comments::beautify_doc_string; use rustc_ast::{self as ast, AttrStyle}; @@ -16,7 +17,7 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_feature::UnstableFeatures; use rustc_hir as hir; use rustc_hir::def::{CtorKind, Res}; -use rustc_hir::def_id::{CrateNum, DefId}; +use rustc_hir::def_id::{CrateNum, DefId, DefIndex}; use rustc_hir::lang_items::LangItem; use rustc_hir::Mutability; use rustc_index::vec::IndexVec; @@ -28,7 +29,6 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol, SymbolStr}; use rustc_span::{self, FileName, Loc}; use rustc_target::abi::VariantIdx; use rustc_target::spec::abi::Abi; -use smallvec::{smallvec, SmallVec}; use crate::clean::cfg::Cfg; use crate::clean::external_path; @@ -45,7 +45,7 @@ use self::ItemKind::*; use self::SelfTy::*; use self::Type::*; -thread_local!(crate static MAX_DEF_ID: RefCell> = Default::default()); +thread_local!(crate static MAX_DEF_IDX: RefCell> = Default::default()); #[derive(Clone, Debug)] crate struct Crate { @@ -293,8 +293,8 @@ impl Item { /// /// [`next_def_id()`]: DocContext::next_def_id() crate fn is_fake(&self) -> bool { - MAX_DEF_ID.with(|m| { - m.borrow().get(&self.def_id.krate).map(|id| self.def_id >= *id).unwrap_or(false) + MAX_DEF_IDX.with(|m| { + m.borrow().get(&self.def_id.krate).map(|&idx| idx <= self.def_id.index).unwrap_or(false) }) } } @@ -1539,12 +1539,12 @@ impl PrimitiveType { } } - crate fn impls(&self, tcx: TyCtxt<'_>) -> &'static SmallVec<[DefId; 4]> { + crate fn impls(&self, tcx: TyCtxt<'_>) -> &'static ArrayVec<[DefId; 4]> { Self::all_impls(tcx).get(self).expect("missing impl for primitive type") } - crate fn all_impls(tcx: TyCtxt<'_>) -> &'static FxHashMap> { - static CELL: OnceCell>> = OnceCell::new(); + crate fn all_impls(tcx: TyCtxt<'_>) -> &'static FxHashMap> { + static CELL: OnceCell>> = OnceCell::new(); CELL.get_or_init(move || { use self::PrimitiveType::*; @@ -1568,7 +1568,7 @@ impl PrimitiveType { } let single = |a: Option| a.into_iter().collect(); - let both = |a: Option, b: Option| -> SmallVec<_> { + let both = |a: Option, b: Option| -> ArrayVec<_> { a.into_iter().chain(b).collect() }; @@ -1601,8 +1601,8 @@ impl PrimitiveType { .collect() }, Array => single(lang_items.array_impl()), - Tuple => smallvec![], - Unit => smallvec![], + Tuple => ArrayVec::new(), + Unit => ArrayVec::new(), RawPointer => { lang_items .const_ptr_impl() @@ -1612,9 +1612,9 @@ impl PrimitiveType { .chain(lang_items.mut_slice_ptr_impl()) .collect() }, - Reference => smallvec![], - Fn => smallvec![], - Never => smallvec![], + Reference => ArrayVec::new(), + Fn => ArrayVec::new(), + Never => ArrayVec::new(), } }) } diff --git a/src/librustdoc/clean/utils.rs b/src/librustdoc/clean/utils.rs index 50ec6e69fbd40..ce8e0cf981c38 100644 --- a/src/librustdoc/clean/utils.rs +++ b/src/librustdoc/clean/utils.rs @@ -322,20 +322,14 @@ crate fn build_deref_target_impls(cx: &DocContext<'_>, items: &[Item], ret: &mut ItemKind::TypedefItem(ref t, true) => &t.type_, _ => continue, }; - let primitive = match *target { - ResolvedPath { did, .. } if did.is_local() => continue, - ResolvedPath { did, .. } => { - ret.extend(inline::build_impls(cx, None, did, None)); - continue; + + if let Some(prim) = target.primitive_type() { + for &did in prim.impls(tcx).iter().filter(|did| !did.is_local()) { + inline::build_impl(cx, None, did, None, ret); } - _ => match target.primitive_type() { - Some(prim) => prim, - None => continue, - }, - }; - for &did in primitive.impls(tcx) { + } else if let ResolvedPath { did, .. } = *target { if !did.is_local() { - inline::build_impl(cx, None, did, None, ret); + inline::build_impls(cx, None, did, None, ret); } } } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 94773ac77ccb0..95c6989dab4ee 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -474,7 +474,7 @@ impl Options { }; let mut id_map = html::markdown::IdMap::new(); - id_map.populate(html::render::initial_ids()); + id_map.populate(&html::render::INITIAL_IDS); let external_html = match ExternalHtml::load( &matches.opt_strs("html-in-header"), &matches.opt_strs("html-before-content"), diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 16f11e460e6f0..8eea102fa2fdb 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -24,12 +24,15 @@ use rustc_span::source_map; use rustc_span::symbol::sym; use rustc_span::DUMMY_SP; -use std::cell::{Cell, RefCell}; use std::mem; use std::rc::Rc; +use std::{ + cell::{Cell, RefCell}, + collections::hash_map::Entry, +}; use crate::clean; -use crate::clean::{AttributesExt, MAX_DEF_ID}; +use crate::clean::{AttributesExt, MAX_DEF_IDX}; use crate::config::{Options as RustdocOptions, RenderOptions}; use crate::config::{OutputFormat, RenderInfo}; use crate::formats::cache::Cache; @@ -63,8 +66,7 @@ crate struct DocContext<'tcx> { crate ct_substs: RefCell>, /// Table synthetic type parameter for `impl Trait` in argument position -> bounds crate impl_trait_bounds: RefCell>>, - crate fake_def_ids: RefCell>, - crate all_fake_def_ids: RefCell>, + crate fake_def_ids: RefCell>, /// Auto-trait or blanket impls processed so far, as `(self_ty, trait_def_id)`. // FIXME(eddyb) make this a `ty::TraitRef<'tcx>` set. crate generated_synthetics: RefCell, DefId)>>, @@ -138,37 +140,38 @@ impl<'tcx> DocContext<'tcx> { /// [`Debug`]: std::fmt::Debug /// [`clean::Item`]: crate::clean::types::Item crate fn next_def_id(&self, crate_num: CrateNum) -> DefId { - let start_def_id = { - let num_def_ids = if crate_num == LOCAL_CRATE { - self.tcx.hir().definitions().def_path_table().num_def_ids() - } else { - self.enter_resolver(|r| r.cstore().num_def_ids(crate_num)) - }; - - DefId { krate: crate_num, index: DefIndex::from_usize(num_def_ids) } - }; - let mut fake_ids = self.fake_def_ids.borrow_mut(); - let def_id = *fake_ids.entry(crate_num).or_insert(start_def_id); - fake_ids.insert( - crate_num, - DefId { krate: crate_num, index: DefIndex::from(def_id.index.index() + 1) }, - ); - - MAX_DEF_ID.with(|m| { - m.borrow_mut().entry(def_id.krate).or_insert(start_def_id); - }); - - self.all_fake_def_ids.borrow_mut().insert(def_id); + let def_index = match fake_ids.entry(crate_num) { + Entry::Vacant(e) => { + let num_def_idx = { + let num_def_idx = if crate_num == LOCAL_CRATE { + self.tcx.hir().definitions().def_path_table().num_def_ids() + } else { + self.enter_resolver(|r| r.cstore().num_def_ids(crate_num)) + }; + + DefIndex::from_usize(num_def_idx) + }; + + MAX_DEF_IDX.with(|m| { + m.borrow_mut().insert(crate_num, num_def_idx); + }); + e.insert(num_def_idx) + } + Entry::Occupied(e) => e.into_mut(), + }; + *def_index = DefIndex::from(*def_index + 1); - def_id + DefId { krate: crate_num, index: *def_index } } /// Like `hir().local_def_id_to_hir_id()`, but skips calling it on fake DefIds. /// (This avoids a slice-index-out-of-bounds panic.) crate fn as_local_hir_id(&self, def_id: DefId) -> Option { - if self.all_fake_def_ids.borrow().contains(&def_id) { + if MAX_DEF_IDX.with(|m| { + m.borrow().get(&def_id.krate).map(|&idx| idx <= def_id.index).unwrap_or(false) + }) { None } else { def_id.as_local().map(|def_id| self.tcx.hir().local_def_id_to_hir_id(def_id)) @@ -517,7 +520,6 @@ crate fn run_global_ctxt( ct_substs: Default::default(), impl_trait_bounds: Default::default(), fake_def_ids: Default::default(), - all_fake_def_ids: Default::default(), generated_synthetics: Default::default(), auto_traits: tcx .all_traits(LOCAL_CRATE) diff --git a/src/librustdoc/formats/item_type.rs b/src/librustdoc/formats/item_type.rs index ad51adf2fe3f5..f904de22f6ba4 100644 --- a/src/librustdoc/formats/item_type.rs +++ b/src/librustdoc/formats/item_type.rs @@ -158,6 +158,6 @@ impl ItemType { impl fmt::Display for ItemType { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.as_str()) + f.write_str(self.as_str()) } } diff --git a/src/librustdoc/html/escape.rs b/src/librustdoc/html/escape.rs index 60c19551983cd..ce44722a532b0 100644 --- a/src/librustdoc/html/escape.rs +++ b/src/librustdoc/html/escape.rs @@ -16,23 +16,20 @@ impl<'a> fmt::Display for Escape<'a> { let Escape(s) = *self; let pile_o_bits = s; let mut last = 0; - for (i, ch) in s.bytes().enumerate() { - match ch as char { - '<' | '>' | '&' | '\'' | '"' => { - fmt.write_str(&pile_o_bits[last..i])?; - let s = match ch as char { - '>' => ">", - '<' => "<", - '&' => "&", - '\'' => "'", - '"' => """, - _ => unreachable!(), - }; - fmt.write_str(s)?; - last = i + 1; - } - _ => {} - } + for (i, ch) in s.char_indices() { + let s = match ch { + '>' => ">", + '<' => "<", + '&' => "&", + '\'' => "'", + '"' => """, + _ => continue, + }; + fmt.write_str(&pile_o_bits[last..i])?; + fmt.write_str(s)?; + // NOTE: we only expect single byte characters here - which is fine as long as we + // only match single byte characters + last = i + 1; } if last < s.len() { diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index d21998bb8cfe5..7e50d72e60f7d 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -7,7 +7,7 @@ use crate::html::escape::Escape; -use std::fmt::{Display, Write}; +use std::fmt::Display; use std::iter::Peekable; use rustc_lexer::{LiteralKind, TokenKind}; @@ -15,16 +15,18 @@ use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; use rustc_span::with_default_session_globals; +use super::format::Buffer; + /// Highlights `src`, returning the HTML output. crate fn render_with_highlighting( - src: String, + src: &str, + out: &mut Buffer, class: Option<&str>, playground_button: Option<&str>, tooltip: Option<(Option, &str)>, edition: Edition, -) -> String { +) { debug!("highlighting: ================\n{}\n==============", src); - let mut out = String::with_capacity(src.len()); if let Some((edition_info, class)) = tooltip { write!( out, @@ -35,23 +37,19 @@ crate fn render_with_highlighting( } else { String::new() }, - ) - .unwrap(); + ); } - write_header(&mut out, class); - write_code(&mut out, &src, edition); - write_footer(&mut out, playground_button); - - out + write_header(out, class); + write_code(out, &src, edition); + write_footer(out, playground_button); } -fn write_header(out: &mut String, class: Option<&str>) { - write!(out, "
\n", class.unwrap_or_default())
-        .unwrap()
+fn write_header(out: &mut Buffer, class: Option<&str>) {
+    write!(out, "
\n", class.unwrap_or_default());
 }
 
-fn write_code(out: &mut String, src: &str, edition: Edition) {
+fn write_code(out: &mut Buffer, src: &str, edition: Edition) {
     // This replace allows to fix how the code source with DOS backline characters is displayed.
     let src = src.replace("\r\n", "\n");
     Classifier::new(&src, edition).highlight(&mut |highlight| {
@@ -63,8 +61,8 @@ fn write_code(out: &mut String, src: &str, edition: Edition) {
     });
 }
 
-fn write_footer(out: &mut String, playground_button: Option<&str>) {
-    write!(out, "
{}
\n", playground_button.unwrap_or_default()).unwrap()
+fn write_footer(out: &mut Buffer, playground_button: Option<&str>) {
+    write!(out, "
{}
\n", playground_button.unwrap_or_default()); } /// How a span of text is classified. Mostly corresponds to token kinds. @@ -331,13 +329,13 @@ impl<'a> Classifier<'a> { /// Called when we start processing a span of text that should be highlighted. /// The `Class` argument specifies how it should be highlighted. -fn enter_span(out: &mut String, klass: Class) { - write!(out, "", klass.as_html()).unwrap() +fn enter_span(out: &mut Buffer, klass: Class) { + write!(out, "", klass.as_html()); } /// Called at the end of a span of highlighted text. -fn exit_span(out: &mut String) { - write!(out, "").unwrap() +fn exit_span(out: &mut Buffer) { + out.write_str(""); } /// Called for a span of text. If the text should be highlighted differently @@ -351,10 +349,10 @@ fn exit_span(out: &mut String) { /// ``` /// The latter can be thought of as a shorthand for the former, which is more /// flexible. -fn string(out: &mut String, text: T, klass: Option) { +fn string(out: &mut Buffer, text: T, klass: Option) { match klass { - None => write!(out, "{}", text).unwrap(), - Some(klass) => write!(out, "{}", klass.as_html(), text).unwrap(), + None => write!(out, "{}", text), + Some(klass) => write!(out, "{}", klass.as_html(), text), } } diff --git a/src/librustdoc/html/highlight/tests.rs b/src/librustdoc/html/highlight/tests.rs index f97c8a7ab7148..305cf61091dc6 100644 --- a/src/librustdoc/html/highlight/tests.rs +++ b/src/librustdoc/html/highlight/tests.rs @@ -1,4 +1,5 @@ use super::write_code; +use crate::html::format::Buffer; use expect_test::expect_file; use rustc_span::edition::Edition; @@ -18,9 +19,9 @@ const STYLE: &str = r#" fn test_html_highlighting() { let src = include_str!("fixtures/sample.rs"); let html = { - let mut out = String::new(); + let mut out = Buffer::new(); write_code(&mut out, src, Edition::Edition2018); - format!("{}
{}
\n", STYLE, out) + format!("{}
{}
\n", STYLE, out.into_inner()) }; expect_file!["fixtures/sample.html"].assert_eq(&html); } @@ -30,7 +31,7 @@ fn test_dos_backline() { let src = "pub fn foo() {\r\n\ println!(\"foo\");\r\n\ }\r\n"; - let mut html = String::new(); + let mut html = Buffer::new(); write_code(&mut html, src, Edition::Edition2018); - expect_file!["fixtures/dos_line.html"].assert_eq(&html); + expect_file!["fixtures/dos_line.html"].assert_eq(&html.into_inner()); } diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs index c6ff4b57a6e59..e5a686bd07d07 100644 --- a/src/librustdoc/html/layout.rs +++ b/src/librustdoc/html/layout.rs @@ -114,7 +114,6 @@ crate fn render( {after_content}\
\ - {static_extra_scripts}\ {extra_scripts}\ \ \ @@ -135,22 +134,23 @@ crate fn render( root_path = page.root_path, css_class = page.css_class, logo = { - let p = format!("{}{}", page.root_path, layout.krate); - let p = ensure_trailing_slash(&p); if layout.logo.is_empty() { format!( - "\ + "\ ", - path = p, + root = page.root_path, + path = ensure_trailing_slash(&layout.krate), static_root_path = static_root_path, suffix = page.resource_suffix ) } else { format!( - "\ -
logo
", - p, layout.logo + "\ +
logo
", + root = page.root_path, + path = ensure_trailing_slash(&layout.krate), + logo = layout.logo ) } }, @@ -194,7 +194,7 @@ crate fn render( )) .collect::(), suffix = page.resource_suffix, - static_extra_scripts = page + extra_scripts = page .static_extra_scripts .iter() .map(|e| { @@ -204,17 +204,13 @@ crate fn render( extra_script = e ) }) - .collect::(), - extra_scripts = page - .extra_scripts - .iter() - .map(|e| { + .chain(page.extra_scripts.iter().map(|e| { format!( "", root_path = page.root_path, extra_script = e ) - }) + })) .collect::(), filter_crates = if layout.generate_search_filter { "