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

Prevent sending ETW within a lock #1146

Merged
merged 2 commits into from
Feb 7, 2023
Merged

Prevent sending ETW within a lock #1146

merged 2 commits into from
Feb 7, 2023

Conversation

lifengl
Copy link
Member

@lifengl lifengl commented Jan 16, 2023

This code amplifies ETW trace costs, and makes them into thread pool issues in some traces which leads to more noises in performance tests.

Lifeng Lu added 2 commits January 16, 2023 12:58
This code amplifies overhead of ETW tracing into thread pool issues, and increases noise in perf tests.
@lifengl lifengl requested a review from AArnott January 17, 2023 02:10
@drewnoakes
Copy link
Member

Could this end up with events appearing in incorrect order via ETW, and would that be an issue?

@lifengl
Copy link
Member Author

lifengl commented Jan 28, 2023

it looks like the optProf data is out of dated in this repo. Maybe OptProf run stops working in this repo? I will rerun it to see whether it can be resolved. @AArnott

@AArnott
Copy link
Member

AArnott commented Jan 28, 2023

@lifengl, this repo had been having optprof run problems recently, but I fixed them recently, and we've had a few insertions in the last week.
I think the failures you're seeing are all related to v16.10, which is still broken. But that shouldn't impact this PR or our main branch.

@lifengl
Copy link
Member Author

lifengl commented Feb 7, 2023

@lifengl, this repo had been having optprof run problems recently, but I fixed them recently, and we've had a few insertions in the last week. I think the failures you're seeing are all related to v16.10, which is still broken. But that shouldn't impact this PR or our main branch.

it somehow blocks this PR to be merged. Maybe this one is created earlier so builds in 17.6 are not excluded?

@AArnott
Copy link
Member

AArnott commented Feb 7, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AArnott
Copy link
Member

AArnott commented Feb 7, 2023

@lifengl now it isn't optprof, but loc that is breaking the official build. I have #1156 that I believe will resolve it.

@AArnott
Copy link
Member

AArnott commented Feb 7, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lifengl lifengl merged commit a31490c into main Feb 7, 2023
@lifengl lifengl deleted the dev/lifengl/noEtwInLock branch February 7, 2023 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants