fix(profiler): pre-register vtable receiver classes via JVMTI (PROF-14618) by jbachorik · Pull Request #527 · DataDog/java-profiler

jbachorik · 2026-05-13T10:25:27Z

What does this PR do?:

Restores vtable_target frame capture in CPU-only recordings. After PR #512 (signal-safe walkVM), _class_map was only populated by allocation/liveness paths via lookupClass. In a CPU-only recording these paths never fire, so bounded_lookup(.., size_limit=0) in hotspotSupport.cpp always missed and vtable-target frames were silently dropped.

This change pre-populates _class_map from safe JVM-thread contexts:

Profiler::preregisterLoadedClasses(jvmtiEnv*) calls GetLoadedClasses + GetClassSignature + ObjectSampler::normalizeClassSignature, inserting every reference class into _class_map. Invoked inside the existing exclusive _class_map_lock at Profiler::start() and Profiler::dump() so signal handlers see a coherent, fully-populated map.
VM::ClassPrepare is moved out-of-line to vmEntry.cpp (vmEntry.h is included by profiler.h, hence the prior inline definition could not reference Profiler::instance()). The handler still calls loadMethodIDs first, then conditionally inserts the new class when _features.vtable_target is set.

The hot signal-handler path in hotspotSupport.cpp is unchanged — pre-registration runs only on JVM threads, never under a signal.

Motivation:

Follow-up to PR #512 / PROF-14582. lookupClass from a signal handler is unsafe (malloc inside Dictionary::lookup), so the signal-safe path was made read-only via bounded_lookup(size_limit=0). That left CPU-only recordings without any path that would populate the map, silently breaking the vtable-target feature.

Additional Notes:

Profiler::start() is called before _features is assigned, so the gate inside the exclusive-lock block reads args._features.vtable_target && VMStructs::hasClassNames() (mirroring the sanitisation at profiler.cpp:1129). Profiler::dump() uses the same condition on _features directly.
preregisterLoadedClasses calls _class_map.lookup directly rather than Profiler::lookupClass, because the caller already holds the exclusive lock — lookupClass would re-attempt tryLockShared and either spin or return -1, defeating the purpose.
JVMTI errors on GetLoadedClasses / GetClassSignature are now logged via Log::warn so silent skips are observable.
Primitive and array signatures from GetClassSignature pass through normalizeClassSignature unstripped; the resulting keys never match walkVM's lookup format, so they are harmless dictionary entries.
Locking-overhead concerns raised in code review are addressed by the locking-improvement work in fix(profiler): lock-free class/endpoint/context maps via TripleBufferedDictionary #524 — once that lands, _class_map_lock no longer blocks signal readers across the full bulk-iteration window.
The Java integration test was intentionally dropped from this PR. vtable_target has no CLI toggle yet and the synthetic BCI_ALLOC frame is recorded as a class id (not as text in STACK_TRACE_STRING), so a useful integration test needs a separate follow-up.

How to test the change?:

New C++ unit test: dictionary_concurrent_ut.cpp::BulkInsertThenBoundedLookupHitsMirrorInsertedIds validates that bulk pre-registration under an exclusive lock makes every key visible to bounded_lookup(.., 0) from a shared context, and that absent keys return INT_MAX.
All 6 DictionaryConcurrent tests pass locally via ./gradlew :ddprof-lib:gtestDebug_dictionary_concurrent_ut.

For Datadog employees:

If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
This PR doesn't touch any of that.
JIRA: PROF-14618

Copilot

Pull request overview

This PR restores vtable_target frame capture for CPU-only recordings by ensuring _class_map is populated from JVM-thread contexts (rather than relying on allocation/liveness paths that don’t run in CPU-only mode), keeping the signal-handler lookup path read-only and signal-safe.

Changes:

Bulk pre-registers loaded classes into _class_map during Profiler::start() and Profiler::dump() under the existing exclusive _class_map_lock.
Moves VM::ClassPrepare out-of-line and extends it to insert newly prepared classes into _class_map when vtable_target is enabled.
Adds a C++ unit test covering the “bulk insert then bounded_lookup(…, 0) hits” contract and stages a (currently disabled) Java integration test.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ddprof-test/src/test/java/com/datadoghq/profiler/cpu/VtableTargetCpuTest.java	Adds a staged (disabled) integration test outlining expected vtable-target behavior in CPU samples.
ddprof-lib/src/test/cpp/dictionary_concurrent_ut.cpp	Adds a unit test validating bulk insertion visibility to `bounded_lookup(..., 0)` and sentinel behavior on miss.
ddprof-lib/src/main/cpp/vmEntry.h	Moves `VM::ClassPrepare` definition out of the header into `vmEntry.cpp`.
ddprof-lib/src/main/cpp/vmEntry.cpp	Implements `VM::ClassPrepare` to insert normalized class signatures into the class map when `vtable_target` is enabled.
ddprof-lib/src/main/cpp/profiler.h	Adds `Profiler::preregisterLoadedClasses(jvmtiEnv*)` declaration and documents intended locking/threading constraints.
ddprof-lib/src/main/cpp/profiler.cpp	Calls `preregisterLoadedClasses()` during start/dump class-map resets and implements the bulk JVMTI enumeration/insertion logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+  const char* slice = nullptr;
+  size_t slice_len = 0;
+  if (ObjectSampler::normalizeClassSignature(sig, &slice, &slice_len)) {
+    (void)profiler->lookupClass(slice, slice_len);


+  // Pre-populate _class_map with all currently-loaded reference classes so that
+  // signal-safe lookups in walkVM (vtable_target) can resolve them without ever
+  // needing to malloc. Caller MUST hold _class_map_lock EXCLUSIVELY. Runs on a
+  // JVM thread (never in a signal handler).


+// (4a) Bulk exclusive-lock insert (mimicking preregisterLoadedClasses) followed
+// by read-only bounded_lookup(0) on each inserted key. Verifies that the
+// pre-registration contract holds: every key inserted while the exclusive lock
+// is held is subsequently visible to bounded_lookup(0) from a shared context.
+TEST(DictionaryConcurrent, BulkInsertThenBoundedLookupHitsMirrorInsertedIds) {


dd-octo-sts · 2026-05-13T11:42:44Z

CI Test Results

Run: #25813605948 | Commit: 3196908 | Duration: 25m 22s (longest job)

❌ 23 of 32 test jobs failed

Status Overview

JDK	glibc-aarch64/debug	glibc-amd64/debug	musl-aarch64/debug	musl-amd64/debug
8	-	❌	-	-
8-ibm	-	❌	-	-
8-j9	✅	❌	-	-
8-librca	-	-	❌	❌
8-orcl	-	❌	-	-
11	-	❌	-	-
11-j9	✅	❌	-	-
11-librca	-	-	❌	❌
17	✅	❌	-	-
17-graal	✅	❌	-	-
17-j9	✅	❌	-	-
17-librca	-	-	❌	❌
21	✅	❌	-	-
21-graal	✅	❌	-	-
21-librca	-	-	❌	❌
25	✅	❌	-	-
25-graal	✅	❌	-	-
25-librca	-	-	❌	❌

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled