-
Couldn't load subscription status.
- Fork 5.2k
[EventPipe][UserEvents] Deduplicate EventPipeEvent Metadata emission #117686
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
[EventPipe][UserEvents] Deduplicate EventPipeEvent Metadata emission #117686
Conversation
aef6cc3 to
2df5e09
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 deduplicates EventPipe event metadata per session by tracking metadata emission with a mask, and adds an atomic compare-and-exchange for int64 across runtimes.
- Introduce
metadata_written_maskand its update function onEventPipeEvent - Use
ep_rt_atomic_compare_exchange_int64_tto atomically set/reset metadata bits - Modify session and provider logic to emit metadata only once per session and reset on config changes
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/eventpipe/ep-session.c | Wrap metadata header emit in per-session mask check |
| src/native/eventpipe/ep-rt.h | Declare ep_rt_atomic_compare_exchange_int64_t |
| src/native/eventpipe/ep-provider.c | Pass session_mask to refresh functions and clear metadata mask |
| src/native/eventpipe/ep-event.h | Add metadata_written_mask field and update function declaration |
| src/native/eventpipe/ep-event.c | Initialize and implement ep_event_update_metadata_written_mask |
| src/mono/mono/eventpipe/ep-rt-mono.h | Implement ep_rt_atomic_compare_exchange_int64_t for Mono branch |
| src/coreclr/vm/eventing/eventpipe/ep-rt-coreclr.h | Implement ep_rt_atomic_compare_exchange_int64_t for CoreCLR branch |
| src/coreclr/nativeaot/Runtime/eventpipe/ep-rt-aot.h/.cpp | Implement ep_rt_atomic_compare_exchange_int64_t for NativeAOT branch |
Comments suppressed due to low confidence (2)
src/native/eventpipe/ep-session.c:830
- The metadata payload is written unconditionally, even when
should_write_metadatais false. You should wrap both the header and payload writes (includingio[i].iov_base,iov_len, andextension_lenupdates) inside theif (should_write_metadata)block to avoid emitting metadata twice or without a header.
io[io_index].iov_len = metadata_len;
src/native/eventpipe/ep-session.c:821
- Declaring
metadata_leninside theifblock makes it inaccessible when writing the payload below. This will cause a compilation error. Consider declaringmetadata_lenbefore theifso it can be reused for both header and payload writes (or move the payload write inside theif).
uint32_t metadata_len = ep_event_get_metadata_len (ep_event);
2df5e09 to
f7ec9a8
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.
LGTM!
Move metadata_written_mask bit flip to after writing the event Fix bytes_written operator precedence bug
|
/ba-g The windows-x64 NativeAOT lane failure should be #117795, the linux System.Net.HttpListener.Tests failure is asserting on the same assertion as #117795, the rest are test timeouts mostly seemingly due to machine inavailability, which shouldn't be impacted by this PRs changes as there are no user_events tests yet. |
Updating docs in response to dotnet/runtime#117686 and dotnet/runtime#118294
Follow-up to #115265
EventPipeEvent Metadata is typically emitted on the first time it's being written, a.k.a. the "Metadata Event". Typically, EventPipe Sessions would track the "Metadata Event" through this logic, leveraging single-threaded writers to track whether or not metadata has been written for that particular session.
For user_events-based EventPipe Sessions, events are written directly to the kernel's
user_events_datafile. As such, we need to keep track of whether or not Metadata has been transmitted for a particular EventPipeEvent for a particular EventPipe Session. To account for:This PR adds a
metadata_written_maskfield toEventPipeEvents to keep track of whether an Event has been written for a particular active session. In addition, for a lock-free and thread safe mechanism to update themetadata_written_mask,ep_rt_atomic_compare_exchange_int64_tis implemented.Testing
Manual testing with the same C program used in #117435 and #115265.
Metadata is only emitted once for each EventID 50 and 303, indicated by just 2 instances of
extension=01.Moreover, after disabling the session, the
EventPipeEventinstance'smetadata_written_maskbits had been reset accordingly.