Skip to content

Commit 1d7af29

Browse files
Add GC heuristic
1 parent d4e917f commit 1d7af29

File tree

7 files changed

+86
-14
lines changed

7 files changed

+86
-14
lines changed

src/bin/miri.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ use std::num::NonZero;
2828
use std::path::PathBuf;
2929
use std::str::FromStr;
3030

31-
use miri::{BacktraceStyle, BorrowTrackerMethod, ProvenanceMode, RetagFields, ValidationMode};
31+
use miri::{
32+
BacktraceStyle, BorrowTrackerMethod, ProvenanceGcSettings, ProvenanceMode, RetagFields,
33+
ValidationMode,
34+
};
3235
use rustc_data_structures::sync::Lrc;
3336
use rustc_driver::Compilation;
3437
use rustc_hir::def_id::LOCAL_CRATE;
@@ -607,7 +610,10 @@ fn main() {
607610
let interval = param.parse::<u32>().unwrap_or_else(|err| {
608611
show_error!("-Zmiri-provenance-gc requires a `u32`: {}", err)
609612
});
610-
miri_config.gc_interval = interval;
613+
miri_config.gc_settings = match interval {
614+
0 => ProvenanceGcSettings::Disabled,
615+
interval => ProvenanceGcSettings::Regularly { interval },
616+
};
611617
} else if let Some(param) = arg.strip_prefix("-Zmiri-measureme=") {
612618
miri_config.measureme_out = Some(param.to_string());
613619
} else if let Some(param) = arg.strip_prefix("-Zmiri-backtrace=") {

src/borrow_tracker/tree_borrows/mod.rs

+14
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ impl<'tcx> Tree {
6060
};
6161
let global = machine.borrow_tracker.as_ref().unwrap();
6262
let span = machine.current_span();
63+
if self.tree_grew_significantly_since_last_gc() {
64+
machine.request_gc();
65+
}
6366
self.perform_access(
6467
tag,
6568
Some((range, access_kind, diagnostics::AccessCause::Explicit(access_kind))),
@@ -85,6 +88,9 @@ impl<'tcx> Tree {
8588
};
8689
let global = machine.borrow_tracker.as_ref().unwrap();
8790
let span = machine.current_span();
91+
if self.tree_grew_significantly_since_last_gc() {
92+
machine.request_gc();
93+
}
8894
self.dealloc(tag, alloc_range(Size::ZERO, size), global, alloc_id, span)
8995
}
9096

@@ -106,6 +112,9 @@ impl<'tcx> Tree {
106112
alloc_id: AllocId, // diagnostics
107113
) -> InterpResult<'tcx> {
108114
let span = machine.current_span();
115+
if self.tree_grew_significantly_since_last_gc() {
116+
machine.request_gc();
117+
}
109118
// `None` makes it the magic on-protector-end operation
110119
self.perform_access(tag, None, global, alloc_id, span)
111120
}
@@ -297,6 +306,11 @@ trait EvalContextPrivExt<'tcx>: crate::MiriInterpCxExt<'tcx> {
297306
)?;
298307
// Record the parent-child pair in the tree.
299308
tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?;
309+
310+
// Also request a GC if things are getting too large.
311+
if tree_borrows.tree_grew_significantly_since_last_gc() {
312+
this.machine.request_gc();
313+
}
300314
drop(tree_borrows);
301315

302316
// Also inform the data race model (but only if any bytes are actually affected).

src/borrow_tracker/tree_borrows/tree.rs

+20-1
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,8 @@ pub struct Tree {
259259
pub(super) rperms: RangeMap<UniValMap<LocationState>>,
260260
/// The index of the root node.
261261
pub(super) root: UniIndex,
262+
/// The number of nodes when we last ran the Garbage Collector
263+
nodes_at_last_gc: usize,
262264
}
263265

264266
/// A node in the borrow tree. Each node is uniquely identified by a tag via
@@ -604,7 +606,7 @@ impl Tree {
604606
perms.insert(root_idx, LocationState::new_init(Permission::new_active()));
605607
RangeMap::new(size, perms)
606608
};
607-
Self { root: root_idx, nodes, rperms, tag_mapping }
609+
Self { root: root_idx, nodes, rperms, tag_mapping, nodes_at_last_gc: 1 }
608610
}
609611
}
610612

@@ -833,13 +835,30 @@ impl<'tcx> Tree {
833835

834836
/// Integration with the BorTag garbage collector
835837
impl Tree {
838+
/// The number of nodes in this tree
839+
pub fn nodes_count(&self) -> usize {
840+
self.tag_mapping.len()
841+
}
842+
843+
/// Whether the tree grew significantly since the last provenance GC run
844+
pub fn tree_grew_significantly_since_last_gc(&self) -> bool {
845+
let current = self.nodes_count();
846+
// do not trigger the GC for small nodes
847+
let last = self.nodes_at_last_gc.max(50);
848+
// trigger the GC if the tree doubled since the last run,
849+
// or otherwise got "significantly" larger.
850+
// Note that for trees < 100 nodes, nothing happens.
851+
current > 2 * last || current > last + 1500
852+
}
853+
836854
pub fn remove_unreachable_tags(&mut self, live_tags: &FxHashSet<BorTag>) {
837855
self.remove_useless_children(self.root, live_tags);
838856
// Right after the GC runs is a good moment to check if we can
839857
// merge some adjacent ranges that were made equal by the removal of some
840858
// tags (this does not necessarily mean that they have identical internal representations,
841859
// see the `PartialEq` impl for `UniValMap`)
842860
self.rperms.merge_adjacent_thorough();
861+
self.nodes_at_last_gc = self.nodes_count();
843862
}
844863

845864
/// Checks if a node is useless and should be GC'ed.

src/eval.rs

+15-3
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,18 @@ pub enum ValidationMode {
8383
Deep,
8484
}
8585

86+
/// Settings for the provenance GC
87+
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
88+
pub enum ProvenanceGcSettings {
89+
/// The GC is disabled
90+
Disabled,
91+
/// The GC is to run regularly, every `inverval` basic blocks
92+
Regularly { interval: u32 },
93+
/// The GC runs as needed, determined by heuristics.
94+
/// See `should_run_provenance_gc`
95+
Heuristically,
96+
}
97+
8698
/// Configuration needed to spawn a Miri instance.
8799
#[derive(Clone)]
88100
pub struct MiriConfig {
@@ -145,8 +157,8 @@ pub struct MiriConfig {
145157
/// The location of a shared object file to load when calling external functions
146158
/// FIXME! consider allowing users to specify paths to multiple files, or to a directory
147159
pub native_lib: Option<PathBuf>,
148-
/// Run a garbage collector for BorTags every N basic blocks.
149-
pub gc_interval: u32,
160+
/// Settings for the provenance GC (e.g. how often it runs)
161+
pub gc_settings: ProvenanceGcSettings,
150162
/// The number of CPUs to be reported by miri.
151163
pub num_cpus: u32,
152164
/// Requires Miri to emulate pages of a certain size
@@ -188,7 +200,7 @@ impl Default for MiriConfig {
188200
report_progress: None,
189201
retag_fields: RetagFields::Yes,
190202
native_lib: None,
191-
gc_interval: 10_000,
203+
gc_settings: ProvenanceGcSettings::Heuristically,
192204
num_cpus: 1,
193205
page_size: None,
194206
collect_leak_backtraces: true,

src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ pub use crate::diagnostics::{
131131
EvalContextExt as _, NonHaltingDiagnostic, TerminationInfo, report_error,
132132
};
133133
pub use crate::eval::{
134-
AlignmentCheck, BacktraceStyle, IsolatedOp, MiriConfig, RejectOpWith, ValidationMode,
135-
create_ecx, eval_entry,
134+
AlignmentCheck, BacktraceStyle, IsolatedOp, MiriConfig, ProvenanceGcSettings, RejectOpWith,
135+
ValidationMode, create_ecx, eval_entry,
136136
};
137137
pub use crate::helpers::{AccessKind, EvalContextExt as _};
138138
pub use crate::intrinsics::EvalContextExt as _;

src/machine.rs

+12-6
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
44
use std::any::Any;
55
use std::borrow::Cow;
6-
use std::cell::RefCell;
6+
use std::cell::{Cell, RefCell};
77
use std::collections::hash_map::Entry;
88
use std::path::Path;
99
use std::{fmt, process};
@@ -556,9 +556,10 @@ pub struct MiriMachine<'tcx> {
556556
pub native_lib: Option<!>,
557557

558558
/// Run a garbage collector for BorTags every N basic blocks.
559-
pub(crate) gc_interval: u32,
559+
pub(crate) gc_settings: ProvenanceGcSettings,
560560
/// The number of blocks that passed since the last BorTag GC pass.
561561
pub(crate) since_gc: u32,
562+
pub(crate) gc_requested: Cell<bool>,
562563

563564
/// The number of CPUs to be reported by miri.
564565
pub(crate) num_cpus: u32,
@@ -716,8 +717,9 @@ impl<'tcx> MiriMachine<'tcx> {
716717
native_lib: config.native_lib.as_ref().map(|_| {
717718
panic!("calling functions from native libraries via FFI is only supported on Unix")
718719
}),
719-
gc_interval: config.gc_interval,
720+
gc_settings: config.gc_settings,
720721
since_gc: 0,
722+
gc_requested: Cell::new(false),
721723
num_cpus: config.num_cpus,
722724
page_size,
723725
stack_addr,
@@ -784,6 +786,10 @@ impl<'tcx> MiriMachine<'tcx> {
784786
.and_then(|(_allocated, deallocated)| *deallocated)
785787
.map(Span::data)
786788
}
789+
790+
pub(crate) fn request_gc(&self) {
791+
self.gc_requested.set(true)
792+
}
787793
}
788794

789795
impl VisitProvenance for MiriMachine<'_> {
@@ -828,8 +834,9 @@ impl VisitProvenance for MiriMachine<'_> {
828834
report_progress: _,
829835
basic_block_count: _,
830836
native_lib: _,
831-
gc_interval: _,
837+
gc_settings: _,
832838
since_gc: _,
839+
gc_requested: _,
833840
num_cpus: _,
834841
page_size: _,
835842
stack_addr: _,
@@ -1491,8 +1498,7 @@ impl<'tcx> Machine<'tcx> for MiriMachine<'tcx> {
14911498
// stacks.
14921499
// When debug assertions are enabled, run the GC as often as possible so that any cases
14931500
// where it mistakenly removes an important tag become visible.
1494-
if ecx.machine.gc_interval > 0 && ecx.machine.since_gc >= ecx.machine.gc_interval {
1495-
ecx.machine.since_gc = 0;
1501+
if ecx.should_run_provenance_gc() {
14961502
ecx.run_provenance_gc();
14971503
}
14981504

src/provenance_gc.rs

+15
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,23 @@ fn remove_unreachable_allocs<'tcx>(this: &mut MiriInterpCx<'tcx>, allocs: FxHash
208208

209209
impl<'tcx> EvalContextExt<'tcx> for crate::MiriInterpCx<'tcx> {}
210210
pub trait EvalContextExt<'tcx>: MiriInterpCxExt<'tcx> {
211+
fn should_run_provenance_gc(&self) -> bool {
212+
let this = self.eval_context_ref();
213+
match this.machine.gc_settings {
214+
ProvenanceGcSettings::Disabled => false,
215+
ProvenanceGcSettings::Regularly { interval } => this.machine.since_gc >= interval,
216+
ProvenanceGcSettings::Heuristically =>
217+
// no GC run if the last one was recently
218+
this.machine.since_gc >= 75
219+
// run GC if requested, or every 10000 blocks otherwise
220+
&& (this.machine.since_gc >= 10_000 || this.machine.gc_requested.get()),
221+
}
222+
}
223+
211224
fn run_provenance_gc(&mut self) {
212225
let this = self.eval_context_mut();
226+
this.machine.since_gc = 0;
227+
this.machine.gc_requested.set(false);
213228

214229
// We collect all tags and AllocId from every part of the interpreter.
215230
let mut tags = FxHashSet::default();

0 commit comments

Comments
 (0)