Record a high resolution timestamp for each allocation - #965
Closed
godlygeek wants to merge 6 commits into
Closed
Conversation
Record monotonic timestamps for allocation and deallocation events in all-allocation capture files, surface the data through the reader metadata and Python API, and add a CLI flag to enable the feature. The CLI rejects --allocation-timestamps with aggregated captures since that format does not preserve per-event ordering. Signed-off-by: Pablo Galindo Salgado <pablogsal@gmail.com>
Vacates record-type value 2 so that FRAME_PUSH can be relocated there in a following change. Value 4 was previously unused, and the writer and reader refer to the enumerator symbolically, so this is a pure renumbering with no behavioral change. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
FRAME_PUSH only ever used a single flag bit (is_entry_frame), so it does not need the 64-127 range with 6 flag bits. Relocate it to the 2-3 range (1 flag bit) to free up 64-127 for a future record type. The reader can no longer identify FRAME_PUSH by just a high bit, so extractRecordTypeAndFlags now matches it with an exact (value & ~1) == 2 check, placed after the higher-bit record types. Comments describing the encoding are updated accordingly. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Instead of appending a delta-encoded timestamp to every allocation record, the streaming writer now emits a separate CLOCK_ADVANCED record whenever an allocation's timestamp is higher than the last one emitted. The reader accumulates these advances into a running clock and stamps each subsequent allocation with the clock's current value. CLOCK_ADVANCED occupies record-type values 64-127 (freed up by the earlier FRAME_PUSH relocation). The 6 flag bits carry the advance in microseconds: values 1-63 encode the advance directly with no extra bytes (the common case), while 0 is a sentinel indicating the advance follows as an unsigned varint. Allocations that share a microsecond now cost zero timestamp bytes, and small advances cost a single byte. The Allocation/AllocationRecord structures are unchanged. Signed-off-by: Matt Wozniski <mwozniski@bloomberg.net>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #965 +/- ##
==========================================
+ Coverage 92.22% 92.23% +0.01%
==========================================
Files 99 99
Lines 12556 12651 +95
Branches 432 434 +2
==========================================
+ Hits 11580 11669 +89
- Misses 976 982 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
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.
This was originally included in #899, but was dropped when we realized these timestamps weren't actually necessary for the report we wanted to generate. Since we had already put in all the work to get the timestamp recording into a production-worthy state, I'm putting up this PR so that we can resurrect the timestamps if we ever do need them.