-
Notifications
You must be signed in to change notification settings - Fork 319
Avoid SpanBuilder allocation for startSpan #9998
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
Conversation
Previous static extraction caused a test to fail because links logic relied on having a side effect on the builder instance's List<AgentSpanLink>
| /** Spans are built using this builder */ | ||
| public abstract static class CoreSpanBuilder implements AgentTracer.SpanBuilder { | ||
| protected static final boolean IGNORE_SCOPE_DEFAULT = false; | ||
| protected static final int SPAN_ID_DEFAULT = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe UNINITIALIZED_SPAN_ID or UNSET_SPAN_ID ?
Same suggestion for DEFAULT_TIMESTAMP_MICRO below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, that would be better. I'll make that change.
| final PropagationTags propagationTags; | ||
|
|
||
| if (this.spanId == 0) { | ||
| if (spanId == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we rename SPAN_ID_DEFAULT to UNSET_SPAN_ID then this could be
if (spanId == UNSET_SPAN_ID) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense, I added the constants at the end to make the long arguments easier to read, but it makes sense to use the constant here, too
mcculls
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor suggestions to improve readability
(also GitHub is reporting a conflict needs fixing in CoreTracer wrt. master)
Co-authored-by: Stuart McCulloch <stuart.mcculloch@datadoghq.com>
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 61 metrics, 4 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.108 s) : 0, 1107580
Total [baseline] (10.935 s) : 0, 10934689
Agent [candidate] (1.104 s) : 0, 1104475
Total [candidate] (10.777 s) : 0, 10777144
section appsec
Agent [baseline] (1.286 s) : 0, 1285986
Total [baseline] (11.174 s) : 0, 11173982
Agent [candidate] (1.292 s) : 0, 1291806
Total [candidate] (11.258 s) : 0, 11258466
section iast
Agent [baseline] (1.239 s) : 0, 1239467
Total [baseline] (11.234 s) : 0, 11233602
Agent [candidate] (1.244 s) : 0, 1243528
Total [candidate] (11.372 s) : 0, 11371852
section profiling
Agent [baseline] (1.232 s) : 0, 1231592
Total [baseline] (11.092 s) : 0, 11092102
Agent [candidate] (1.241 s) : 0, 1241007
Total [candidate] (11.057 s) : 0, 11057274
gantt
title petclinic - break down per module: candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.491 ms) : 0, 1491
crashtracking [candidate] (1.484 ms) : 0, 1484
BytebuddyAgent [baseline] (709.809 ms) : 0, 709809
BytebuddyAgent [candidate] (709.79 ms) : 0, 709790
GlobalTracer [baseline] (251.126 ms) : 0, 251126
GlobalTracer [candidate] (250.058 ms) : 0, 250058
AppSec [baseline] (32.433 ms) : 0, 32433
AppSec [candidate] (31.963 ms) : 0, 31963
Debugger [baseline] (65.024 ms) : 0, 65024
Debugger [candidate] (63.907 ms) : 0, 63907
Remote Config [baseline] (647.795 µs) : 0, 648
Remote Config [candidate] (628.188 µs) : 0, 628
Telemetry [baseline] (8.439 ms) : 0, 8439
Telemetry [candidate] (8.21 ms) : 0, 8210
Flare Poller [baseline] (3.818 ms) : 0, 3818
Flare Poller [candidate] (3.708 ms) : 0, 3708
section appsec
crashtracking [baseline] (1.49 ms) : 0, 1490
crashtracking [candidate] (1.491 ms) : 0, 1491
BytebuddyAgent [baseline] (734.241 ms) : 0, 734241
BytebuddyAgent [candidate] (738.139 ms) : 0, 738139
GlobalTracer [baseline] (241.509 ms) : 0, 241509
GlobalTracer [candidate] (242.704 ms) : 0, 242704
IAST [baseline] (24.795 ms) : 0, 24795
IAST [candidate] (24.953 ms) : 0, 24953
AppSec [baseline] (174.251 ms) : 0, 174251
AppSec [candidate] (174.83 ms) : 0, 174830
Debugger [baseline] (61.693 ms) : 0, 61693
Debugger [candidate] (61.903 ms) : 0, 61903
Remote Config [baseline] (673.08 µs) : 0, 673
Remote Config [candidate] (662.798 µs) : 0, 663
Telemetry [baseline] (8.467 ms) : 0, 8467
Telemetry [candidate] (8.214 ms) : 0, 8214
Flare Poller [baseline] (3.92 ms) : 0, 3920
Flare Poller [candidate] (3.91 ms) : 0, 3910
section iast
crashtracking [baseline] (1.489 ms) : 0, 1489
crashtracking [candidate] (1.488 ms) : 0, 1488
BytebuddyAgent [baseline] (832.311 ms) : 0, 832311
BytebuddyAgent [candidate] (833.489 ms) : 0, 833489
GlobalTracer [baseline] (237.35 ms) : 0, 237350
GlobalTracer [candidate] (238.756 ms) : 0, 238756
IAST [baseline] (26.652 ms) : 0, 26652
IAST [candidate] (26.914 ms) : 0, 26914
AppSec [baseline] (34.545 ms) : 0, 34545
AppSec [candidate] (34.894 ms) : 0, 34894
Debugger [baseline] (60.706 ms) : 0, 60706
Debugger [candidate] (61.209 ms) : 0, 61209
Remote Config [baseline] (532.689 µs) : 0, 533
Remote Config [candidate] (545.161 µs) : 0, 545
Telemetry [baseline] (7.574 ms) : 0, 7574
Telemetry [candidate] (7.715 ms) : 0, 7715
Flare Poller [baseline] (3.465 ms) : 0, 3465
Flare Poller [candidate] (3.471 ms) : 0, 3471
section profiling
crashtracking [baseline] (1.446 ms) : 0, 1446
crashtracking [candidate] (1.439 ms) : 0, 1439
BytebuddyAgent [baseline] (734.018 ms) : 0, 734018
BytebuddyAgent [candidate] (740.394 ms) : 0, 740394
GlobalTracer [baseline] (222.476 ms) : 0, 222476
GlobalTracer [candidate] (223.473 ms) : 0, 223473
AppSec [baseline] (32.276 ms) : 0, 32276
AppSec [candidate] (32.61 ms) : 0, 32610
Debugger [baseline] (62.986 ms) : 0, 62986
Debugger [candidate] (63.714 ms) : 0, 63714
Remote Config [baseline] (661.901 µs) : 0, 662
Remote Config [candidate] (652.837 µs) : 0, 653
Telemetry [baseline] (7.924 ms) : 0, 7924
Telemetry [candidate] (8.129 ms) : 0, 8129
Flare Poller [baseline] (3.924 ms) : 0, 3924
Flare Poller [candidate] (3.818 ms) : 0, 3818
ProfilingAgent [baseline] (96.86 ms) : 0, 96860
ProfilingAgent [candidate] (97.282 ms) : 0, 97282
Profiling [baseline] (97.453 ms) : 0, 97453
Profiling [candidate] (97.868 ms) : 0, 97868
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.114 s) : 0, 1114280
Total [baseline] (8.9 s) : 0, 8900406
Agent [candidate] (1.098 s) : 0, 1098173
Total [candidate] (8.819 s) : 0, 8819114
section iast
Agent [baseline] (1.24 s) : 0, 1239677
Total [baseline] (9.527 s) : 0, 9526701
Agent [candidate] (1.251 s) : 0, 1250669
Total [candidate] (9.548 s) : 0, 9548130
gantt
title insecure-bank - break down per module: candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.519 ms) : 0, 1519
crashtracking [candidate] (1.475 ms) : 0, 1475
BytebuddyAgent [baseline] (716.635 ms) : 0, 716635
BytebuddyAgent [candidate] (706.756 ms) : 0, 706756
GlobalTracer [baseline] (251.835 ms) : 0, 251835
GlobalTracer [candidate] (248.15 ms) : 0, 248150
AppSec [baseline] (32.59 ms) : 0, 32590
AppSec [candidate] (32.009 ms) : 0, 32009
Debugger [baseline] (63.739 ms) : 0, 63739
Debugger [candidate] (62.537 ms) : 0, 62537
Remote Config [baseline] (654.507 µs) : 0, 655
Remote Config [candidate] (618.06 µs) : 0, 618
Telemetry [baseline] (8.351 ms) : 0, 8351
Telemetry [candidate] (8.23 ms) : 0, 8230
Flare Poller [baseline] (3.817 ms) : 0, 3817
Flare Poller [candidate] (3.668 ms) : 0, 3668
section iast
crashtracking [baseline] (1.491 ms) : 0, 1491
crashtracking [candidate] (1.498 ms) : 0, 1498
BytebuddyAgent [baseline] (832.744 ms) : 0, 832744
BytebuddyAgent [candidate] (841.898 ms) : 0, 841898
GlobalTracer [baseline] (237.739 ms) : 0, 237739
GlobalTracer [candidate] (238.212 ms) : 0, 238212
IAST [baseline] (27.592 ms) : 0, 27592
IAST [candidate] (27.837 ms) : 0, 27837
AppSec [baseline] (33.663 ms) : 0, 33663
AppSec [candidate] (34.05 ms) : 0, 34050
Debugger [baseline] (59.85 ms) : 0, 59850
Debugger [candidate] (60.116 ms) : 0, 60116
Remote Config [baseline] (532.98 µs) : 0, 533
Remote Config [candidate] (552.302 µs) : 0, 552
Telemetry [baseline] (7.621 ms) : 0, 7621
Telemetry [candidate] (7.646 ms) : 0, 7646
Flare Poller [baseline] (3.495 ms) : 0, 3495
Flare Poller [candidate] (3.498 ms) : 0, 3498
LoadParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 18 metrics, 17 unstable metrics.
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section baseline
no_agent (18.18 ms) : 17994, 18365
. : milestone, 18180,
appsec (18.438 ms) : 18252, 18623
. : milestone, 18438,
code_origins (17.995 ms) : 17816, 18173
. : milestone, 17995,
iast (17.771 ms) : 17595, 17947
. : milestone, 17771,
profiling (19.777 ms) : 19576, 19977
. : milestone, 19777,
tracing (17.517 ms) : 17342, 17692
. : milestone, 17517,
section candidate
no_agent (19.282 ms) : 19086, 19479
. : milestone, 19282,
appsec (18.786 ms) : 18592, 18980
. : milestone, 18786,
code_origins (18.612 ms) : 18425, 18798
. : milestone, 18612,
iast (17.617 ms) : 17441, 17792
. : milestone, 17617,
profiling (18.676 ms) : 18486, 18865
. : milestone, 18676,
tracing (17.816 ms) : 17639, 17993
. : milestone, 17816,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section baseline
no_agent (1.224 ms) : 1212, 1236
. : milestone, 1224,
iast (3.342 ms) : 3296, 3387
. : milestone, 3342,
iast_FULL (6.044 ms) : 5984, 6105
. : milestone, 6044,
iast_GLOBAL (3.77 ms) : 3709, 3832
. : milestone, 3770,
profiling (2.155 ms) : 2134, 2177
. : milestone, 2155,
tracing (1.874 ms) : 1858, 1890
. : milestone, 1874,
section candidate
no_agent (1.203 ms) : 1191, 1215
. : milestone, 1203,
iast (3.295 ms) : 3251, 3338
. : milestone, 3295,
iast_FULL (6.152 ms) : 6089, 6215
. : milestone, 6152,
iast_GLOBAL (3.749 ms) : 3682, 3816
. : milestone, 3749,
profiling (2.063 ms) : 2043, 2083
. : milestone, 2063,
tracing (1.822 ms) : 1806, 1837
. : milestone, 1822,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 10 metrics, 2 unstable metrics. Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section baseline
no_agent (15.736 s) : 15736000, 15736000
. : milestone, 15736000,
appsec (14.828 s) : 14828000, 14828000
. : milestone, 14828000,
iast (18.165 s) : 18165000, 18165000
. : milestone, 18165000,
iast_GLOBAL (18.061 s) : 18061000, 18061000
. : milestone, 18061000,
profiling (14.575 s) : 14575000, 14575000
. : milestone, 14575000,
tracing (14.827 s) : 14827000, 14827000
. : milestone, 14827000,
section candidate
no_agent (15.256 s) : 15256000, 15256000
. : milestone, 15256000,
appsec (14.79 s) : 14790000, 14790000
. : milestone, 14790000,
iast (18.079 s) : 18079000, 18079000
. : milestone, 18079000,
iast_GLOBAL (18.199 s) : 18199000, 18199000
. : milestone, 18199000,
profiling (15.02 s) : 15020000, 15020000
. : milestone, 15020000,
tracing (14.633 s) : 14633000, 14633000
. : milestone, 14633000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.56.0-SNAPSHOT~0348766d62, baseline=1.57.0-SNAPSHOT~b65a3cefa6
dateFormat X
axisFormat %s
section baseline
no_agent (1.483 ms) : 1471, 1495
. : milestone, 1483,
appsec (2.455 ms) : 2403, 2507
. : milestone, 2455,
iast (2.212 ms) : 2148, 2276
. : milestone, 2212,
iast_GLOBAL (2.263 ms) : 2198, 2328
. : milestone, 2263,
profiling (2.064 ms) : 2012, 2116
. : milestone, 2064,
tracing (2.039 ms) : 1988, 2089
. : milestone, 2039,
section candidate
no_agent (1.482 ms) : 1470, 1493
. : milestone, 1482,
appsec (3.653 ms) : 3439, 3867
. : milestone, 3653,
iast (2.221 ms) : 2156, 2285
. : milestone, 2221,
iast_GLOBAL (2.26 ms) : 2195, 2325
. : milestone, 2260,
profiling (2.502 ms) : 2339, 2664
. : milestone, 2502,
tracing (2.055 ms) : 2004, 2106
. : milestone, 2055,
|
| spanId = this.spanId; | ||
| } | ||
|
|
||
| // Find the parent context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was moved into startSpan, since the links are needed to pass to DDSpan constructor.
Previously, this code was relying on having a side effect on links which was then picked up by startSpan later on.
| // Propagate internal trace. | ||
| // Note: if we are not in the context of distributed tracing, and we are starting the first | ||
| // root span, parentContext will be null at this point. | ||
| if (parentContext instanceof DDSpanContext) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to rename to resolvedParentContext to make the specific context more clear.
Yes, I'm in conflict with myself. I'll sort that out. |
…dd-trace-java into dougqh/span-builder-elimination
What Does This Do
This change avoids allocating a SpanBuilder in CoreTracer.startSpan methods
Motivation
SpanBuilder-s were one of the largest sources of allocation in the tracer
The prior SpanBuilder recycling eliminated much of the allocation, but relied on ThreadLocal caches which can actually increase allocation in virtual threads
By eliminating SpanBuilder-s altogether, almost all instrumentation called inside virtual threads now gets the same improvements that were already available to regular threads
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any useful labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]