Skip to content

Revert "A few fixes in the threadpool semaphore. Unify Windows/Unix implementation of LIFO policy."#125193

Open
agocke wants to merge 1 commit intomainfrom
revert-123921-lifo
Open

Revert "A few fixes in the threadpool semaphore. Unify Windows/Unix implementation of LIFO policy."#125193
agocke wants to merge 1 commit intomainfrom
revert-123921-lifo

Conversation

@agocke
Copy link
Member

@agocke agocke commented Mar 4, 2026

Reverts #123921

This change appears to have caused a large regression in NuGet restore performance. Reverting is confirmed to produce a significant improvement (10-15%).

@agocke agocke requested a review from MichalStrehovsky as a code owner March 4, 2026 21:54
Copilot AI review requested due to automatic review settings March 4, 2026 21:54
@agocke agocke requested review from jkotas and removed request for MichalStrehovsky March 4, 2026 21:54
@dotnet-policy-service
Copy link
Contributor

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

Copy link
Contributor

Copilot AI left a 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 reverts PR #123921 which had unified the Windows/Unix LIFO semaphore implementation and introduced futex-based thread blocking. The revert is motivated by a confirmed 10-15% regression in NuGet restore performance caused by the original change. The revert restores the original platform-specific LowLevelLifoSemaphore implementations (IO Completion Port on Windows, WaitSubsystem on Unix), removes the futex infrastructure (LowLevelFutex, LowLevelThreadBlocker), and reverts the spinning heuristics and Backoff.Exponential changes.

Changes:

  • Restores platform-specific LowLevelLifoSemaphore implementations (LowLevelLifoSemaphore.Windows.cs using IO Completion Ports, LowLevelLifoSemaphore.Unix.cs using WaitSubsystem) and removes the unified futex-based thread blocking infrastructure.
  • Reverts the thread pool semaphore spinning strategy from an adaptive per-core-availability approach back to a simple configurable spin count with random exponential backoff.
  • Removes the Synchronization.lib dependency and WaitOnAddress/WakeByAddressSingle Win32 API usage across managed and native code.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/native/libs/System.Native/pal_threading.h Removes futex function declarations
src/native/libs/System.Native/pal_threading.c Removes futex function implementations (Linux syscall + fallback)
src/native/libs/System.Native/entrypoints.c Removes futex DllImport entries
src/libraries/.../PortableThreadPool.WorkerThread.cs Reverts semaphore Wait call to 1-param, restores spin count config at call site
src/libraries/.../LowLevelThreadBlocker.cs Deletes unified thread blocker class
src/libraries/.../LowLevelLifoSemaphore.cs Reverts to platform-dispatching partial class with simpler spin logic
src/libraries/.../LowLevelLifoSemaphore.Windows.cs Restores IOCP-based Windows implementation
src/libraries/.../LowLevelLifoSemaphore.Unix.cs Restores WaitSubsystem-based Unix implementation
src/libraries/.../LowLevelFutex.Windows.cs Deletes Windows futex wrapper
src/libraries/.../LowLevelFutex.Unix.cs Deletes Unix futex wrapper
src/libraries/.../Backoff.cs Reverts to void return, removes attempt>0 guard, restores max bits to 14
src/libraries/.../System.Private.CoreLib.Shared.projitems Updates build item includes for restored/removed files
src/libraries/.../Interop.WaitOnAddress.cs Deletes WaitOnAddress/WakeByAddressSingle P/Invoke
src/libraries/.../Interop.CriticalSection.cs Removes [SuppressGCTransition] from LeaveCriticalSection
src/libraries/.../Interop.Libraries.cs Removes Synch library constant, adds duplicate Oleaut32
src/libraries/.../Interop.LowLevelMonitor.cs Removes [SuppressGCTransition] from LowLevelMonitor_Release
src/libraries/.../Interop.Futex.cs Deletes futex P/Invoke declarations
src/coreclr/.../reproNative.vcxproj Removes Synchronization.lib from SDK libs
src/coreclr/.../WindowsAPIs.txt Removes WaitOnAddress/WakeByAddressSingle API entries
src/coreclr/.../Microsoft.NETCore.Native.Windows.targets Removes Synchronization.lib
docs/coding-guidelines/interop-guidelines.md Reverts documentation examples to pre-#123921 state

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants