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

Support C# 10 'record struct' #3401

Merged
merged 1 commit into from Nov 10, 2021

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Nov 10, 2021

Fixes #3384
Fixes #3395

@codecov
Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #3401 (22d1715) into master (67850fe) will decrease coverage by 0.36%.
The diff coverage is 20.07%.

@@            Coverage Diff             @@
##           master    #3401      +/-   ##
==========================================
- Coverage   93.45%   93.09%   -0.37%     
==========================================
  Files        1030     1038       +8     
  Lines      111283   111809     +526     
  Branches     3946     3969      +23     
==========================================
+ Hits       104000   104089      +89     
- Misses       6267     6702     +435     
- Partials     1016     1018       +2     

@sharwell sharwell merged commit aacf8a5 into DotNetAnalyzers:master Nov 10, 2021
@sharwell sharwell deleted the record-struct branch November 10, 2021 18:45
@sharwell sharwell added this to the 1.2-beta.next milestone Nov 10, 2021
NickCraver added a commit to NickCraver/azure-functions-host that referenced this pull request Dec 1, 2021
Part of Azure#7908. This is a contention hot path under load for 2 reasons: allocations on each key access (start and end of each event) and the `Monitor.Wait()` around the update portion of `.AddOrUpdate()`. There's also a string replication which isn't used on the `Data` string itself, but that's upstream and will pitch it in another PR.

This change overall: optimizes the key access using a `readonly record struct` (for fast hashing), `Lazy<T>` (so we don't lose initial metrics), and moves the update outside of the dictionary path since each of the attributes on a metric are atomicly viable.

Note though, this breaks StyleCop sporadically - before this can go in we'd need to update StyleCop and pull in DotNetAnalyzers/StyleCopAnalyzers#3401 which will be in the next 1.2.x beta. I've pinged Sam about getting a release there we can use.
liliankasem pushed a commit to Azure/azure-functions-host that referenced this pull request Feb 24, 2023
Part of #7908. This is a contention hot path under load for 2 reasons: allocations on each key access (start and end of each event) and the `Monitor.Wait()` around the update portion of `.AddOrUpdate()`. There's also a string replication which isn't used on the `Data` string itself, but that's upstream and will pitch it in another PR.

This change overall: optimizes the key access using a `readonly record struct` (for fast hashing), `Lazy<T>` (so we don't lose initial metrics), and moves the update outside of the dictionary path since each of the attributes on a metric are atomicly viable.

Note though, this breaks StyleCop sporadically - before this can go in we'd need to update StyleCop and pull in DotNetAnalyzers/StyleCopAnalyzers#3401 which will be in the next 1.2.x beta. I've pinged Sam about getting a release there we can use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant