-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Erarndt/drain packet queue reorder fix #12190
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
Erarndt/drain packet queue reorder fix #12190
Conversation
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 addresses ARM64-specific failures by refactoring the node communication packet queue implementation to prevent instruction reordering issues and improve resource management. The changes replace thread-based packet draining with an async task-based approach and add explicit memory barriers.
Key changes:
- Refactored packet queue draining from dedicated thread to async Task-based implementation
- Added memory barriers to prevent instruction reordering on ARM64 architecture
- Improved resource management with proper using statements and disposal patterns
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs
Show resolved
Hide resolved
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs
Show resolved
Hide resolved
src/Build/BackEnd/Components/Communications/NodeProviderOutOfProcBase.cs
Show resolved
Hide resolved
rainersigwald
left a comment
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.
I'm happy to go in with this but let's let @YuliiaKovalova coordinate an isolated insertion so we can hopefully see more clearly if it's introducing problems.
YuliiaKovalova
left a comment
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.
now it's the best time to have it isolated :)
This reverts commit 533b6c5.
Reverts #12190 -- it reintroduced ARM64 failures, see https://dev.azure.com/devdiv/DevDiv/_workitems/edit/2533913/
Fixes #
Context
This PR addresses issues that were identified in this PR #11918 that were causing failures on ARM64.
Changes Made
Testing
Notes