-
Notifications
You must be signed in to change notification settings - 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_mask
and its update function onEventPipeEvent
- Use
ep_rt_atomic_compare_exchange_int64_t
to 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_metadata
is false. You should wrap both the header and payload writes (includingio[i].iov_base
,iov_len
, andextension_len
updates) 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_len
inside theif
block makes it inaccessible when writing the payload below. This will cause a compilation error. Consider declaringmetadata_len
before theif
so 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_data
file. 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_mask
field toEventPipeEvent
s 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_t
is 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
EventPipeEvent
instance'smetadata_written_mask
bits had been reset accordingly.