Skip to content

Use ThreadStatic fields over LocalDataStoreSlot for improved performance#9959

Open
h3xds1nz wants to merge 13 commits intodotnet:mainfrom
h3xds1nz:use-threadstatic-over-threadslot
Open

Use ThreadStatic fields over LocalDataStoreSlot for improved performance#9959
h3xds1nz wants to merge 13 commits intodotnet:mainfrom
h3xds1nz:use-threadstatic-over-threadslot

Conversation

@h3xds1nz
Copy link
Member

@h3xds1nz h3xds1nz commented Oct 15, 2024

Description

Replaces LocalDataStoreSlot with [ThreadStatic] fields for improved performance and increased type safety.

  • Static cost of initializing a LocalDataStoreSlot is about 500 ns and 184 B, that will forever live.
  • The retrieval process using Thread.GetData is about 40% more expensive.
  • There is no reason to use LocalDataStoreSlot these days (though it uses ThreadLocal<object?> under the hood since .NET Core 3.0 afaik so the performance is not THAT bad, while NetFX still goes with the original impl).

In the other commits, I've just cleaned up some CAS remnants (Critical/Unsecure methods/properties) from ComponentDispatcher, that just unnecessarily complicate the code these days. Since the formatting was different style on each, I've unified that.

10 property retrievals before/after (already initialized)

Method Mean [ns] Error [ns] StdDev [ns] Code Size [B] Allocated [B]
Original 25.15 ns 0.061 ns 0.054 ns 334 B -
PR_EDIT 14.07 ns 0.157 ns 0.139 ns 130 B -

Customer Impact

Improved performance, increased code base quality and type safety for developers.

Regression

No.

Testing

Local build, sample app run and testing with all the modified types.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz force-pushed the use-threadstatic-over-threadslot branch from e45283a to 5f642cb Compare April 4, 2025 09:23
@h3xds1nz
Copy link
Member Author

h3xds1nz commented Apr 4, 2025

Rebased and resolved merge conflicts.

@codecov
Copy link

codecov bot commented Apr 4, 2025

Codecov Report

Attention: Patch coverage is 48.27586% with 30 lines in your changes missing coverage. Please review.

Project coverage is 10.90597%. Comparing base (2ded801) to head (5f642cb).
Report is 4 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main       #9959         +/-   ##
===================================================
- Coverage   10.95887%   10.90597%   -0.05290%     
===================================================
  Files           3310        3310                 
  Lines         664667      664297        -370     
  Branches       74667       74664          -3     
===================================================
- Hits           72840       72448        -392     
- Misses        590685      590717         +32     
+ Partials        1142        1132         -10     
Flag Coverage Δ
Debug 10.90597% <48.27586%> (-0.05290%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

Community Contribution A label for all community Contributions Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants