-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add CGU size heuristic for partitioning #47415
Changes from 4 commits
8447f4f
e60b0f8
c8e9da4
5c9a8b5
62703cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
|
||
use syntax::ast::NodeId; | ||
use syntax::symbol::InternedString; | ||
use ty::Instance; | ||
use ty::{Instance, TyCtxt}; | ||
use util::nodemap::FxHashMap; | ||
use rustc_data_structures::base_n; | ||
use rustc_data_structures::stable_hasher::{HashStable, StableHasherResult, | ||
|
@@ -25,6 +25,21 @@ pub enum MonoItem<'tcx> { | |
GlobalAsm(NodeId), | ||
} | ||
|
||
impl<'tcx> MonoItem<'tcx> { | ||
pub fn size_estimate<'a>(&self, tcx: &TyCtxt<'a, 'tcx, 'tcx>) -> usize { | ||
match *self { | ||
MonoItem::Fn(instance) => { | ||
// Estimate the size of a function based on how many statements | ||
// it contains. | ||
tcx.instance_def_size_estimate(instance.def) | ||
}, | ||
// Conservatively estimate the size of a static declaration | ||
// or assembly to be 1. | ||
MonoItem::Static(_) | MonoItem::GlobalAsm(_) => 1, | ||
} | ||
} | ||
} | ||
|
||
impl<'tcx> HashStable<StableHashingContext<'tcx>> for MonoItem<'tcx> { | ||
fn hash_stable<W: StableHasherResult>(&self, | ||
hcx: &mut StableHashingContext<'tcx>, | ||
|
@@ -52,6 +67,7 @@ pub struct CodegenUnit<'tcx> { | |
/// as well as the crate name and disambiguator. | ||
name: InternedString, | ||
items: FxHashMap<MonoItem<'tcx>, (Linkage, Visibility)>, | ||
size_estimate: Option<usize>, | ||
} | ||
|
||
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] | ||
|
@@ -101,6 +117,7 @@ impl<'tcx> CodegenUnit<'tcx> { | |
CodegenUnit { | ||
name: name, | ||
items: FxHashMap(), | ||
size_estimate: None, | ||
} | ||
} | ||
|
||
|
@@ -131,6 +148,25 @@ impl<'tcx> CodegenUnit<'tcx> { | |
let hash = hash & ((1u128 << 80) - 1); | ||
base_n::encode(hash, base_n::CASE_INSENSITIVE) | ||
} | ||
|
||
pub fn estimate_size<'a>(&mut self, tcx: &TyCtxt<'a, 'tcx, 'tcx>) { | ||
// Estimate the size of a codegen unit as (approximately) the number of MIR | ||
// statements it corresponds to. | ||
self.size_estimate = Some(self.items.keys().map(|mi| mi.size_estimate(tcx)).sum()); | ||
} | ||
|
||
pub fn size_estimate(&self) -> usize { | ||
// Should only be called if `estimate_size` has previously been called. | ||
assert!(self.size_estimate.is_some()); | ||
self.size_estimate.unwrap() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could replace this combination of |
||
} | ||
|
||
pub fn modify_size_estimate(&mut self, delta: usize) { | ||
assert!(self.size_estimate.is_some()); | ||
if let Some(size_estimate) = self.size_estimate { | ||
self.size_estimate = Some(size_estimate + delta); | ||
} | ||
} | ||
} | ||
|
||
impl<'tcx> HashStable<StableHashingContext<'tcx>> for CodegenUnit<'tcx> { | ||
|
@@ -140,6 +176,7 @@ impl<'tcx> HashStable<StableHashingContext<'tcx>> for CodegenUnit<'tcx> { | |
let CodegenUnit { | ||
ref items, | ||
name, | ||
.. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We keep these destructuring let CodegenUnit {
ref items,
name,
// The size estimate is not relevant to the hash
size_estimate: _,
} = *self; Making these exhaustive has proven to be effective for preventing errors introduced by unrelated refactorings. |
||
} = *self; | ||
|
||
name.hash_stable(hcx, hasher); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -761,6 +761,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, | |
DepKind::EraseRegionsTy | | ||
DepKind::NormalizeTy | | ||
DepKind::SubstituteNormalizeAndTestPredicates | | ||
DepKind::InstanceDefSizeEstimate | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the correct location? Or should it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes that's the correct location. Very good!
|
||
|
||
// This one should never occur in this context | ||
DepKind::Null => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2669,6 +2669,19 @@ fn crate_hash<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, | |
tcx.hir.crate_hash | ||
} | ||
|
||
fn instance_def_size_estimate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, | ||
instance_def: InstanceDef<'tcx>) | ||
-> usize { | ||
match instance_def { | ||
InstanceDef::Item(def_id) => { | ||
let mir = tcx.optimized_mir(def_id); | ||
mir.basic_blocks().iter().map(|bb| bb.statements.len()).sum() | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should estimate the size of |
||
// Estimate the size of compiler-generated shims to be 1. | ||
_ => 1 | ||
} | ||
} | ||
|
||
pub fn provide(providers: &mut ty::maps::Providers) { | ||
context::provide(providers); | ||
erase_regions::provide(providers); | ||
|
@@ -2686,6 +2699,7 @@ pub fn provide(providers: &mut ty::maps::Providers) { | |
original_crate_name, | ||
crate_hash, | ||
trait_impls_of: trait_def::trait_impls_of_provider, | ||
instance_def_size_estimate, | ||
..*providers | ||
}; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is
[]
correct here? I wasn't sure what this meant.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's correct. Leaving the brackets empty means that this is a regular query/dep-node.