-
Notifications
You must be signed in to change notification settings - Fork 9
Implement 64-bit trace ID system with double-buffered storage and liveness tracking #262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
55f7fa6
to
98d82be
Compare
98d82be
to
0e22d49
Compare
0e22d49
to
0ff5e12
Compare
0ff5e12
to
233747e
Compare
233747e
to
32c8306
Compare
32c8306
to
ed00356
Compare
360fcb4
to
9bad2a2
Compare
9bad2a2
to
3957819
Compare
3957819
to
55050db
Compare
55050db
to
c96dc57
Compare
1 similar comment
da6a866
to
f36b049
Compare
1 similar comment
c314d52
to
53be30c
Compare
53be30c
to
71799ce
Compare
71799ce
to
06f6ddc
Compare
1 similar comment
bb1865b
to
f3cd240
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a comprehensive 64-bit trace ID system with double-buffered storage and liveness tracking for call traces. The changes enhance call trace management by adding liveness-aware preservation across JFR dumps, contention handling with visibility into dropped traces, and a modular hash table architecture with instance-based collision avoidance.
- Adds liveness-aware double-buffered storage with selective trace preservation
- Implements 64-bit trace ID system with instance-based collision avoidance
- Introduces contention handling with dropped trace visibility in JFR output
- Extracts dedicated CallTraceHashTable class for improved modularity
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
File | Description |
---|---|
LivenessTrackingTest.java | Comprehensive Java integration test for end-to-end liveness tracking validation |
TagContextTest.java | Enhanced test with dropped sample tracking and counter validation improvements |
ContendedCallTraceStorageTest.java | New test for measuring and validating contention in CallTraceStorage operations |
test_callTraceStorage.cpp | Extensive C++ unit tests covering liveness scenarios and concurrent operations |
wallClock.h/cpp | Updated copyright headers and 64-bit trace ID migration |
thread.h | Updated trace ID fields and methods to use 64-bit values |
profiler.h/cpp | Major refactoring for 64-bit trace IDs and liveness checker integration |
objectSampler.cpp | Updated copyright and 64-bit trace ID adoption |
livenessTracker.h/cpp | Enhanced with 64-bit trace IDs and self-registration with profiler |
flightRecorder.h/cpp | Updated for 64-bit trace ID support and improved trace processing |
counters.h | Added new counter for tracking dropped traces |
callTraceStorage.h/cpp | Major refactoring with double-buffering and liveness integration |
callTraceHashTable.h/cpp | New dedicated hash table implementation extracted from storage |
arch_dd.h | Added COMMA macro consolidation |
CLAUDE.md | New documentation file with project guidance and architecture details |
Comments suppressed due to low confidence (1)
ddprof-lib/src/main/cpp/callTraceStorage.cpp:125
- Copying the entire unordered_set could be expensive for large sets. Consider using std::move or passing the set by reference to avoid the copy overhead, especially since this is in the critical processTraces path.
preserve_set = _preserve_set; // Copy the set for lock-free processing
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ddprof-test/src/test/java/com/datadoghq/profiler/memleak/LivenessTrackingTest.java
Show resolved
Hide resolved
ddprof-test/src/test/java/com/datadoghq/profiler/ContendedCallTraceStorageTest.java
Outdated
Show resolved
Hide resolved
2 similar comments
Major architectural changes: - Replace monolithic CallTraceStorage with double-buffered hash table design - Add CallTraceHashTable with lock-free concurrent access and instance-based trace IDs - Implement liveness tracking system to preserve active traces across JFR dumps - Add dropped trace handling for lock contention with proper JFR integration Key features: - 64-bit trace IDs combining instance ID and slot for collision avoidance - Split-lock strategy minimizing exclusive lock time during trace collection - Platform-specific ASGCT_CallFrame alignment using LP64_ONLY macro - Comprehensive test coverage including contention and liveness scenarios 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…essTrackingTest.java Co-authored-by: Copilot <[email protected]>
…TraceStorageTest.java Co-authored-by: Copilot <[email protected]>
435eb08
to
2e8ab12
Compare
What does this PR do?:
This PR implements a liveness-aware double-buffered call trace storage system with several key improvements:
CallTraceHashTable
classMotivation:
The changes address critical issues in call trace management:
Additional Notes:
Key Features:
Liveness-Aware Storage:
processTraces()
methodContention Handling:
<dropped due to contention>
shown in JFR stack traces instead of null entriesDouble-Buffered Architecture:
(instance_id << 32) | slot
preventing collisionsHash Table Improvements:
CallTraceHashTable
class (441 lines)Implementation Details:
Core Storage Refactoring:
CallTraceStorage
reduced from 265→142 lines through hash table extraction64-bit Trace ID Migration:
recordJVMTISample()
,recordSample()
,recordDeferredSample()
LivenessTracker
for 64-bit trace ID handlingPlatform Compatibility:
arch_dd.h
for consistent designated initializer syntaxNew Files:
callTraceHashTable.{h,cpp}
- Dedicated hash table implementation (441 lines)test_callTraceStorage.cpp
- Comprehensive unit tests with liveness scenarios (387 lines)LivenessTrackingTest.java
- Java integration test for end-to-end validation (246 lines)ContendedCallTraceStorageTest.java
- Contention measurement and validation test (249 lines)Modified Files:
profiler.{h,cpp}
,objectSampler.cpp
,wallClock.{h,cpp}
- 64-bit trace ID adoptioncallTraceStorage.{h,cpp}
- major refactoring with liveness integrationflightRecorder.{h,cpp}
- 64-bit trace ID support and dropped trace handlinglivenessTracker.{h,cpp}
- 64-bit trace ID migrationarch_dd.h
- COMMA macro consolidationHow to test the change?:
The implementation includes extensive test coverage:
CallTraceStorage
liveness scenariosFor Datadog employees:
@DataDog/security-design-and-guidance
.Summary: +1689 lines, -447 lines (net +1239 lines)
This implementation provides a robust foundation for liveness-aware profiling with clear visibility into contention issues while maintaining high performance through lock-free operations and efficient storage management.