Skip to content

Commit 4ea8553

Browse files
committed
Add expansion-guard integration test and bound the degraded-state comment
Drive expandTableIfNeeded()'s slot-id overflow guard through a real put() call path via a test-only seed seam, and note the skip state is bounded by the next processTraces() rotation.
1 parent 9747d1a commit 4ea8553

3 files changed

Lines changed: 72 additions & 1 deletion

File tree

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,10 @@ CallTraceHashTable::CallTraceHashTable() : _instance_id(0), _parent_storage(null
114114
_overflow = 0;
115115
}
116116

117+
void CallTraceHashTable::seedTableForTesting(u32 slot_base, u32 capacity) {
118+
_table = LongHashTable::allocate(nullptr, capacity, slot_base, &_allocator);
119+
}
120+
117121
CallTraceHashTable::~CallTraceHashTable() {
118122
// LinearAllocator handles all memory cleanup automatically
119123
// No need to explicitly destroy tables since they're allocated from LinearAllocator
@@ -304,7 +308,10 @@ void CallTraceHashTable::expandTableIfNeeded(LongHashTable* table, u32 size) {
304308
if (wouldExceedSlotIdRange(table->slotBase(), capacity)) {
305309
// Expanding would push slot_base + local_slot past 2^32, carrying
306310
// into instance_id's bit range. Skip expansion; put() keeps working
307-
// on the current table at a higher load factor.
311+
// on the current table at a higher load factor. This degraded state
312+
// is bounded, not permanent: the next processTraces() rotation resets
313+
// slot_base to 0 via clearTableOnly(), so normal JFR-flush cadence
314+
// recovers it.
308315
Counters::increment(CALLTRACE_STORAGE_EXPANSION_SKIPPED);
309316
return;
310317
}

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ class CallTraceStorage;
5656
class CallTraceHashTable {
5757
static constexpr double LOAD_RATIO = 3.0 / 4.0;
5858

59+
friend class CallTraceHashTableTestAccessor;
60+
5961
public:
6062
static CallTrace _overflow_trace;
6163

@@ -91,6 +93,13 @@ class CallTraceHashTable {
9193

9294
void expandTableIfNeeded(LongHashTable* table, u32 size);
9395

96+
// Test-only seam: replaces the live table with a synthetic one seeded at
97+
// an explicit slot_base/capacity, so gtest can drive expandTableIfNeeded()
98+
// through its real call path at the 2^32 slot-id boundary without
99+
// billions of real put() calls to get there. Only ever invoked via
100+
// CallTraceHashTableTestAccessor (test_callTraceStorage.cpp).
101+
void seedTableForTesting(u32 slot_base, u32 capacity);
102+
94103
public:
95104
CallTraceHashTable();
96105
~CallTraceHashTable();

ddprof-lib/src/test/cpp/test_callTraceStorage.cpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "callTraceHashTable.h"
1616
#include "gtest_crash_handler.h"
1717
#include "arch.h"
18+
#include "counters.h"
1819

1920
// Test name for crash handler
2021
static constexpr char TEST_NAME[] = "CallTraceStorageTest";
@@ -914,6 +915,60 @@ std::unique_ptr<CallTraceHashTable, void(*)(CallTraceHashTable*)> makeHeapCallTr
914915
}
915916
} // namespace
916917

918+
// Test-only accessor granting access to CallTraceHashTable::seedTableForTesting,
919+
// following the CallTraceHashTable::friend CallTraceHashTableTestAccessor;
920+
// convention used elsewhere in this codebase (e.g. ObjectSamplerTestAccessor,
921+
// ProfilerTestAccessor).
922+
class CallTraceHashTableTestAccessor {
923+
public:
924+
static void seedTable(CallTraceHashTable* table, u32 slot_base, u32 capacity) {
925+
table->seedTableForTesting(slot_base, capacity);
926+
}
927+
};
928+
929+
// Companion to Test 4 below: the expansion-overflow guard's real integration
930+
// point inside
931+
// expandTableIfNeeded() must skip the actual table swap (not just the pure
932+
// wouldExceedSlotIdRange() helper exercised in Test 4) once a real put()
933+
// crosses the load-ratio threshold on a table seeded at the 2^32 slot-id
934+
// boundary, incrementing CALLTRACE_STORAGE_EXPANSION_SKIPPED and keeping
935+
// put() functional afterwards. The table is seeded via
936+
// CallTraceHashTableTestAccessor since reaching this boundary through real
937+
// put()-driven expansions alone would require billions of inserts.
938+
TEST_F(CallTraceStorageTest, ExpansionGuardSkipsRealExpansionNearSlotIdBoundary) {
939+
auto hash_table_ptr = makeHeapCallTraceHashTable();
940+
ASSERT_NE(hash_table_ptr.get(), nullptr) << "Failed to allocate aligned memory for CallTraceHashTable";
941+
CallTraceHashTable& hash_table = *hash_table_ptr;
942+
hash_table.setInstanceId(1);
943+
944+
// Matches the "one past the boundary" case from Test 4: with this
945+
// slot_base/capacity pair, wouldExceedSlotIdRange() is true, so
946+
// expandTableIfNeeded() must take the skip branch instead of expanding.
947+
constexpr u64 kSlotIdRange = 0x100000000ull; // 2^32
948+
u64 boundary_slot_base = kSlotIdRange - kInitialCapacity - (u64)kInitialCapacity * 2 + 1;
949+
CallTraceHashTableTestAccessor::seedTable(&hash_table, static_cast<u32>(boundary_slot_base), kInitialCapacity);
950+
951+
long long skipped_before = Counters::getCounter(CALLTRACE_STORAGE_EXPANSION_SKIPPED);
952+
953+
// Insert past the load-ratio threshold; every put() past it re-checks the
954+
// guard (since the table is never actually swapped), so the counter must
955+
// increase by more than one, proving the degraded state persists rather
956+
// than being a one-off fluke.
957+
const u32 TOTAL_COUNT = kExpansionThreshold + 100;
958+
for (u32 i = 0; i < TOTAL_COUNT; i++) {
959+
ASGCT_CallFrame frame;
960+
frame.bci = static_cast<int>(i);
961+
frame.method_id = reinterpret_cast<jmethodID>(static_cast<uintptr_t>(i + 1));
962+
u64 trace_id = hash_table.put(1, &frame, false, 1);
963+
ASSERT_NE(trace_id, kOverflowTraceId) << "Unexpected overflow at i=" << i;
964+
ASSERT_NE(trace_id, CallTraceStorage::DROPPED_TRACE_ID) << "Unexpected drop at i=" << i;
965+
}
966+
967+
long long skipped_after = Counters::getCounter(CALLTRACE_STORAGE_EXPANSION_SKIPPED);
968+
EXPECT_GT(skipped_after - skipped_before, 1)
969+
<< "Expansion-overflow guard did not repeatedly skip expansion near the slot-id boundary";
970+
}
971+
917972
// Test 1: forcing at least one expansion must not produce duplicate
918973
// trace_ids, and no successful put() may return OVERFLOW_TRACE_ID or
919974
// DROPPED_TRACE_ID.

0 commit comments

Comments
 (0)