Skip to content

Commit a48899b

Browse files
committed
Fix FrontierTable capacity poisoning across shared-JVM tests
resetSearchStateForTest() only reset the frontier table's occupancy, not its capacity, so an earlier test's smaller framecap= permanently capped the singleton for every later test in the same JVM.
1 parent f2d8978 commit a48899b

2 files changed

Lines changed: 49 additions & 3 deletions

File tree

ddprof-lib/src/main/cpp/referenceChains.cpp

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,22 @@ FrontierTable::FrontierTable(int max_cap)
3838

3939
FrontierTable::~FrontierTable() { free(_table); }
4040

41+
void FrontierTable::resetCapacityForTest(int max_cap) {
42+
_table_lock.lock();
43+
free(_table);
44+
_table = nullptr;
45+
_table_max_cap = std::max(max_cap, 0);
46+
_table_cap = std::min(INITIAL_TABLE_CAPACITY, _table_max_cap);
47+
if (_table_cap > 0) {
48+
_table = (FrontierEntry *)calloc(_table_cap, sizeof(FrontierEntry));
49+
if (_table == nullptr) {
50+
_table_cap = 0;
51+
}
52+
}
53+
_table_size.store(0, std::memory_order_relaxed);
54+
_table_lock.unlock();
55+
}
56+
4157
bool FrontierTable::growLocked(int required_cap) {
4258
if (required_cap <= _table_cap) {
4359
return true;
@@ -237,8 +253,14 @@ Error ReferenceChainTracker::start(Arguments &args) {
237253
// frontier table once and keep it across repeated start()/stop() cycles -
238254
// do not reallocate on a second start() with a possibly different cap, for
239255
// the same reason LivenessTracker keeps its first-initialize() result.
256+
// Recorded unconditionally, even on a start() call that finds _frontier
257+
// already constructed (see _configured_frontier_cap's own comment) - this
258+
// is what resetSearchStateForTest() rebuilds the table at, undoing
259+
// whatever cap an earlier test in this same JVM happened to construct it
260+
// with.
261+
_configured_frontier_cap = args._reference_chains_frontier_cap;
240262
if (_frontier == nullptr) {
241-
_frontier = new FrontierTable(args._reference_chains_frontier_cap);
263+
_frontier = new FrontierTable(_configured_frontier_cap);
242264
}
243265

244266
_hop_cap = args._reference_chains_hop_cap;
@@ -647,7 +669,12 @@ void ReferenceChainTracker::resetSearchStateForTest(jvmtiEnv *jvmti,
647669
_search_pain_ms = 0;
648670

649671
if (_frontier != nullptr) {
650-
_frontier->resetForRestart();
672+
// Rebuilds the table at this test's own _configured_frontier_cap,
673+
// undoing any smaller framecap= an earlier test left it permanently
674+
// sized at (this class's own header comment on @TestMethodOrder) -
675+
// restartSearch()'s production path only calls the cheaper
676+
// resetForRestart() since it never needs to change the cap mid-JVM.
677+
_frontier->resetCapacityForTest(_configured_frontier_cap);
651678
}
652679
_next_tag = 1;
653680

ddprof-lib/src/main/cpp/referenceChains.h

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,17 @@ class alignas(alignof(SpinLock)) FrontierTable {
335335
_table_lock.unlock();
336336
}
337337

338+
// Debug-only test seam (ReferenceChainTracker::resetSearchStateForTest()).
339+
// Unlike resetForRestart(), which only forgets this table's occupancy,
340+
// this discards the table's whole allocation and rebuilds it at
341+
// `max_cap` - the only way to undo the "sized once, on the first start()
342+
// in this JVM" capacity choice (this class's own constructor comment)
343+
// that a differently-configured test running earlier in the same,
344+
// no-forkEvery JVM (ProfilerTestPlugin.kt) would otherwise leave every
345+
// later test permanently stuck with. Defined in referenceChains.cpp
346+
// alongside the constructor it mirrors.
347+
void resetCapacityForTest(int max_cap);
348+
338349
int capacity() const { return _table_cap; }
339350
int maxCapacity() const { return _table_max_cap; }
340351

@@ -415,6 +426,13 @@ class ReferenceChainTracker {
415426
// multiple start/stop recording cycles.
416427
FrontierTable *_frontier;
417428

429+
// args._reference_chains_frontier_cap as of the most recent start() call -
430+
// recorded unconditionally (even once _frontier already exists and start()
431+
// itself skips reconstructing it), so resetSearchStateForTest() has
432+
// something to rebuild the table at other than whatever cap the first
433+
// start() in this JVM happened to use (see _frontier's own comment).
434+
int _configured_frontier_cap;
435+
418436
// Class-tag -> StringDictionary id table. Populated by
419437
// resolveLoadedClasses(), read by heapReferenceCallback(). Survives
420438
// stop()/start() cycles for the same reason _frontier does - a class,
@@ -765,7 +783,8 @@ class ReferenceChainTracker {
765783
std::atomic<bool> _abort_pass_requested;
766784

767785
ReferenceChainTracker()
768-
: _enabled(false), _frontier(nullptr), _last_class_map_generation(0),
786+
: _enabled(false), _frontier(nullptr), _configured_frontier_cap(0),
787+
_last_class_map_generation(0),
769788
_last_resolved_class_count(0),
770789
_gc_start_epoch(0),
771790
_gc_finish_epoch(0), _next_tag(1), _next_class_tag_magnitude(1),

0 commit comments

Comments
 (0)