| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Scan-Build Report
Bug Summary
Reports
|
||||||||||||||||||||||||||||||||||||
Sorry, something went wrong.
CI Test ResultsRun: #36586975017 | Commit: 1b7b809 | Duration: 15m 22s (longest job)
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-09-29 15:36:15 UTC |
Sorry, something went wrong.
|
❌ 39 passed, 1 failed out of 40 configurations Test Matrix
Failure Detailsmusl-x64-hotspot-jdk21Tracer+profiler: " This may indicate tracer is not capturing any requests" "" "[7/8] Checking for unexpected events..." " ✓ No unexpected events found" "" "[8/8] Scenario-specific validation (ddprof_with_tracer)..." " Validating tracer+profiler scenario..." " ✓ Tracer+profiler scenario checks passed" "" "=== Validation Summary ===" "ExecutionSample: 0.0 events (datadog.ExecutionSample)" "Stack traces: 0.0 samples" "Thread diversity: 0.0 threads" "Allocation samples: 0 events (jdk.ObjectAllocationSample)" "ThreadAllocationStatistics: 52 events" "" "VALIDATION_FAILED: One or more checks did not pass" Script executed successfully: 425 commands VALIDATION_FAILED: Validation checks did not pass Links
|
Sorry, something went wrong.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Your 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. |
Sorry, something went wrong.
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: ab3b0ebbe1
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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".
Sorry, something went wrong.
There was a problem hiding this comment.
Normal liveness events can write an unknown leak tag. A long reference chain can also exceed the fixed JFR buffer and damage memory.
🤖 Datadog Autotest · Commit ab3b0eb · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Sorry, something went wrong.
The null-pointer store is the deliberate never-returning crash, but clang scan-build reports a null dereference. Under __clang_analyzer__, replace it with __builtin_trap() - the analyzer understands the trap as a halt and explores no false path past it; the real build keeps the store.
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.
There was a problem hiding this comment.
Nice change. I did a manual review, see below some findings.
Sorry, something went wrong.
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.
There was a problem hiding this comment.
🗿 🤖 🔴
Sphinx Review found 1 critical/high severity finding(s) that must be addressed.
Sorry, something went wrong.
- 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
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 2 Pipeline jobs failed
Useful? React with 👍 / 👎 This comment will be updated automatically if new data arrives.🔗 Commit SHA: d23c6b6 | Docs | View more details | Give us feedback! |
Sorry, something went wrong.
There was a problem hiding this comment.
Nice work, thanks! I've suggested 1 potential optimization and left 1 minor comment but nothing blocking for me.
Sorry, something went wrong.
@kaahos Thanks! I applied the suggestions - they were all valuable! |
Sorry, something went wrong.
Are you sure you pushed those fixes? I don't see them. |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good. You might have forgotten to push some fixes (see other comment).
Sorry, something went wrong.
|
@rkennke Yep. The signature window timed out and the push was just abandoned. Repushed. Thanks for pointing that out! |
Sorry, something went wrong.
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.
* Model crashNow's intentional crash as a trap under scan-build The null-pointer store is the deliberate never-returning crash, but clang scan-build reports a null dereference. Under __clang_analyzer__, replace it with __builtin_trap() - the analyzer understands the trap as a halt and explores no false path past it; the real build keeps the store. * Add JFR event types for reference chains 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. * Keep chain events inside the recording buffer; keep truncated labels 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. * Drop transient and stale line-number references from comments 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. * Address review: merge chain hops, trim comments, drop doc/jira refs - 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 * Make comments layer-local: no references to later stack layers 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. * Drop a later-layer class name from the arguments test comment * Clamp reference-chain edge labels to the reserved cap when writing * Reuse reservation constants and drop edge_labels stack array in recordReferenceChain
* Model crashNow's intentional crash as a trap under scan-build The null-pointer store is the deliberate never-returning crash, but clang scan-build reports a null dereference. Under __clang_analyzer__, replace it with __builtin_trap() - the analyzer understands the trap as a halt and explores no false path past it; the real build keeps the store. * Add JFR event types for reference chains 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. * Keep chain events inside the recording buffer; keep truncated labels 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. * Drop transient and stale line-number references from comments 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. * Address review: merge chain hops, trim comments, drop doc/jira refs - 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 * Make comments layer-local: no references to later stack layers 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. * Drop a later-layer class name from the arguments test comment * Clamp reference-chain edge labels to the reserved cap when writing * Reuse reservation constants and drop edge_labels stack array in recordReferenceChain
* Model crashNow's intentional crash as a trap under scan-build The null-pointer store is the deliberate never-returning crash, but clang scan-build reports a null dereference. Under __clang_analyzer__, replace it with __builtin_trap() - the analyzer understands the trap as a halt and explores no false path past it; the real build keeps the store. * Add JFR event types for reference chains 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. * Keep chain events inside the recording buffer; keep truncated labels 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. * Drop transient and stale line-number references from comments 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. * Address review: merge chain hops, trim comments, drop doc/jira refs - 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 * Make comments layer-local: no references to later stack layers 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. * Drop a later-layer class name from the arguments test comment * Clamp reference-chain edge labels to the reserved cap when writing * Reuse reservation constants and drop edge_labels stack array in recordReferenceChain
| Back | FazBrowse Home | New Git URL |
What does this PR do?:
Adds the JFR plumbing for reference-chain events: the ReferenceChainEvent / ReferenceChainAbandonedEvent payloads (event.h), their dynamic JFR metadata and constant-pool handling (jfrMetadata.*), the FlightRecorder emission paths that serialize chain events and per-hop edge labels into recording buffers (flightRecorder.*), and the referencechains Arguments block (arguments.*) that configures the feature.
Motivation:
Part 2 of the stacked series for reference chains (PROF-15341). Emission is pull-style — the later tracker layer snapshots events and hands them to FlightRecorder — so this layer is independently reviewable and compiles/links without the tracker.
Additional Notes:
Stacked series for PROF-15341. Merge order: #804 (scan-build crashNow fix) -> this PR -> #797 -> #798 -> #799 -> #800 -> #801 -> #802. #795 (UBSan ELF fix) is already merged. Until #804 lands, this PR's diff additionally shows the small faultInjection.cpp change from #804.
How to test the change?:
The JFR round-trip unit tests for chain events land in the gtest PR later in this stack; buildDebug compiles and links this layer standalone.
For Datadog employees: