Remove Unsafe.As from ThreadInt64PersistentCounter by using strongly-typed ThreadLocalNode #121434
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…alNode Replace object-based API with strongly-typed ThreadLocalNode in ThreadInt64PersistentCounter, removing all Unsafe.As casts. This change propagates through all ThreadPool implementations (Windows, Unix, Portable, Browser, WASI). Changes: - Make ThreadLocalNode internal sealed (was private sealed) - Update Increment/Decrement/Add methods to accept ThreadLocalNode directly - Change CreateThreadLocalCountObject() return type to ThreadLocalNode - Update all call sites to use typed node instead of object - Remove Debug.Assert and Unsafe.As usage from static methods This improves code clarity, type safety, and should provide minor performance improvements by eliminating type checks and casts. Co-authored-by: agocke <[email protected]>
Copilot
AI
changed the title
[WIP] Remove unnecessary uses of Unsafe.As in ThreadInt64PersistentCounter
Remove Unsafe.As from ThreadInt64PersistentCounter by using strongly-typed ThreadLocalNode
Nov 7, 2025
agocke
reviewed
Nov 7, 2025
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadInt64PersistentCounter.cs
Outdated
Show resolved
Hide resolved
Remove Increment/Decrement/Add static methods and call instance methods directly at all call sites. This simplifies the API since the static methods were just pass-through wrappers. Co-authored-by: agocke <[email protected]>
agocke
approved these changes
Nov 7, 2025
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the thread pool's completion counter implementation to use strongly-typed ThreadInt64PersistentCounter.ThreadLocalNode references instead of weakly-typed object? references, eliminating unnecessary boxing and unsafe casts.
- Removed static helper methods (
Increment,Decrement,Add) fromThreadInt64PersistentCounterthat performed unsafe casts - Changed
ThreadLocalNodevisibility fromprivatetointernalto enable direct usage - Updated all thread pool implementations across platforms to use strongly-typed references
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Threading/ThreadInt64PersistentCounter.cs | Removed static helper methods that took object parameters and unsafe casts; changed ThreadLocalNode to internal; updated return type of CreateThreadLocalCountObject() |
| src/libraries/System.Private.CoreLib/src/System/Threading/WindowsThreadPool.cs | Updated field, method parameters, and return types to use ThreadLocalNode instead of object?; replaced static helper calls with instance method calls |
| src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.cs | Updated field, method parameters, and return types to use ThreadLocalNode instead of object?; replaced static helper calls with instance method calls |
| src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs | Updated local variables, field types, and method calls to use ThreadLocalNode and call instance methods directly |
| src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Windows.cs | Updated method signatures and conditional forwarding to use strongly-typed ThreadLocalNode |
| src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Unix.cs | Updated method signatures and forwarding to use strongly-typed ThreadLocalNode |
| src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Wasi.cs | Updated method signatures in stub implementation to use strongly-typed ThreadLocalNode? |
| src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Browser.cs | Updated method signatures in stub implementation to use strongly-typed ThreadLocalNode? |
This was referenced Nov 7, 2025
Open
EgorBo
approved these changes
Nov 7, 2025
eduardo-vp
approved these changes
Nov 7, 2025
This was referenced Nov 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The goal here is to remove some uses of Unsafe.As in ThreadInt64PersistentCounter. All of these uses were due to the type of the ThreadLocalNode being private and the return type being
object. By making the type internal we can remove the need for casts entirely.