Reuse SpanBuilder within a Thread #9537
Conversation
Initial performance experiment The idea is to store a CoreSpanBuilder per thread, since usually only SpanBuilder is in use at a given time per thread -- and CoreSpanBuilder isn't thread safe This simple change provides a giant boost in small heaps Improving Spring petclinic throughput from -39% to -19% with 80m heap
|
Hi! 👋 Thanks for your pull request! 🎉 To help us review it, please make sure to:
If you need help, please check our contributing guidelines. |
dougqh
added
comp: core
|
🎯 Code Coverage 🔗 Commit SHA: e26959d | Docs | Was this helpful? Give us feedback! |
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 57 metrics, 8 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.015 s) : 0, 1015044
Total [baseline] (10.746 s) : 0, 10745925
Agent [candidate] (1.027 s) : 0, 1027337
Total [candidate] (10.787 s) : 0, 10787358
section appsec
Agent [baseline] (1.201 s) : 0, 1201204
Total [baseline] (11.126 s) : 0, 11126394
Agent [candidate] (1.194 s) : 0, 1194402
Total [candidate] (11.118 s) : 0, 11118193
section iast
Agent [baseline] (1.151 s) : 0, 1150762
Total [baseline] (10.997 s) : 0, 10997187
Agent [candidate] (1.156 s) : 0, 1155924
Total [candidate] (11.05 s) : 0, 11050035
section profiling
Agent [baseline] (1.174 s) : 0, 1174130
Total [baseline] (11.084 s) : 0, 11083988
Agent [candidate] (1.164 s) : 0, 1163885
Total [candidate] (11.009 s) : 0, 11009210
gantt
title petclinic - break down per module: candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.462 ms) : 0, 1462
crashtracking [candidate] (1.462 ms) : 0, 1462
BytebuddyAgent [baseline] (692.409 ms) : 0, 692409
BytebuddyAgent [candidate] (698.44 ms) : 0, 698440
GlobalTracer [baseline] (241.661 ms) : 0, 241661
GlobalTracer [candidate] (244.536 ms) : 0, 244536
AppSec [baseline] (32.868 ms) : 0, 32868
AppSec [candidate] (33.042 ms) : 0, 33042
Debugger [baseline] (6.436 ms) : 0, 6436
Debugger [candidate] (6.486 ms) : 0, 6486
Remote Config [baseline] (705.337 µs) : 0, 705
Remote Config [candidate] (706.981 µs) : 0, 707
Telemetry [baseline] (9.185 ms) : 0, 9185
Telemetry [candidate] (9.253 ms) : 0, 9253
Flare Poller [baseline] (9.024 ms) : 0, 9024
Flare Poller [candidate] (12.016 ms) : 0, 12016
section appsec
crashtracking [baseline] (1.472 ms) : 0, 1472
crashtracking [candidate] (1.453 ms) : 0, 1453
BytebuddyAgent [baseline] (721.685 ms) : 0, 721685
BytebuddyAgent [candidate] (717.106 ms) : 0, 717106
GlobalTracer [baseline] (236.014 ms) : 0, 236014
GlobalTracer [candidate] (235.493 ms) : 0, 235493
IAST [baseline] (24.935 ms) : 0, 24935
IAST [candidate] (24.777 ms) : 0, 24777
AppSec [baseline] (175.692 ms) : 0, 175692
AppSec [candidate] (175.362 ms) : 0, 175362
Debugger [baseline] (6.161 ms) : 0, 6161
Debugger [candidate] (6.036 ms) : 0, 6036
Remote Config [baseline] (663.139 µs) : 0, 663
Remote Config [candidate] (652.435 µs) : 0, 652
Telemetry [baseline] (8.572 ms) : 0, 8572
Telemetry [candidate] (8.485 ms) : 0, 8485
Flare Poller [baseline] (4.815 ms) : 0, 4815
Flare Poller [candidate] (3.94 ms) : 0, 3940
section iast
crashtracking [baseline] (1.47 ms) : 0, 1470
crashtracking [candidate] (1.472 ms) : 0, 1472
BytebuddyAgent [baseline] (814.62 ms) : 0, 814620
BytebuddyAgent [candidate] (818.778 ms) : 0, 818778
GlobalTracer [baseline] (232.1 ms) : 0, 232100
GlobalTracer [candidate] (232.921 ms) : 0, 232921
IAST [baseline] (26.246 ms) : 0, 26246
IAST [candidate] (26.41 ms) : 0, 26410
AppSec [baseline] (35.598 ms) : 0, 35598
AppSec [candidate] (35.605 ms) : 0, 35605
Debugger [baseline] (6.078 ms) : 0, 6078
Debugger [candidate] (6.089 ms) : 0, 6089
Remote Config [baseline] (604.187 µs) : 0, 604
Remote Config [candidate] (599.676 µs) : 0, 600
Telemetry [baseline] (8.548 ms) : 0, 8548
Telemetry [candidate] (8.499 ms) : 0, 8499
Flare Poller [baseline] (4.149 ms) : 0, 4149
Flare Poller [candidate] (4.181 ms) : 0, 4181
section profiling
crashtracking [baseline] (1.447 ms) : 0, 1447
crashtracking [candidate] (1.444 ms) : 0, 1444
BytebuddyAgent [baseline] (729.124 ms) : 0, 729124
BytebuddyAgent [candidate] (721.344 ms) : 0, 721344
GlobalTracer [baseline] (219.449 ms) : 0, 219449
GlobalTracer [candidate] (218.937 ms) : 0, 218937
AppSec [baseline] (33.541 ms) : 0, 33541
AppSec [candidate] (33.138 ms) : 0, 33138
Debugger [baseline] (8.079 ms) : 0, 8079
Debugger [candidate] (6.497 ms) : 0, 6497
Remote Config [baseline] (737.493 µs) : 0, 737
Remote Config [candidate] (1.48 ms) : 0, 1480
Telemetry [baseline] (15.077 ms) : 0, 15077
Telemetry [candidate] (15.681 ms) : 0, 15681
Flare Poller [baseline] (4.223 ms) : 0, 4223
Flare Poller [candidate] (4.214 ms) : 0, 4214
ProfilingAgent [baseline] (108.463 ms) : 0, 108463
ProfilingAgent [candidate] (108.649 ms) : 0, 108649
Profiling [baseline] (109.901 ms) : 0, 109901
Profiling [candidate] (109.225 ms) : 0, 109225
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.023 s) : 0, 1022658
Total [baseline] (8.712 s) : 0, 8712057
Agent [candidate] (1.018 s) : 0, 1018268
Total [candidate] (8.653 s) : 0, 8653154
section iast
Agent [baseline] (1.16 s) : 0, 1160375
Total [baseline] (9.286 s) : 0, 9285827
Agent [candidate] (1.152 s) : 0, 1151825
Total [candidate] (9.266 s) : 0, 9265918
gantt
title insecure-bank - break down per module: candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.477 ms) : 0, 1477
crashtracking [candidate] (1.456 ms) : 0, 1456
BytebuddyAgent [baseline] (697.856 ms) : 0, 697856
BytebuddyAgent [candidate] (692.808 ms) : 0, 692808
GlobalTracer [baseline] (243.478 ms) : 0, 243478
GlobalTracer [candidate] (242.604 ms) : 0, 242604
AppSec [baseline] (33.12 ms) : 0, 33120
AppSec [candidate] (32.777 ms) : 0, 32777
Debugger [baseline] (6.491 ms) : 0, 6491
Debugger [candidate] (6.417 ms) : 0, 6417
Remote Config [baseline] (721.993 µs) : 0, 722
Remote Config [candidate] (696.666 µs) : 0, 697
Telemetry [baseline] (9.286 ms) : 0, 9286
Telemetry [candidate] (9.272 ms) : 0, 9272
Flare Poller [baseline] (8.887 ms) : 0, 8887
Flare Poller [candidate] (11.093 ms) : 0, 11093
section iast
crashtracking [baseline] (1.467 ms) : 0, 1467
crashtracking [candidate] (1.449 ms) : 0, 1449
BytebuddyAgent [baseline] (822.014 ms) : 0, 822014
BytebuddyAgent [candidate] (815.847 ms) : 0, 815847
GlobalTracer [baseline] (233.126 ms) : 0, 233126
GlobalTracer [candidate] (232.503 ms) : 0, 232503
AppSec [baseline] (35.935 ms) : 0, 35935
AppSec [candidate] (35.088 ms) : 0, 35088
Debugger [baseline] (6.144 ms) : 0, 6144
Debugger [candidate] (6.101 ms) : 0, 6101
Remote Config [baseline] (620.225 µs) : 0, 620
Remote Config [candidate] (606.308 µs) : 0, 606
Telemetry [baseline] (8.802 ms) : 0, 8802
Telemetry [candidate] (8.511 ms) : 0, 8511
Flare Poller [baseline] (4.268 ms) : 0, 4268
Flare Poller [candidate] (4.16 ms) : 0, 4160
IAST [baseline] (26.632 ms) : 0, 26632
IAST [candidate] (26.292 ms) : 0, 26292
LoadParameters
See matching parameters
SummaryFound 2 performance improvements and 2 performance regressions! Performance is the same for 8 metrics, 12 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section baseline
no_agent (37.165 ms) : 36862, 37467
. : milestone, 37165,
appsec (48.093 ms) : 47660, 48525
. : milestone, 48093,
code_origins (46.409 ms) : 46006, 46812
. : milestone, 46409,
iast (46.0 ms) : 45598, 46401
. : milestone, 46000,
profiling (46.343 ms) : 45927, 46758
. : milestone, 46343,
tracing (45.421 ms) : 45023, 45820
. : milestone, 45421,
section candidate
no_agent (36.89 ms) : 36598, 37183
. : milestone, 36890,
appsec (47.601 ms) : 47172, 48030
. : milestone, 47601,
code_origins (46.61 ms) : 46196, 47023
. : milestone, 46610,
iast (45.954 ms) : 45563, 46345
. : milestone, 45954,
profiling (51.076 ms) : 50591, 51560
. : milestone, 51076,
tracing (44.01 ms) : 43640, 44381
. : milestone, 44010,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section baseline
no_agent (4.406 ms) : 4357, 4455
. : milestone, 4406,
iast (10.176 ms) : 10003, 10349
. : milestone, 10176,
iast_FULL (14.313 ms) : 14029, 14597
. : milestone, 14313,
iast_GLOBAL (10.894 ms) : 10701, 11088
. : milestone, 10894,
profiling (8.705 ms) : 8567, 8843
. : milestone, 8705,
tracing (7.795 ms) : 7686, 7905
. : milestone, 7795,
section candidate
no_agent (4.528 ms) : 4469, 4587
. : milestone, 4528,
iast (9.675 ms) : 9515, 9835
. : milestone, 9675,
iast_FULL (13.92 ms) : 13643, 14196
. : milestone, 13920,
iast_GLOBAL (11.09 ms) : 10889, 11291
. : milestone, 11090,
profiling (9.126 ms) : 8981, 9270
. : milestone, 9126,
tracing (7.778 ms) : 7669, 7888
. : milestone, 7778,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 1 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section baseline
no_agent (1.472 ms) : 1461, 1484
. : milestone, 1472,
appsec (3.715 ms) : 3497, 3933
. : milestone, 3715,
iast (2.212 ms) : 2148, 2275
. : milestone, 2212,
iast_GLOBAL (2.252 ms) : 2188, 2315
. : milestone, 2252,
profiling (2.037 ms) : 1986, 2087
. : milestone, 2037,
tracing (2.022 ms) : 1973, 2071
. : milestone, 2022,
section candidate
no_agent (1.475 ms) : 1464, 1487
. : milestone, 1475,
appsec (3.736 ms) : 3519, 3953
. : milestone, 3736,
iast (2.215 ms) : 2152, 2279
. : milestone, 2215,
iast_GLOBAL (2.237 ms) : 2173, 2300
. : milestone, 2237,
profiling (2.054 ms) : 2003, 2105
. : milestone, 2054,
tracing (2.025 ms) : 1975, 2074
. : milestone, 2025,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.54.0-SNAPSHOT~e26959db92, baseline=1.55.0-SNAPSHOT~054a9d5313
dateFormat X
axisFormat %s
section baseline
no_agent (14.948 s) : 14948000, 14948000
. : milestone, 14948000,
appsec (15.119 s) : 15119000, 15119000
. : milestone, 15119000,
iast (18.496 s) : 18496000, 18496000
. : milestone, 18496000,
iast_GLOBAL (18.058 s) : 18058000, 18058000
. : milestone, 18058000,
profiling (14.841 s) : 14841000, 14841000
. : milestone, 14841000,
tracing (15.317 s) : 15317000, 15317000
. : milestone, 15317000,
section candidate
no_agent (15.078 s) : 15078000, 15078000
. : milestone, 15078000,
appsec (14.797 s) : 14797000, 14797000
. : milestone, 14797000,
iast (18.379 s) : 18379000, 18379000
. : milestone, 18379000,
iast_GLOBAL (18.053 s) : 18053000, 18053000
. : milestone, 18053000,
profiling (15.738 s) : 15738000, 15738000
. : milestone, 15738000,
tracing (15.006 s) : 15006000, 15006000
. : milestone, 15006000,
|
| public static final String OPTIMIZED_MAP_ENABLED = "optimized.map.enabled"; | ||
| public static final String TAG_NAME_UTF8_CACHE_SIZE = "tag.name.utf8.cache.size"; | ||
| public static final String TAG_VALUE_UTF8_CACHE_SIZE = "tag.value.utf8.cache.size"; | ||
| public static final String SPAN_BUILDER_REUSE_ENABLED = "span.builder.reuse.enabled"; |
There was a problem hiding this comment.
I'm not sure if this is best class to be placing these in. Or if this is the proper namespace. dd.trace... might be better
There was a problem hiding this comment.
I agree dd.trace prefix fits better
There was a problem hiding this comment.
❔ question: Quick question for my understanding: how is that supposed to work with manual tracing API? (DD or OTel)
Let’s say:
var spanBuilder = tracer.spanBuilder("span1");
// …
var span = tracer.spanBuilder("span2").startSpan();
// …
spanBuilder.setAttribute("key", "value").startSpan(); The second startSpan() call will start the span with the wrong name ("span2" instead of "span1") and there is nothing we can do on the API to prevent such use case.
The solution that I put in place yesterday was that I track whether or not the SpanBuilder is "in-use". A SpanBuilder is considered "in -use" from the point that it is So So the 'in-use' check solves the case mentioned in the original comment. You'll still get two distinct SpanBuilders. But there are still other cases that worry me, specifically, I'm worried that you can actually call build on the SpanBuilder multiple times. That's not idiomatic, but it would probably work today. My solution doesn't handle that. I don't believe our own instrumentation produces multiple Spans per builder today, so we could solve the problem by having two slightly different sets of rules: one for automatic and one for manual. We can accomplish that by having two sets of methods: one set that will reuse SpanBuilders that we use in automatic instrumentation, and another set that always creates a new SpanBuilder that we use in manual instrumentation (and the OTel bridge). UPDATE: buildSpan can maintain the existing semantics including allowing building multiple spans. That does mean that we have to update some instrumentation to get the full benefit, but at least, we know the change is safe. |
Refactored code, so tests work regardless of Config
…ace-java into dougqh/spanbuilder-pooling
To avoid breaking any potential code that builds multiple spans from the same SpanBuilder, updated the SpanBuilder pooling approach Introduced a new method singleSpanBuilder which can build one and only one span, this method can be used by automatic instrumentation as an optimization. singleSpanBuilder is now used inside the startSpan convenience methods, since we know they only build and return one span. Any automatic instrumentation using startSpan gets the optimization for free. buildSpan maintains its original semantics, so all existing continues to work as is.
…ace-java into dougqh/spanbuilder-pooling
In a microbenchmark, buildSpan was performing worse than previously. To address, that shortcoming and to clean-up the code... Made CoreSpanBuilder abstract and introduced two child classes: MultiSpanBuilder and ReusableSingleSpanBuilder MultiSpanBuilder is used by buildSpan ReusableSingleSpanBuilder is used by singleSpanBuilder / startSpan (indirectly)
…ace-java into dougqh/spanbuilder-pooling
The current code does try to always reuse the SpanBuilder, it just safe guards against the case where the prior caller hasn't finished with it. However, the assert won't be completely reliable. If the SpanBuilder gets reset before the next call to buildSpan, then I cannot detect that. Or at least, I don't think I can detect it without adding a wrapper object (which is counter to the aim).
Yes, I considered that. I kind of like the explicitness of |
| private static final boolean SPAN_BUILDER_REUSE_ENABLED = | ||
| Config.get().isSpanBuilderReuseEnabled(); |
There was a problem hiding this comment.
I'm not super familiar with dd-java-agent configuration, just curious, is there a possibility for remote on-the-fly configuration? Or this flag can be applied only with restart? Just thinking out loud about usability.
There was a problem hiding this comment.
Not currently - although, that is something that we've discussed doing off and on
I'm honestly not expecting any customer to ever set this, so I'm going with the approach that works best for the JIT
| static final class ReusableSingleSpanBuilderThreadLocalCache | ||
| extends ThreadLocal<ReusableSingleSpanBuilder> { | ||
| private final CoreTracer tracer; |
There was a problem hiding this comment.
Just curious if it will be OK on latest JDKs with virtual threads?
There was a problem hiding this comment.
ThreadLocal will still work with virtual threads but the main issue is the related memory leak as we never remove entries. And CoreSpanBuilder isn't a small object:
https://docs.oracle.com/en/java/javase/21/core/virtual-threads.html#GUID-68216B85-7B43-423E-91BA-11489B1ACA61:~:text=the%20connection%20pool.-,Don%27t%20Cache%20Expensive%20Reusable%20Objects%20in%20Thread%2DLocal%20Variables,-Virtual%20threads%20support
Added an init instead of calling reset when creating a new ReusableSingleSpanBuilder Added some asserts to catch misuse of the API
Adding ThreadUtils class that enables checking if Threads are virtual threads
| @Override | ||
| public SpanBuilder singleSpanBuilder( | ||
| final String instrumentationName, final CharSequence spanName) { | ||
| return null; |
There was a problem hiding this comment.
question: wouldn't it be better to have a DUMB instance in cases like this. That might be my null fear speaking.
| // Used to track whether the ReusableSingleSpanBuilder is actively being used | ||
| // ReusableSingleSpanBuilder becomes "inUse" after a succesful reset and remains "inUse" | ||
| // until "buildSpan" is called | ||
| protected boolean inUse; |
There was a problem hiding this comment.
nitpick: protected visibility while class is final. Shouldn't it be package visible ?
| return buildSpan(DEFAULT_INSTRUMENTATION_NAME, spanName); | ||
| } | ||
|
|
||
| SpanBuilder buildSpan(String instrumentationName, CharSequence spanName); |
There was a problem hiding this comment.
nitpick: Might be useful to add the javadoc here as well. Or is singleSpanBuilder temporary to keep other instrumentations operational first with the current method ?
Or even distinguish between the span with or without childs ?
|
|
||
| /** | ||
| * Returns a SpanBuilder that can be used to produce one and only one span. By imposing the | ||
| * single span creation limitation, this method is more efficient than {@link buildSpan} |
There was a problem hiding this comment.
typo: For the link to work, a # is needed.
| * single span creation limitation, this method is more efficient than {@link buildSpan} | |
| * single span creation limitation, this method is more efficient than {@link #buildSpan} |
| // that case could result in permanently burning the cache for a given thread. | ||
|
|
||
| // That could be solved with additional logic during ReusableSingleSpanBuilder#buildSpan | ||
| // that checks to see if the cached the Builder is in use and then replaces it |
There was a problem hiding this comment.
typo:
| // that checks to see if the cached the Builder is in use and then replaces it | |
| // that checks to see if the cached Builder is in use and then replaces it |
| } | ||
| } | ||
|
|
||
| static final class MultiSpanBuilder extends CoreSpanBuilder { |
There was a problem hiding this comment.
suggestion: Some javadoc
/**
* A span builder that creates a new instance for each span.
*
* <p>This builder is used when span builder reuse is disabled or when building spans that may
* have multiple children. Each call to {@link #buildSpan()} creates a fresh span instance
* without any state reuse.
*/
static final class MultiSpanBuilder extends CoreSpanBuilder {
| } | ||
| } | ||
|
|
||
| static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { |
There was a problem hiding this comment.
suggestion: Some javadoc suggestions
| static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { | |
| /** | |
| * A reusable span builder optimized for building single spans with no children. | |
| * | |
| * <p>This builder is pooled in a thread-local cache to reduce allocation overhead for | |
| * high-frequency span creation. | |
| * | |
| * <p><b>Virtual thread handling:</b> Virtual threads do not use thread-local caching because | |
| * they are created and destroyed frequently. Instead, a new builder instance is allocated for | |
| * each span created on a virtual thread. | |
| * | |
| * @see CoreTracer#singleSpanBuilder(String, CharSequence) | |
| * @see ReusableSingleSpanBuilderThreadLocalCache | |
| */ | |
| static final class ReusableSingleSpanBuilder extends CoreSpanBuilder { |
| /** | ||
| * Resets the ReusableSingleSpanBuilder, so it may be used to build another single span Returns | ||
| * true if the reset was successful Returns false if this ReusableSingleSpanBuilder is still | ||
| * "in-use" | ||
| */ | ||
| final boolean reset(String instrumentationName, CharSequence operationName) { |
There was a problem hiding this comment.
suggestion:
| /** | |
| * Resets the ReusableSingleSpanBuilder, so it may be used to build another single span Returns | |
| * true if the reset was successful Returns false if this ReusableSingleSpanBuilder is still | |
| * "in-use" | |
| */ | |
| final boolean reset(String instrumentationName, CharSequence operationName) { | |
| /** | |
| * Resets the {@link ReusableSingleSpanBuilder}, so it may be used to build another single span | |
| * @returns <code>true</code> if the reset was successful, otherwise <code>false</code> | |
| * if this <code>ReusableSingleSpanBuilder</code> is still "in-use". | |
| */ |
|
|
||
| @Override | ||
| public CoreSpanBuilder ignoreActiveSpan() { | ||
| public final CoreSpanBuilder ignoreActiveSpan() { |
There was a problem hiding this comment.
nitpick: Just noted that some method are private, and may not need to be final, but I understand your perspective on anticipating changes here since the class is not final. Also it aligns with other methods. So feel free to dismiss :)
| this.instrumentationName = instrumentationName; | ||
| this.operationName = operationName; | ||
|
|
||
| if (this.tagLedger != null) this.tagLedger.reset(); | ||
| this.timestampMicro = 0L; | ||
| this.parent = null; | ||
| this.serviceName = null; | ||
| this.resourceName = null; | ||
| this.errorFlag = false; | ||
| this.spanType = null; | ||
| this.ignoreScope = false; | ||
| this.builderRequestContextDataAppSec = null; | ||
| this.builderRequestContextDataIast = null; | ||
| this.builderCiVisibilityContextData = null; | ||
| this.links = null; | ||
| this.spanId = 0L; | ||
|
|
There was a problem hiding this comment.
thought: Is there way to test nothing go forbidden here if CoreSpanBuilder gets a new field ? Maybe via some tests ?
What Does This Do
This changes reuses a SpanBuilder within a thread.
Motivation
SpanBuilders are one of the major causes of allocation within the client library, but typically only one SpanBuilder is needed at a time in a thread. By reseting and reusing the same SpanBuilder, tracing allocation can be reduced significantly.
By reducing allocation, the garbage collector needs to run less frequently improving the maximum sustainable throughput of the host application. In local tests with Spring Petclinic, this reduces the throughput penalty with tiny heaps (<= 80m) significantly from -40% to -20%. On larger heaps, the gain is more modest, since garbage collector has less of an impact on the application.
In this initial version, the thread local cache isn't used in virtual threads. This restriction exists to avoid creating more memory churn than we save, since virtual threads are created more often than regular threads.