Skip to content

Commit

Permalink
Auto merge of #118073 - saethlin:gc-dead-allocs, r=RalfJung
Browse files Browse the repository at this point in the history
Miri: GC the dead_alloc_map too

dead_alloc_map is the last piece of state in the interpreter I can find that leaks. With this PR, all of the long-term memory growth I can find in Miri with programs that do things like run a big `loop {` or run property tests is attributable to some data structure properties in borrow tracking, and is _extremely_ slow.

My only gripe with the commit in this PR is that I don't have a new test for it. I'd like to have a regression test for this, but it would have to be statistical I think because the peak memory of a process that Linux reports is not exactly the same run-to-run. Which means it would have to not be very sensitive to slow leaks (some guesswork suggests for acceptable CI time we would be checking for like 10% memory growth over a minute or two, which is still pretty fast IMO).

Unless someone has a better idea for how to detect a regression, I think on balance I'm fine with manually keeping an eye on the memory use situation.

r? RalfJung
  • Loading branch information
bors committed Nov 23, 2023
2 parents fc13ca6 + f5dae8e commit 38eecca
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
11 changes: 11 additions & 0 deletions compiler/rustc_const_eval/src/interpret/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,17 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// This function is used by Miri's provenance GC to remove unreachable entries from the dead_alloc_map.
pub fn remove_unreachable_allocs(&mut self, reachable_allocs: &FxHashSet<AllocId>) {
// Unlike all the other GC helpers where we check if an `AllocId` is found in the interpreter or
// is live, here all the IDs in the map are for dead allocations so we don't
// need to check for liveness.
#[allow(rustc::potential_query_instability)] // Only used from Miri, not queries.
self.memory.dead_alloc_map.retain(|id, _| reachable_allocs.contains(id));
}
}

/// Allocation accessors
impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
/// Helper function to obtain a global (tcx) allocation.
Expand Down
3 changes: 2 additions & 1 deletion src/tools/miri/src/provenance_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
}

fn remove_unreachable_allocs(&mut self, allocs: FxHashSet<AllocId>) {
let this = self.eval_context_ref();
let this = self.eval_context_mut();
let allocs = LiveAllocs {
ecx: this,
collected: allocs,
Expand All @@ -205,5 +205,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: MiriInterpCxExt<'mir, 'tcx> {
if let Some(borrow_tracker) = &this.machine.borrow_tracker {
borrow_tracker.borrow_mut().remove_unreachable_allocs(&allocs);
}
this.remove_unreachable_allocs(&allocs.collected);
}
}

0 comments on commit 38eecca

Please sign in to comment.