Skip to content

Commit

Permalink
Auto merge of #113562 - saethlin:larger-incr-comp-offset, r=nnethercote
Browse files Browse the repository at this point in the history
Use u64 for incr comp allocation offsets

Fixes #76037
Fixes #95780
Fixes #111613

These issues are all reporting ICEs caused by using `u32` to store offsets to allocations in the incremental compilation cache. This PR aims to lift that limitation by changing the offset type in question to `u64`.

There are two perf runs in this PR. The first reports a regression, and the second does not. The changes are the same in both. I rebased the PR then did the second perf run because I noticed that the primary regression in it was very commonly seen in spurious regression reports.

I do not know what the perf run will report when this is merged. I would not be surprised to see regression or neutral, but the cachegrind diffs for the regression point at `try_mark_previous_green` which is a common source of inexplicable regressions and I don't think should be perturbed by this PR.

I'm not opposed to adding a regression test such as
```rust
fn main() {
    println!("{}", [37; 1 << 30].len());
}
```
But that program takes 1 minute to compile and consumes 4.6 GB of memory then writes that much to disk. Is that a concerning amount of resource use for a test?

r? `@nnethercote`
  • Loading branch information
bors committed Jul 17, 2023
2 parents 299179e + 4e117a9 commit 6f65ef5
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 6 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
trace!("encoding {} further alloc ids", new_n - n);
for idx in n..new_n {
let id = self.interpret_allocs[idx];
let pos = self.position() as u32;
let pos = self.position() as u64;
interpret_alloc_index.push(pos);
interpret::specialized_encode_alloc_id(self, tcx, id);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ pub(crate) struct CrateRoot {
traits: LazyArray<DefIndex>,
impls: LazyArray<TraitImpls>,
incoherent_impls: LazyArray<IncoherentImpls>,
interpret_alloc_index: LazyArray<u32>,
interpret_alloc_index: LazyArray<u64>,
proc_macro_data: Option<ProcMacroData>,

tables: LazyTables,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/mir/interpret/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ pub struct AllocDecodingState {
// For each `AllocId`, we keep track of which decoding state it's currently in.
decoding_state: Vec<Lock<State>>,
// The offsets of each allocation in the data stream.
data_offsets: Vec<u32>,
data_offsets: Vec<u64>,
}

impl AllocDecodingState {
Expand All @@ -289,7 +289,7 @@ impl AllocDecodingState {
AllocDecodingSession { state: self, session_id }
}

pub fn new(data_offsets: Vec<u32>) -> Self {
pub fn new(data_offsets: Vec<u64>) -> Self {
let decoding_state =
std::iter::repeat_with(|| Lock::new(State::Empty)).take(data_offsets.len()).collect();

Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_middle/src/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ struct Footer {
query_result_index: EncodedDepNodeIndex,
side_effects_index: EncodedDepNodeIndex,
// The location of all allocations.
interpret_alloc_index: Vec<u32>,
// Most uses only need values up to u32::MAX, but benchmarking indicates that we can use a u64
// without measurable overhead. This permits larger const allocations without ICEing.
interpret_alloc_index: Vec<u64>,
// See `OnDiskCache.syntax_contexts`
syntax_contexts: FxHashMap<u32, AbsoluteBytePos>,
// See `OnDiskCache.expn_data`
Expand Down Expand Up @@ -301,7 +303,7 @@ impl<'sess> OnDiskCache<'sess> {
interpret_alloc_index.reserve(new_n - n);
for idx in n..new_n {
let id = encoder.interpret_allocs[idx];
let pos: u32 = encoder.position().try_into().unwrap();
let pos: u64 = encoder.position().try_into().unwrap();
interpret_alloc_index.push(pos);
interpret::specialized_encode_alloc_id(&mut encoder, tcx, id);
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/parameterized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ trivially_parameterized_over_tcx! {
usize,
(),
u32,
u64,
bool,
std::string::String,
crate::metadata::ModChild,
Expand Down

0 comments on commit 6f65ef5

Please sign in to comment.