Faster TraceBuffer for CRuby

[marcotc]

tl;dr: reduction of 17-93% of allocated bytes, 5-15% faster.

Leveraging the fact that the native Ruby Array "is thread-safe in practice because CRuby runs threads one at a time and does not do context switching during the execution of C functions", this PR implements a version of the TraceBuffer that does not use explicit locking.

The buffer is one of the tracer hot-stops, being a sync point between the application critical path and the tracer's worker thread. All traces will eventually be pushed into the buffer, so improvements to it affect all instrumentations.

This version works correctly on CRuby, but will not maintain the same guarantees under other runtimes, like JRuby. For this reason, we kept the existing implementation, which utilizes explicit locking, for non-CRuby environments.

The benchmarks below (also included in the PR) use the default buffer size of 1000 traces.
When pushing over 1000 traces into the buffer, our fair eviction policy will take place. The 2000 traces benchmark covers this case. Increasing the number of traces pushed even more yielded the same performance results, so we stop at 2000.

The reduction in memory usage is the most notable improvement, with memory usage being constant for the new implementation:

Before(ThreadSafeBuffer) [bytes allocated (objects created)]
    10 traces:   91840.00 (   1900.00)
   100 traces:  177376.00 (   1900.00)
  1000 traces: 1244200.00 (   1900.00)
  2000 traces: 1244200.00 (   1900.00)

After(CRubyTraceBuffer)  [bytes allocated (objects created)]
    10 traces:   76000.00 (   1900.00)
   100 traces:   76000.00 (   1900.00)
  1000 traces:   76000.00 (   1900.00)
  2000 traces:   76000.00 (   1900.00)

Comparison (% reduction, increase negative)
    10 traces:      17.25 (      0.00)
   100 traces:      57.15 (      0.00)
  1000 traces:      93.89 (      0.00)
  2000 traces:      93.89 (      0.00)

While wall time has had a modest improvement:

Before(ThreadSafeBuffer) [operations/sec]
    10 spans:   74676.66
   100 spans:   11465.74
  1000 spans:    1208.54
  2000 spans:     486.27

After(CRubyTraceBuffer)  [operations/sec]
    10 spans:   82571.16
   100 spans:   13084.07
  1000 spans:    1397.91
  2000 spans:     512.44

Comparison (% faster; slower if negative)
    10 spans:      10.57
   100 spans:      14.11
  1000 spans:      15.67
  2000 spans:       5.38

Process finished with exit code 0

[ericmustin]

I have a few small questions and nits, generally fine approving just want to be careful around the tag renaming stuff...this is great work, thanks @marcotc

[ericmustin]

Not a big deal but do we really want to add perf numbers in code comments? it's also a very broad range so i'm not sure how useful this comment is, maybe we can just say 'This implementation allocates significantly less memory and has modest speedup compared to Datadog::ThreadSafeBuffer' or basically, something that won't be used against us in the feature 😅

[marcotc]

I was trying to ensure I had supportive arguments documented, but hard numbers are sure to be variable across any execution environment.

I want to make sure to capture information that allows for future decision making regarding trade-offs being taken here.

I'm ambivalent in keeping or removing the numbers. What do you think @brettlangdon?

[brettlangdon]

I love the transparency, but this code comment could become stale pretty quickly.

I agree with Eric, going with a simpler comment to ensure people are considerate of performance impact whenever they modify the code is 👍🏻 and then we can think about how to expose this metric in a different way.

[marcotc]

Thank you guys, I removed hard numbers from the comment.
There's a link to benchmarks than can be run at any time to validate if the stated performance gains still hold in this comment block already, so I'm thinking that covers the hard-numbers part.

[ericmustin]

I'm still confused on the @buffer_accepted_lengths bit tbh but doesn't seem blocking to me, same for the code comment nit, so deferring to your best judgement on both. nice work!

The base branch was changed.

[ericmustin]

🚀