Conversation
2 tasks
Contributor
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
CI Test ResultsRun: #35750787990 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-22 17:27:24 UTC |
Contributor
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
from
September 17, 2026 19:08
e8f5ee5 to
93f2dbf
Compare
jbachorik
added this pull request to stack #803
September 17, 2026 20:02
jbachorik
marked this pull request as ready for review
September 17, 2026 20:03
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
jbachorik
marked this pull request as draft
September 17, 2026 20:04
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
from
September 17, 2026 20:05
93f2dbf to
217a7ab
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 93f2dbf486
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![datadog-datadog-prod-us1-2[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
Stopping a recording can omit all reference-chain events since the last dump. The lifecycle also keeps strong thread references and stale chain data across recording sessions.
🤖 Datadog Autotest · Commit 93f2dbf · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
from
September 17, 2026 20:19
217a7ab to
b5588b7
Compare
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
from
September 17, 2026 20:20
b5588b7 to
b09e78d
Compare
2 tasks
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
2 times, most recently
from
September 17, 2026 21:33
e99dbc2 to
61cfd3d
Compare
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
from
September 17, 2026 22:15
61cfd3d to
ac29386
Compare
This comment has been minimized.
This comment has been minimized.
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
from
September 18, 2026 07:06
5f7ef97 to
2fbc79b
Compare
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
2 times, most recently
from
September 22, 2026 15:55
f535cb7 to
a0b9eb2
Compare
Contributor
Reliability & Chaos Results❌ 21 failure(s) detected Pipeline: https://gitlab.ddbuild.io/DataDog/java-profiler/-/pipelines/139231267 ❌ profiler tcmalloc amd64Xjit❌ profiler tcmalloc amd64Xmemory❌ profiler tracer gmalloc aarch64Xmemory❌ chaos: profiler gmalloc aarch64 21 0 3 temXchaos❌ chaos: profiler gmalloc aarch64 25 0 3 temXchaos❌ chaos: profiler gmalloc amd64 21 0 3 temXchaos❌ chaos: profiler gmalloc amd64 25 0 3 temXchaos❌ chaos: profiler jemalloc aarch64 25 0 3 temXchaos❌ chaos: profiler jemalloc amd64 21 0 3 temXchaos❌ chaos: profiler jemalloc amd64 25 0 3 temXchaos❌ chaos: profiler tcmalloc aarch64 21 0 3 temXchaos❌ chaos: profiler tcmalloc amd64 21 0 3 temXchaos❌ chaos: profiler tcmalloc amd64 25 0 3 temXchaos❌ chaos: profiler tracer gmalloc aarch64 21 0 3 temXchaos❌ chaos: profiler tracer gmalloc amd64 21 0 3 temXchaos❌ chaos: profiler tracer gmalloc amd64 25 0 3 temXchaos❌ chaos: profiler tracer jemalloc aarch64 21 0 3 temXchaos❌ chaos: profiler tracer jemalloc aarch64 25 0 3 temXchaos❌ chaos: profiler tracer jemalloc amd64 25 0 3 temXchaos❌ chaos: profiler tracer tcmalloc aarch64 21 0 3 temXchaos❌ chaos: profiler tracer tcmalloc amd64 25 0 3 temXchaos |
Introduces the ReferenceChainEvent/ReferenceChainAbandonedEvent payloads (event.h), their JFR metadata (jfrMetadata.*), and the FlightRecorder emission paths that serialize chain events into JFR recording buffers, including the constant-pool handling for per-hop edge labels. Emission is pull-style: profiler.cpp snapshots events and hands them to FlightRecorder; this layer does not depend on the tracker itself.
Review findings on the JFR plumbing layer: - MAX_REFERENCE_CHAIN_EVENT_HOPS was a fixed 4096, permitting a ~438 KB worst-case event (near-limit edge labels) against a ~61 KB recording buffer - the reservation flushed first but the margin underflowed, so the write ran past the buffer (debug assert, release corruption). The cap is now derived from RECORDING_BUFFER_LIMIT minus the event's fixed fields, divided by the per-hop worst case, so a full-cap event always fits. - Truncation dropped ALL edge labels: the label count was gated on _edges.size() == emitted_size, which only holds for untruncated chains. Labels align with the chain's leaf-first element order, so truncation now emits the first emitted_size labels and loses only the root-side ones. - ObjectLivenessEvent::leak_tag is default-initialized to 0 so any construction path that forgets to set it serializes a defined untagged value (flush_table() overwrites it from the entry, which track() zeroes at insert). Moves the JFR round-trip and arguments parsing unit tests into this layer (they test exactly this code), rewrites the round-trip test to construct events directly instead of through the tracker, and adds byte-level boundary tests: oversize-chain truncation with label preservation, the size-prefix invariant, and the default leak tag.
Uncommitted plan documents, rotted .cpp:NNN line references, and a nonexistent j9WallClock.cpp path replaced with symbol references that stay valid as the code moves.
- ReferenceChainEvent carries one vector of ReferenceChainHop (klass id + retention-edge label) instead of two parallel vectors - Compress the sub-option floor/ceiling rationale and the provisional default constant comments to one concise statement each - Drop design-doc and Jira references from code comments; revert the unrelated LineNumberTable comment rewrite
The event/argument comments named collector classes, methods and files that do not exist at this layer of the stack; describe the contracts without those forward references instead.
ReferenceChainTracker: per-klass class tags, the frontier table of retained references, the BFS expansion thread, chain resolution into per-sample ReferenceChainEvent payloads, and leak-tag correlation. LivenessTracker: the per-klass population table, heap-floor ring with time-to-OOM projection, and leak-candidate selection feeding the tracker's search gate. Adds the referencechains Arguments block, the string-dictionary generation counter used to invalidate the class-tag cache, and the container-memory/os queries the projection needs.
hopLabelClassFor() was the only GetObjectsWithTags() call site that did not Deallocate() the returned object/tag arrays - a per-cache-miss leak on the BFS thread's hop-label path (caught by the asan gtest run). Free both right after the class object is extracted, and null-guard the error path, matching the file's other call sites.
- collectStaticFieldAnchorsForRotation: the other tier is the last consumer of the anchor budget - stop accumulating its leftover back into budget_left (dead store). - buildCanaryChainEvent: capture the chain size before moving the vector into the event instead of reading the moved-from object for the log. - secondsToOOM: check ringThirdsStats()'s return for the time ring instead of reading time_stats uninitialized on its (unreachable-in- practice, but analyzer-visible) failure path - same head/fill/min-fill gate as the byte call, so it cannot trigger once the byte call passed.
- Leak-tag pool: start() reset the free list while the preserved tracking table still had tagged entries (the table survives stop()/start()), so a new object could receive a tag another live object owns and a later double release could write past the free list. Reclaim owned tags first and keep their correlation info. - track() published a reserved slot before initializing it while shared-mode scanners (tagLeakInstances, getLiveTraceIds) could read the uninitialized malloc storage; slots now carry a release/acquire ready flag and fresh table regions start unpublished. - secondsToOOM projects BOTH the heap and the container boundary and takes the shorter time instead of picking a ring by the raw limit comparison - container usage includes native memory and siblings, so a container with a numerically larger limit can still be closer to exhaustion. - cleanup_table() claimed the GC epoch before acquiring the table lock, so a newer epoch's fold could enter the population history before an older one's; the claim now happens under the lock (the pre-lock check remains as an advisory early exit). - threadLoop's urgency ramp multiplied the budget by four on every rounded pause-target change and never restored it; the boost now applies once per urgency episode and the configured budget is restored when it ends. - The terminal restart gate now charges the finished search's accumulated safepoint cost BEFORE checking affordability, so an expensive search no longer earns one free immediate successor (restartSearch() no longer spends it itself; the pain-budget test asserts the new order). - hopLabelClassFor() deleted cls twice on the superclass-walk path. - The class-shape reconciliation loop never deleted the class-object local refs GetObjectsWithTags() returned (BFS thread - pins classes against unload). - walkStaticFieldAnchors() early breaks left later anchors' local refs undeleted; a cleanup pass now releases them. - The static-field sweep's truncation cursor resumed by a visited-count index that assumes HotSpot's LIFO FollowReferences order; it now redoes the chunk, which is order-independent (shared code must not rely on HotSpot internals). - buildDiscoveredInstanceChains() treated a cache hit from an earlier search generation as current; the generation check now mirrors the representative-refresh paths. - cacheResolvedChain() reports success so coverage accounting (found bits, resolved counts) only advances for a chain that was actually stored. - os_linux: container usage is now read from the same cgroup level that supplied the selected limit (an ancestor limit covers sibling cgroups whose usage the leaf excludes). Moves referenceChains_ut.cpp and livenessTracker_ut.cpp into this layer - they test exactly this code, and the pain-budget ordering change requires its test to land with it.
A stale full scratch surviving klassPopulationResetForTest() lets the first post-reset GC fold fill the population table in one pass, making the synthetic-epoch seeded entry the permanent LRU-eviction victim - a fold landing mid-seeding (few-ms GC cadence on slow runners) resets the seeded ring and breaks the trend gate. Observed as the shouldSelectSeededKlassAsLeakCandidateOnPositiveSlope flake on musl-aarch64 (librca 21 and 11).
Comments pointing at locally-kept plan documents, rotted .cpp:NNN references, and a duplicated four-times comment block consolidated to the constant it documents.
fillHopEdgeLabels() fills hop edge labels in place; buildChainEvent() and the canary builder assemble ReferenceChainEvent::_hops from the parallel internal vectors.
No references to Java integration tests, stresstest repros, or uncommitted plan documents from this layer; the gtest files carry the comment cleanups with the code they test.
jbachorik
force-pushed
the
jb/rc-4-profiler-wiring
branch
from
September 25, 2026 10:19
a0b9eb2 to
cdea008
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?:
Wires the reference-chain engine into the profiler lifecycle:
Profilerstarts/stopsReferenceChainTrackerandLivenessTrackerwith the recording, drains resolved chain events into JFR ondump(), and emits abandoned-search events.ObjectSampleralways informsLivenessTrackerof the recording's flags so they cannot go stale across recordings.javaApi.cppadds the reference-chain natives and test seams;JavaProfiler.javadeclares them.vmEntryGC-event hooks feed the tracker.Motivation:
Part 4 of the stacked series for PROF-15341; makes the tracker from the previous PR reachable from a live recording.
Additional Notes:
Stacked on #797.
How to test the change?:
buildDebugcompiles and links; end-to-end behavior is covered by the Java integration tests later in the stack.For Datadog employees: