Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rustdoc: Use ThinVec in a few places #83828

Merged
merged 1 commit into from
Apr 7, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ 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};
use rustc_attr::{ConstStability, Deprecation, Stability, StabilityLevel};
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::thin_vec::ThinVec;
use rustc_feature::UnstableFeatures;
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, Res};
Expand Down Expand Up @@ -54,7 +56,7 @@ crate struct Crate {
crate src: FileName,
crate module: Item,
crate externs: Vec<(CrateNum, ExternalCrate)>,
crate primitives: Vec<(DefId, PrimitiveType)>,
crate primitives: ThinVec<(DefId, PrimitiveType)>,
// These are later on moved into `CACHEKEY`, leaving the map empty.
// Only here so that they can be filtered through the rustdoc passes.
crate external_traits: Rc<RefCell<FxHashMap<DefId, TraitWithExtraInfo>>>,
Expand All @@ -73,8 +75,8 @@ crate struct ExternalCrate {
crate name: Symbol,
crate src: FileName,
crate attrs: Attributes,
crate primitives: Vec<(DefId, PrimitiveType)>,
crate keywords: Vec<(DefId, Symbol)>,
crate primitives: ThinVec<(DefId, PrimitiveType)>,
crate keywords: ThinVec<(DefId, Symbol)>,
Comment on lines +78 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make it thinner by combining this two into one pointer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? These are stored in the ThinVec, on the heap, not on the stack.

Copy link
Contributor

@pickfire pickfire Apr 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but since both of them are as rare, so we can just combine both into one pointer rather than two right? But yeah, if one is needed then we need the pointer already.

Only useful if both are always together at the same time.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible that combining them into one boxed field would make it smaller, but

  1. that would require code changes, and the slight perf improvement is likely not worth it;
  2. there probably won't be much of a perf improvement because padding may take up the space anyway.

}

/// Anything with a source location and set of attributes and, optionally, a
Expand Down