Skip to content

Commit f1655d8

Browse files
authored
Run binding tests with extreme_assertions (#392)
* Add may_trace_duplicate_edges to PlanConstraints. Duplicate edge check in extreme_assertions won't run for some plans. * Add side metadata sanity for PageProtect. * Add a warning when extreme_assertions is enabled. * Allow longer timeout for binding tests * Add comments for how to create a plan constructor
1 parent 54a0499 commit f1655d8

File tree

12 files changed

+63
-9
lines changed

12 files changed

+63
-9
lines changed

.github/workflows/post-review-ci.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
# JikesRVM
5353
jikesrvm-binding-test:
5454
runs-on: ubuntu-18.04
55-
timeout-minutes: 60
55+
timeout-minutes: 90
5656
if: contains(github.event.pull_request.labels.*.name, 'PR-testing')
5757
steps:
5858
- name: Checkout MMTk Core
@@ -183,7 +183,7 @@ jobs:
183183
# OpenJDK
184184
openjdk-binding-test:
185185
runs-on: ubuntu-18.04
186-
timeout-minutes: 120
186+
timeout-minutes: 240
187187
if: contains(github.event.pull_request.labels.*.name, 'PR-testing')
188188
steps:
189189
- name: Checkout MMTk Core

docs/tutorial/code/mygc_semispace/global.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ impl<VM: VMBinding> MyGC<VM> {
202202
common: CommonPlan::new(vm_map, mmapper, options, heap, &MYGC_CONSTRAINTS, global_metadata_specs.clone()),
203203
};
204204

205+
// Use SideMetadataSanity to check if each spec is valid. This is also needed for check
206+
// side metadata in extreme_assertions.
205207
let mut side_metadata_sanity_checker = SideMetadataSanity::new();
206208
res.common.verify_side_metadata_sanity(&mut side_metadata_sanity_checker);
207209
res.copyspace0.verify_side_metadata_sanity(&mut side_metadata_sanity_checker);

src/memory_manager.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ pub fn gc_init<VM: VMBinding>(mmtk: &'static mut MMTK<VM>, heap_size: usize) {
7171
mmtk.plan
7272
.gc_init(heap_size, &crate::VM_MAP, &mmtk.scheduler);
7373
info!("Initialized MMTk with {:?}", mmtk.options.plan);
74+
#[cfg(feature = "extreme_assertions")]
75+
warn!("The feature 'extreme_assertions' is enabled. MMTk will run expensive run-time checks. Slow performance should be expected.");
7476
}
7577

7678
/// Request MMTk to create a mutator for the given thread. For performance reasons, A VM should

src/plan/gencopy/global.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,8 @@ impl<VM: VMBinding> GenCopy<VM> {
265265
next_gc_full_heap: AtomicBool::new(false),
266266
};
267267

268+
// Use SideMetadataSanity to check if each spec is valid. This is also needed for check
269+
// side metadata in extreme_assertions.
268270
{
269271
let mut side_metadata_sanity_checker = SideMetadataSanity::new();
270272
res.common

src/plan/global.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,17 @@ pub fn create_plan<VM: VMBinding>(
160160
///
161161
/// The global instance defines and manages static resources
162162
/// (such as memory and virtual memory resources).
163+
///
164+
/// Constructor:
165+
///
166+
/// For the constructor of a new plan, there are a few things the constructor _must_ do
167+
/// (please check existing plans and see what they do in the constructor):
168+
/// 1. Create a HeapMeta, and use this HeapMeta to initialize all the spaces.
169+
/// 2. Create a vector of all the side metadata specs with `SideMetadataContext::new_global_specs()`,
170+
/// the parameter is a vector of global side metadata specs that are specific to the plan.
171+
/// 3. Initialize all the spaces the plan uses with the heap meta, and the global metadata specs vector.
172+
/// 4. Create a `SideMetadataSanity` object, and invoke verify_side_metadata_sanity() for each space (or
173+
/// invoke verify_side_metadata_sanity() in `CommonPlan`/`BasePlan` for the spaces in the common/base plan).
163174
pub trait Plan: 'static + Sync + Downcast {
164175
type VM: VMBinding;
165176

src/plan/marksweep/global.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ pub const MS_CONSTRAINTS: PlanConstraints = PlanConstraints {
4040
gc_header_bits: 2,
4141
gc_header_words: 0,
4242
num_specialized_scans: 1,
43+
may_trace_duplicate_edges: true,
4344
..PlanConstraints::default()
4445
};
4546

@@ -150,6 +151,8 @@ impl<VM: VMBinding> MarkSweep<VM> {
150151
),
151152
};
152153

154+
// Use SideMetadataSanity to check if each spec is valid. This is also needed for check
155+
// side metadata in extreme_assertions.
153156
{
154157
let mut side_metadata_sanity_checker = SideMetadataSanity::new();
155158
res.common

src/plan/nogc/global.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ impl<VM: VMBinding> NoGC<VM> {
139139
),
140140
};
141141

142+
// Use SideMetadataSanity to check if each spec is valid. This is also needed for check
143+
// side metadata in extreme_assertions.
142144
let mut side_metadata_sanity_checker = SideMetadataSanity::new();
143145
res.base
144146
.verify_side_metadata_sanity(&mut side_metadata_sanity_checker);

src/plan/pageprotect/global.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ impl<VM: VMBinding> PageProtect<VM> {
148148
let mut heap = HeapMeta::new(HEAP_START, HEAP_END);
149149
let global_metadata_specs = SideMetadataContext::new_global_specs(&[]);
150150

151-
PageProtect {
151+
let ret = PageProtect {
152152
space: LargeObjectSpace::new(
153153
"los",
154154
true,
@@ -168,6 +168,19 @@ impl<VM: VMBinding> PageProtect<VM> {
168168
&CONSTRAINTS,
169169
global_metadata_specs,
170170
),
171+
};
172+
173+
// Use SideMetadataSanity to check if each spec is valid. This is also needed for check
174+
// side metadata in extreme_assertions.
175+
{
176+
use crate::util::metadata::side_metadata::SideMetadataSanity;
177+
let mut side_metadata_sanity_checker = SideMetadataSanity::new();
178+
ret.common
179+
.verify_side_metadata_sanity(&mut side_metadata_sanity_checker);
180+
ret.space
181+
.verify_side_metadata_sanity(&mut side_metadata_sanity_checker);
171182
}
183+
184+
ret
172185
}
173186
}

src/plan/plan_constraints.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ pub struct PlanConstraints {
1919
/// Size (in bytes) beyond which copied objects must be copied to the LOS.
2020
/// This depends on the copy allocator.
2121
pub max_non_los_copy_bytes: usize,
22+
/// Some plans may allow benign race for testing mark bit, and this will lead to trace the same edges
23+
/// multiple times. If a plan allows tracing duplicate edges, we will not run duplicate edge check
24+
/// in extreme_assertions.
25+
pub may_trace_duplicate_edges: bool,
2226
pub barrier: BarrierSelector,
2327
// the following seems unused for now
2428
pub needs_linear_scan: bool,
@@ -39,6 +43,7 @@ impl PlanConstraints {
3943
needs_linear_scan: SUPPORT_CARD_SCANNING || LAZY_SWEEP,
4044
needs_concurrent_workers: false,
4145
generate_gc_trace: false,
46+
may_trace_duplicate_edges: false,
4247
needs_forward_after_liveness: false,
4348
barrier: BarrierSelector::NoBarrier,
4449
}

src/plan/semispace/global.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,8 @@ impl<VM: VMBinding> SemiSpace<VM> {
187187
),
188188
};
189189

190+
// Use SideMetadataSanity to check if each spec is valid. This is also needed for check
191+
// side metadata in extreme_assertions.
190192
{
191193
let mut side_metadata_sanity_checker = SideMetadataSanity::new();
192194
res.common

0 commit comments

Comments
 (0)