Use Volatile instead of Interlocked where appropriate

[pentp]

Volatile reads/writes are atomic and have acquire/release semantics, but are for the most part as fast as regular reads/writes. Any interlocked operation is at least 30-40 CPU cycles and needs exclusive cache line ownership, which is especially bad for reads.

Split off from #6048.

[cijothomas]

curious, if this shows improvement in the metric stress tests?

[rajkumar-rangaraj]

@pentp If there is a reasonable improvement in performance, I will spend time analyzing the complete metrics SDK to understand how these changes affect it.

[utpilla]

We need to consider the memory ordering guarantees of Volatile.Write. With Interlocked methods, the read/writes would not be moved before or after a given Interlocked method.

With volatile writes, read/writes that happen after a given Volatile.Write method can be moved before that Volatile.Write method. We need to evaluate if that affects the correctness of our code. There are some write operations that we do after releasing the locks (for exemplar and MetricPoint updates):

• Call OnCollected for Exemplars which resets the internal measurement state
• Update MetricStatus to CollectPending when updating MetricPoints

[pentp]

I went over all uses of Interlocked and also checked the code flow after these lock releases. I now found a few places where the current code incorrectly relies on the preceding interlocked operation for memory ordering, for example in case of MetricPoint.UpdateWithExemplar the order of operations is currently:

Interlocked.Exchange(ref this.runningValue.AsLong, number); // full fence
this.UpdateExemplar(number, tags, offerExemplar); // could run arbitrary lock-free code, though in practice uses locks
this.MetricPointStatus = MetricPointStatus.CollectPending; // no memory ordering guarantees, could become observable before exemplar updates

With this PR:

Volatile.Write(ref this.runningValue.AsLong, number); // release
this.UpdateExemplar(number, tags, offerExemplar);
Volatile.Write(ref this.status, (byte)MetricPointStatus.CollectPending); // release, guarantees all exemplar updates become observable before

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[codecov]

Codecov Report

Attention: Patch coverage is 86.20690% with 4 lines in your changes missing coverage. Please review.

Project coverage is 86.48%. Comparing base (8c1e638) to head (52c37b4).
Report is 11 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...Protocol/PersistentStorage/DirectorySizeTracker.cs	0.00%	2 Missing ⚠️
...c/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs	90.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6051      +/-   ##
==========================================
- Coverage   86.61%   86.48%   -0.13%     
==========================================
  Files         258      259       +1     
  Lines       11795    11880      +85     
==========================================
+ Hits        10216    10275      +59     
- Misses       1579     1605      +26     

Flag	Coverage Δ
unittests-Project-Experimental	86.47% <86.20%> (+0.01%)	⬆️
unittests-Project-Stable	86.14% <86.20%> (-0.29%)	⬇️

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

Files with missing lines	Coverage Δ
src/OpenTelemetry/Internal/InterlockedHelper.cs	100.00% <ø> (ø)
...lemetry/Internal/SelfDiagnosticsConfigRefresher.cs	86.53% <100.00%> (-0.38%)	⬇️
src/OpenTelemetry/Logs/LogRecord.cs	70.58% <100.00%> (ø)
src/OpenTelemetry/Metrics/Exemplar/Exemplar.cs	87.09% <100.00%> (ø)
...trics/Exemplar/SimpleFixedSizeExemplarReservoir.cs	75.00% <100.00%> (ø)
...trics/MetricPoint/MetricPointOptionalComponents.cs	100.00% <100.00%> (ø)
...Protocol/PersistentStorage/DirectorySizeTracker.cs	42.30% <0.00%> (ø)
...c/OpenTelemetry/Metrics/MetricPoint/MetricPoint.cs	94.42% <90.00%> (+0.18%)	⬆️

... and 4 files with indirect coverage changes

[pentp]

Using auto-layout together with the field type change for the two enums below reduces the struct size from 72 bytes to 64 bytes.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[github-actions]

This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or pushing will instruct the bot to automatically remove the label. This bot runs once per day.

[rajkumar-rangaraj]

This PR has been idle for quite some time and is currently adding to our maintenance overhead. I will go ahead and close it for now. When you are ready to address the follow-ups, please feel free to reopen the PR for review. Thank you for your contributions!