Skip to content

Commit 47015c2

Browse files
borsbors
committed
Auto merge of #46490 - michaelwoerister:hash-spans-wip, r=<try>
EXPERIMENTAL: Hash spans unconditionally during incr. comp. It would be good for reliability if we could do without all this special-casing around change detection for spans. This PR removes most extra logic around span hashing and is meant to gauge how much of a performance regression this would cause. Much of the potential performance loss should be recoverable by better factoring queries -- but that's a medium term undertaking and should not block incr. comp. stabilization. cc @nikomatsakis @Mark-Simulacrum
2 parents 226656f + 4c448b8 commit 47015c2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+259
-647
lines changed

‎src/librustc/hir/map/collector.rs

+19-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use super::*;
1212

1313
use dep_graph::{DepGraph, DepKind, DepNodeIndex};
1414
use hir::intravisit::{Visitor, NestedVisitorMap};
15+
use middle::cstore::CrateStore;
1516
use session::CrateDisambiguator;
1617
use std::iter::repeat;
1718
use syntax::ast::{NodeId, CRATE_NODE_ID};
@@ -119,7 +120,9 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
119120
}
120121

121122
pub(super) fn finalize_and_compute_crate_hash(self,
122-
crate_disambiguator: CrateDisambiguator)
123+
crate_disambiguator: CrateDisambiguator,
124+
cstore: &CrateStore,
125+
commandline_args_hash: u64)
123126
-> Vec<MapEntry<'hir>> {
124127
let mut node_hashes: Vec<_> = self
125128
.hir_body_nodes
@@ -132,9 +135,23 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
132135

133136
node_hashes.sort_unstable_by(|&(ref d1, _), &(ref d2, _)| d1.cmp(d2));
134137

138+
let mut upstream_crates: Vec<_> = cstore.crates_untracked().iter().map(|&cnum| {
139+
let name = cstore.crate_name_untracked(cnum).as_str();
140+
let disambiguator = cstore.crate_disambiguator_untracked(cnum)
141+
.to_fingerprint();
142+
let hash = cstore.crate_hash_untracked(cnum);
143+
(name, disambiguator, hash)
144+
}).collect();
145+
146+
upstream_crates.sort_unstable_by(|&(name1, dis1, _), &(name2, dis2, _)| {
147+
(name1, dis1).cmp(&(name2, dis2))
148+
});
149+
135150
self.dep_graph.with_task(DepNode::new_no_params(DepKind::Krate),
136151
&self.hcx,
137-
(node_hashes, crate_disambiguator.to_fingerprint()),
152+
((node_hashes, upstream_crates),
153+
(commandline_args_hash,
154+
crate_disambiguator.to_fingerprint())),
138155
identity_fn);
139156
self.map
140157
}

‎src/librustc/hir/map/mod.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -1059,7 +1059,10 @@ pub fn map_crate<'hir>(sess: &::session::Session,
10591059
intravisit::walk_crate(&mut collector, &forest.krate);
10601060

10611061
let crate_disambiguator = sess.local_crate_disambiguator();
1062-
collector.finalize_and_compute_crate_hash(crate_disambiguator)
1062+
let cmdline_args = sess.opts.dep_tracking_hash();
1063+
collector.finalize_and_compute_crate_hash(crate_disambiguator,
1064+
cstore,
1065+
cmdline_args)
10631066
};
10641067

10651068
if log_enabled!(::log::LogLevel::Debug) {

‎src/librustc/ich/hcx.rs

+3-62
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use hir::map::DefPathHash;
1414
use hir::map::definitions::Definitions;
1515
use ich::{self, CachingCodemapView};
1616
use middle::cstore::CrateStore;
17-
use session::config::DebugInfoLevel::NoDebugInfo;
1817
use ty::{TyCtxt, fast_reject};
1918
use session::Session;
2019

@@ -24,7 +23,7 @@ use std::cell::RefCell;
2423
use std::collections::HashMap;
2524

2625
use syntax::ast;
27-
use syntax::attr;
26+
2827
use syntax::codemap::CodeMap;
2928
use syntax::ext::hygiene::SyntaxContext;
3029
use syntax::symbol::Symbol;
@@ -51,7 +50,6 @@ pub struct StableHashingContext<'gcx> {
5150
body_resolver: BodyResolver<'gcx>,
5251
hash_spans: bool,
5352
hash_bodies: bool,
54-
overflow_checks_enabled: bool,
5553
node_id_hashing_mode: NodeIdHashingMode,
5654

5755
// Very often, we are hashing something that does not need the
@@ -89,8 +87,7 @@ impl<'gcx> StableHashingContext<'gcx> {
8987
definitions: &'gcx Definitions,
9088
cstore: &'gcx CrateStore)
9189
-> Self {
92-
let hash_spans_initial = sess.opts.debuginfo != NoDebugInfo;
93-
let check_overflow_initial = sess.overflow_checks();
90+
let hash_spans_initial = !sess.opts.debugging_opts.incremental_ignore_spans;
9491

9592
debug_assert!(ich::IGNORED_ATTRIBUTES.len() > 0);
9693
IGNORED_ATTR_NAMES.with(|names| {
@@ -110,7 +107,6 @@ impl<'gcx> StableHashingContext<'gcx> {
110107
raw_codemap: sess.codemap(),
111108
hash_spans: hash_spans_initial,
112109
hash_bodies: true,
113-
overflow_checks_enabled: check_overflow_initial,
114110
node_id_hashing_mode: NodeIdHashingMode::HashDefPath,
115111
}
116112
}
@@ -120,11 +116,6 @@ impl<'gcx> StableHashingContext<'gcx> {
120116
self.sess
121117
}
122118

123-
pub fn force_span_hashing(mut self) -> Self {
124-
self.hash_spans = true;
125-
self
126-
}
127-
128119
#[inline]
129120
pub fn while_hashing_hir_bodies<F: FnOnce(&mut Self)>(&mut self,
130121
hash_bodies: bool,
@@ -174,11 +165,6 @@ impl<'gcx> StableHashingContext<'gcx> {
174165
self.definitions.node_to_hir_id(node_id)
175166
}
176167

177-
#[inline]
178-
pub fn hash_spans(&self) -> bool {
179-
self.hash_spans
180-
}
181-
182168
#[inline]
183169
pub fn hash_bodies(&self) -> bool {
184170
self.hash_bodies
@@ -204,58 +190,13 @@ impl<'gcx> StableHashingContext<'gcx> {
204190
})
205191
}
206192

207-
pub fn hash_hir_item_like<F: FnOnce(&mut Self)>(&mut self,
208-
item_attrs: &[ast::Attribute],
209-
is_const: bool,
210-
f: F) {
211-
let prev_overflow_checks = self.overflow_checks_enabled;
212-
if is_const || attr::contains_name(item_attrs, "rustc_inherit_overflow_checks") {
213-
self.overflow_checks_enabled = true;
214-
}
193+
pub fn hash_hir_item_like<F: FnOnce(&mut Self)>(&mut self, f: F) {
215194
let prev_hash_node_ids = self.node_id_hashing_mode;
216195
self.node_id_hashing_mode = NodeIdHashingMode::Ignore;
217196

218197
f(self);
219198

220199
self.node_id_hashing_mode = prev_hash_node_ids;
221-
self.overflow_checks_enabled = prev_overflow_checks;
222-
}
223-
224-
#[inline]
225-
pub fn binop_can_panic_at_runtime(&self, binop: hir::BinOp_) -> bool
226-
{
227-
match binop {
228-
hir::BiAdd |
229-
hir::BiSub |
230-
hir::BiShl |
231-
hir::BiShr |
232-
hir::BiMul => self.overflow_checks_enabled,
233-
234-
hir::BiDiv |
235-
hir::BiRem => true,
236-
237-
hir::BiAnd |
238-
hir::BiOr |
239-
hir::BiBitXor |
240-
hir::BiBitAnd |
241-
hir::BiBitOr |
242-
hir::BiEq |
243-
hir::BiLt |
244-
hir::BiLe |
245-
hir::BiNe |
246-
hir::BiGe |
247-
hir::BiGt => false
248-
}
249-
}
250-
251-
#[inline]
252-
pub fn unop_can_panic_at_runtime(&self, unop: hir::UnOp) -> bool
253-
{
254-
match unop {
255-
hir::UnDeref |
256-
hir::UnNot => false,
257-
hir::UnNeg => self.overflow_checks_enabled,
258-
}
259200
}
260201
}
261202

‎src/librustc/ich/impls_hir.rs

+6-100
Original file line numberDiff line numberDiff line change
@@ -529,63 +529,9 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Expr {
529529
ref attrs
530530
} = *self;
531531

532-
let spans_always_on = match *node {
533-
hir::ExprBox(..) |
534-
hir::ExprArray(..) |
535-
hir::ExprCall(..) |
536-
hir::ExprLit(..) |
537-
hir::ExprCast(..) |
538-
hir::ExprType(..) |
539-
hir::ExprIf(..) |
540-
hir::ExprWhile(..) |
541-
hir::ExprLoop(..) |
542-
hir::ExprMatch(..) |
543-
hir::ExprClosure(..) |
544-
hir::ExprBlock(..) |
545-
hir::ExprAssign(..) |
546-
hir::ExprTupField(..) |
547-
hir::ExprAddrOf(..) |
548-
hir::ExprBreak(..) |
549-
hir::ExprAgain(..) |
550-
hir::ExprRet(..) |
551-
hir::ExprYield(..) |
552-
hir::ExprInlineAsm(..) |
553-
hir::ExprRepeat(..) |
554-
hir::ExprTup(..) |
555-
hir::ExprMethodCall(..) |
556-
hir::ExprPath(..) |
557-
hir::ExprStruct(..) |
558-
hir::ExprField(..) => {
559-
// For these we only hash the span when debuginfo is on.
560-
false
561-
}
562-
// For the following, spans might be significant because of
563-
// panic messages indicating the source location.
564-
hir::ExprBinary(op, ..) => {
565-
hcx.binop_can_panic_at_runtime(op.node)
566-
}
567-
hir::ExprUnary(op, _) => {
568-
hcx.unop_can_panic_at_runtime(op)
569-
}
570-
hir::ExprAssignOp(op, ..) => {
571-
hcx.binop_can_panic_at_runtime(op.node)
572-
}
573-
hir::ExprIndex(..) => {
574-
true
575-
}
576-
};
577-
578-
if spans_always_on {
579-
hcx.while_hashing_spans(true, |hcx| {
580-
span.hash_stable(hcx, hasher);
581-
node.hash_stable(hcx, hasher);
582-
attrs.hash_stable(hcx, hasher);
583-
});
584-
} else {
585-
span.hash_stable(hcx, hasher);
586-
node.hash_stable(hcx, hasher);
587-
attrs.hash_stable(hcx, hasher);
588-
}
532+
span.hash_stable(hcx, hasher);
533+
node.hash_stable(hcx, hasher);
534+
attrs.hash_stable(hcx, hasher);
589535
})
590536
}
591537
}
@@ -712,15 +658,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::TraitItem {
712658
span
713659
} = *self;
714660

715-
let is_const = match *node {
716-
hir::TraitItemKind::Const(..) |
717-
hir::TraitItemKind::Type(..) => true,
718-
hir::TraitItemKind::Method(hir::MethodSig { constness, .. }, _) => {
719-
constness == hir::Constness::Const
720-
}
721-
};
722-
723-
hcx.hash_hir_item_like(attrs, is_const, |hcx| {
661+
hcx.hash_hir_item_like(|hcx| {
724662
name.hash_stable(hcx, hasher);
725663
attrs.hash_stable(hcx, hasher);
726664
generics.hash_stable(hcx, hasher);
@@ -757,15 +695,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::ImplItem {
757695
span
758696
} = *self;
759697

760-
let is_const = match *node {
761-
hir::ImplItemKind::Const(..) |
762-
hir::ImplItemKind::Type(..) => true,
763-
hir::ImplItemKind::Method(hir::MethodSig { constness, .. }, _) => {
764-
constness == hir::Constness::Const
765-
}
766-
};
767-
768-
hcx.hash_hir_item_like(attrs, is_const, |hcx| {
698+
hcx.hash_hir_item_like(|hcx| {
769699
name.hash_stable(hcx, hasher);
770700
vis.hash_stable(hcx, hasher);
771701
defaultness.hash_stable(hcx, hasher);
@@ -884,30 +814,6 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Item {
884814
fn hash_stable<W: StableHasherResult>(&self,
885815
hcx: &mut StableHashingContext<'gcx>,
886816
hasher: &mut StableHasher<W>) {
887-
let is_const = match self.node {
888-
hir::ItemStatic(..) |
889-
hir::ItemConst(..) => {
890-
true
891-
}
892-
hir::ItemFn(_, _, constness, ..) => {
893-
constness == hir::Constness::Const
894-
}
895-
hir::ItemUse(..) |
896-
hir::ItemExternCrate(..) |
897-
hir::ItemForeignMod(..) |
898-
hir::ItemGlobalAsm(..) |
899-
hir::ItemMod(..) |
900-
hir::ItemAutoImpl(..) |
901-
hir::ItemTrait(..) |
902-
hir::ItemImpl(..) |
903-
hir::ItemTy(..) |
904-
hir::ItemEnum(..) |
905-
hir::ItemStruct(..) |
906-
hir::ItemUnion(..) => {
907-
false
908-
}
909-
};
910-
911817
let hir::Item {
912818
name,
913819
ref attrs,
@@ -918,7 +824,7 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for hir::Item {
918824
span
919825
} = *self;
920826

921-
hcx.hash_hir_item_like(attrs, is_const, |hcx| {
827+
hcx.hash_hir_item_like(|hcx| {
922828
name.hash_stable(hcx, hasher);
923829
attrs.hash_stable(hcx, hasher);
924830
node.hash_stable(hcx, hasher);

‎src/librustc/ich/impls_mir.rs

+4-41
Original file line numberDiff line numberDiff line change
@@ -55,48 +55,11 @@ for mir::UnsafetyViolationKind {
5555
}
5656
}
5757
}
58-
impl<'gcx> HashStable<StableHashingContext<'gcx>>
59-
for mir::Terminator<'gcx> {
60-
#[inline]
61-
fn hash_stable<W: StableHasherResult>(&self,
62-
hcx: &mut StableHashingContext<'gcx>,
63-
hasher: &mut StableHasher<W>) {
64-
let mir::Terminator {
65-
ref kind,
66-
ref source_info,
67-
} = *self;
6858

69-
let hash_spans_unconditionally = match *kind {
70-
mir::TerminatorKind::Assert { .. } => {
71-
// Assert terminators generate a panic message that contains the
72-
// source location, so we always have to feed its span into the
73-
// ICH.
74-
true
75-
}
76-
mir::TerminatorKind::Goto { .. } |
77-
mir::TerminatorKind::SwitchInt { .. } |
78-
mir::TerminatorKind::Resume |
79-
mir::TerminatorKind::Return |
80-
mir::TerminatorKind::GeneratorDrop |
81-
mir::TerminatorKind::Unreachable |
82-
mir::TerminatorKind::Drop { .. } |
83-
mir::TerminatorKind::DropAndReplace { .. } |
84-
mir::TerminatorKind::Yield { .. } |
85-
mir::TerminatorKind::Call { .. } |
86-
mir::TerminatorKind::FalseEdges { .. } => false,
87-
};
88-
89-
if hash_spans_unconditionally {
90-
hcx.while_hashing_spans(true, |hcx| {
91-
source_info.hash_stable(hcx, hasher);
92-
})
93-
} else {
94-
source_info.hash_stable(hcx, hasher);
95-
}
96-
97-
kind.hash_stable(hcx, hasher);
98-
}
99-
}
59+
impl_stable_hash_for!(struct mir::Terminator<'tcx> {
60+
kind,
61+
source_info
62+
});
10063

10164
impl<'gcx, T> HashStable<StableHashingContext<'gcx>> for mir::ClearCrossCrate<T>
10265
where T: HashStable<StableHashingContext<'gcx>>

‎src/librustc/session/config.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1084,6 +1084,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
10841084
"dump hash information in textual format to stdout"),
10851085
incremental_verify_ich: bool = (false, parse_bool, [UNTRACKED],
10861086
"verify incr. comp. hashes of green query instances"),
1087+
incremental_ignore_spans: bool = (false, parse_bool, [UNTRACKED],
1088+
"ignore spans during ICH computation -- used for testing"),
10871089
dump_dep_graph: bool = (false, parse_bool, [UNTRACKED],
10881090
"dump the dependency graph to $RUST_DEP_GRAPH (default: /tmp/dep_graph.gv)"),
10891091
query_dep_graph: bool = (false, parse_bool, [UNTRACKED],

0 commit comments

Comments
 (0)