Add a value type version of CircularBuffer

[reyang]

Related to #1315.

To make it easier, please review the 2nd commit which has the diff (the 1st commit is a direct copy of the existing CircularBuffer).

Changes

• Introduced a CircularBufferStruct for value types, which helps to avoid heap allocation.
• Each element in the circular buffer is a struct of a flag and the actual value, the flag is used to indicate whether the value has been fully written/ready.

For significant contributions please make sure you have completed the following items:

• CHANGELOG.md updated for non-trivial changes
• Design discussion issue #
• Changes in public API reviewed

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@874f9d7). Click here to learn what that means.
The diff coverage is 28.57%.

@@            Coverage Diff            @@
##             master    #1325   +/-   ##
=========================================
  Coverage          ?   76.54%           
=========================================
  Files             ?      224           
  Lines             ?     6471           
  Branches          ?        0           
=========================================
  Hits              ?     4953           
  Misses            ?     1518           
  Partials          ?        0           

Impacted Files	Coverage Δ
...s/StackExchangeRedisCallsInstrumentationOptions.cs	100.00% <ø> (ø)
src/OpenTelemetry/Internal/CircularBufferStruct.cs	0.00% <0.00%> (ø)
...Implementation/AspNetInstrumentationEventSource.cs	80.00% <40.00%> (ø)
...umentation.AspNet/Implementation/HttpInListener.cs	86.31% <40.00%> (ø)
...ementation/AspNetCoreInstrumentationEventSource.cs	80.00% <40.00%> (ø)
...tation.AspNetCore/Implementation/HttpInListener.cs	87.61% <40.00%> (ø)
...ent/Implementation/GrpcClientDiagnosticListener.cs	83.78% <40.00%> (ø)
...t/Implementation/GrpcInstrumentationEventSource.cs	62.50% <40.00%> (ø)
...tp/Implementation/HttpHandlerDiagnosticListener.cs	78.82% <40.00%> (ø)
...p/Implementation/HttpInstrumentationEventSource.cs	60.86% <40.00%> (ø)
... and 8 more

[CodeBlanch]

I wonder if it would help to use ref return here? Something like this...

public ref T Read()
{
   var value = this.trait[index].Value;
   this.trait[index].Value = default(T);
   this.trait[index].IsReady = false;
   this.tail++;
   return ref value;
}

If T is readonly struct, compiler might be smart enough to do that automatically.

For max perf this might also work...

public ref T Read()
{
   ref var value = ref this.trait[index].Value;
   // this.trait[index].Value = default(T); <- Not sure if this will clobber the ref or not.
   this.trait[index].IsReady = false;
   this.tail++;
   return ref value;
}

[reyang]

Once this.tail++ happened, other writer threads have the freedom to overwrite the struct value, which will cause race condition if the reader hasn't finished consuming the struct?

[CodeBlanch]

TBH, not sure. This is fringe stuff I only rarely get a chance to use 😄 I had a mistake in the first snippet, updated now. I removed a ref that shouldn't have been in there. The first snippet I think is safe. It makes a copy from the array, and then returns that copy by ref to the caller. At best, saves one copy. Second snippet might have a race. There are strict rules with ref locals, caller has to also use it as a local. So it will be extremely short-lived, but yes there could be an issue if the buffer loops around really quickly. Maybe we go as-is and then we can try to perf hack it while you continue on with the real effort?

[reyang]

I bet the 1st one is covered by NRV (named return value) optimization 😄

[CodeBlanch]

LGTM