Skip to content
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

Enabling RuntimeEventSource in NativeAOT #85424

Merged
merged 11 commits into from
May 11, 2023

Conversation

LakshanF
Copy link
Contributor

@LakshanF LakshanF commented Apr 26, 2023

RuntimeEventSource wasn't enabled in NativeAOT due to not being initialized when EventSourceEnabled is set, missing metadata on critical payload types blocking reflection of these in EventSource.

Added a module initializer to Initialize RuntimeEventSource if EventSourceEnabled is set, made the required types visible to reflection, and also enabled some missing counters in NativeAOT.

@LakshanF LakshanF added this to the 8.0.0 milestone Apr 26, 2023
@LakshanF LakshanF self-assigned this Apr 26, 2023
@ghost
Copy link

ghost commented Apr 26, 2023

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

For a CI check

Author: LakshanF
Assignees: LakshanF
Labels:

area-NativeAOT-coreclr

Milestone: 8.0.0

@LakshanF LakshanF marked this pull request as draft April 26, 2023 22:17
@LakshanF LakshanF requested a review from davmason April 26, 2023 22:18
@LakshanF LakshanF marked this pull request as ready for review April 27, 2023 22:07
@LakshanF LakshanF force-pushed the EnableCountersInEventPipe branch from df98b45 to 9c204cd Compare May 9, 2023 21:02
@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 9, 2023
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label May 9, 2023
Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label May 10, 2023
…tion/Assembly.NativeAot.cs

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label May 10, 2023
Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

Comment on lines +120 to +122
// track count for metrics
if (isFirstFrame && !isFirstRethrowFrame)
Interlocked.Increment(ref s_exceptionCount);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to assume this matches how CoreCLR counts these wrt rethrow or ExceptionDispatchInfo.Throw.

@LakshanF LakshanF merged commit c48716c into dotnet:main May 11, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants