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

Make CreateManifestAndDescriptors a nop for NativeRuntimeEventSource #90416

Closed
wants to merge 1 commit into from

Conversation

stephentoub
Copy link
Member

#86850 did work to make manifest generation cheaper for NativeRuntimeEventSource, but I'm not clear on why the answer wasn't instead to just not do anything for manifest generation for NativeRuntimeEventSource, which is what this PR does.

On my machine, this lowers time-to-main by 33%, including removing all of the dynamic methods / JIT'ing work noted in #90405.

@tommcdon, @davmason, @brianrob, am I missing something?

@ghost
Copy link

ghost commented Aug 11, 2023

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details

#86850 did work to make manifest generation cheaper for NativeRuntimeEventSource, but I'm not clear on why the answer wasn't instead to just not do anything for manifest generation for NativeRuntimeEventSource, which is what this PR does.

On my machine, this lowers time-to-main by 33%, including removing all of the dynamic methods / JIT'ing work noted in #90405.

@tommcdon, @davmason, @brianrob, am I missing something?

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Diagnostics.Tracing

Milestone: -

@stephentoub
Copy link
Member Author

@brianrob informed me offline that this will break EventListeners. 😦

@ghost ghost locked as resolved and limited conversation to collaborators Sep 10, 2023
@stephentoub stephentoub deleted the nresmanifest branch January 8, 2024 15:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant