-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Registry code cleanup had to be reverted to fix perf tests #7893
Comments
@adiaaida I assume this is because I stripped out reading perf keys? (Which isn't actually used in CoreLib directly.) Can you provide some details on the failure you were hitting? |
The failure mode is that when we look at the etl files generated (and subsequently, the any other files that are supposed to have perf data in them), they are empty. This is what @jorive found when he was debugging: "I was debugging the xUnit Performance API, and I noticed on the BenchmarkEventSource (inherits from EventSource), when calling the IsEnabled() method, it always return false." So basically, because of that, we don't get any of the event data when we run perf with your changes. |
A repro would be helpful- I might have messed up my changes to |
@danmosemsft what is the milestone for this? |
Marked it as future- it was opportunistic. |
- Ensure that the registry keys are always disposed - Use smaller subset of registry APIs - Reduce diffs with CoreCLR/CoreFX - Contributes to #11009 and #17899
- Ensure that the registry keys are always disposed - Use smaller subset of registry APIs - Reduce diffs with CoreCLR/CoreFX - Contributes to #11009 and #17899
Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process. This process is part of our issue cleanup automation. |
This issue will now be closed since it had been marked |
@JeremyKuhne
dotnet/coreclr#10983
Not sure whether this is necessary for 2.0 or not. Please set milestone to future if not.
@adiaaida supplied repro instructions offline, it seems not to obad so shouldn't take long to narrow down the problem in your change.
The text was updated successfully, but these errors were encountered: