Reuse SpanKind Entry in ClientDecorator#10503
Reuse SpanKind Entry in ClientDecorator#10503gh-worker-dd-mergequeue-cf854d[bot] merged 38 commits intomasterfrom
Conversation
EntryIterator and EntryChangeIterator are arguably redundant
Fixed bug TagValueConversions.toBoolean Could cause LegacyTagMap.EntryReader to produce incorrect answers to some queries For simplicity, now treating Byte and Short as Integer. That will make calling code doing primitive handling simpler. Fleshing out tests -- more tests to come
Coverage for byte, short, float, and double
Adding methods to AgentSpan / DDSpan that take TagMap.Entry/Reader objects directly This will enable TagMap.Entry reuse which can reduce memory allocation/GC pressure
Methods are intended to be used to create TagMap.Entry objects for repeatedly used values Overloads are provided for all the supported types to be easier for developers not familiar with TagMap internals. Internally, TagMap still uses the more explicit new<X>Entry methods.
Removing statics that were previously moved to TagValueConversions
- tests exposed missing TagMap.Entry.create for boolean - added explanatory strings to some asserts
…aDog/dd-trace-java into dougqh/fdirect-apis-for-tagmap-entry
...otstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java
Show resolved
Hide resolved
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 63 metrics, 8 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.066 s) : 0, 1065923
Total [baseline] (10.863 s) : 0, 10862693
Agent [candidate] (1.066 s) : 0, 1066143
Total [candidate] (10.85 s) : 0, 10850154
section appsec
Agent [baseline] (1.239 s) : 0, 1239402
Total [baseline] (11.179 s) : 0, 11178544
Agent [candidate] (1.241 s) : 0, 1240605
Total [candidate] (11.075 s) : 0, 11074672
section iast
Agent [baseline] (1.24 s) : 0, 1240046
Total [baseline] (11.283 s) : 0, 11283121
Agent [candidate] (1.234 s) : 0, 1233591
Total [candidate] (11.276 s) : 0, 11276210
section profiling
Agent [baseline] (1.197 s) : 0, 1196735
Total [baseline] (10.962 s) : 0, 10961898
Agent [candidate] (1.191 s) : 0, 1190863
Total [candidate] (10.926 s) : 0, 10926189
gantt
title petclinic - break down per module: candidate=1.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.193 ms) : 0, 1193
crashtracking [candidate] (1.183 ms) : 0, 1183
BytebuddyAgent [baseline] (628.501 ms) : 0, 628501
BytebuddyAgent [candidate] (628.266 ms) : 0, 628266
AgentMeter [baseline] (29.072 ms) : 0, 29072
AgentMeter [candidate] (29.05 ms) : 0, 29050
GlobalTracer [baseline] (257.707 ms) : 0, 257707
GlobalTracer [candidate] (257.548 ms) : 0, 257548
AppSec [baseline] (33.039 ms) : 0, 33039
AppSec [candidate] (32.962 ms) : 0, 32962
Debugger [baseline] (64.313 ms) : 0, 64313
Debugger [candidate] (64.475 ms) : 0, 64475
Remote Config [baseline] (629.098 µs) : 0, 629
Remote Config [candidate] (608.201 µs) : 0, 608
Telemetry [baseline] (9.268 ms) : 0, 9268
Telemetry [candidate] (10.72 ms) : 0, 10720
Flare Poller [baseline] (6.116 ms) : 0, 6116
Flare Poller [candidate] (5.341 ms) : 0, 5341
section appsec
crashtracking [baseline] (1.194 ms) : 0, 1194
crashtracking [candidate] (1.193 ms) : 0, 1193
BytebuddyAgent [baseline] (656.448 ms) : 0, 656448
BytebuddyAgent [candidate] (659.834 ms) : 0, 659834
AgentMeter [baseline] (11.939 ms) : 0, 11939
AgentMeter [candidate] (11.964 ms) : 0, 11964
GlobalTracer [baseline] (258.375 ms) : 0, 258375
GlobalTracer [candidate] (258.414 ms) : 0, 258414
IAST [baseline] (25.329 ms) : 0, 25329
IAST [candidate] (25.298 ms) : 0, 25298
AppSec [baseline] (168.514 ms) : 0, 168514
AppSec [candidate] (167.513 ms) : 0, 167513
Debugger [baseline] (67.45 ms) : 0, 67450
Debugger [candidate] (66.495 ms) : 0, 66495
Remote Config [baseline] (661.917 µs) : 0, 662
Remote Config [candidate] (636.688 µs) : 0, 637
Telemetry [baseline] (9.688 ms) : 0, 9688
Telemetry [candidate] (9.407 ms) : 0, 9407
Flare Poller [baseline] (3.8 ms) : 0, 3800
Flare Poller [candidate] (3.747 ms) : 0, 3747
section iast
crashtracking [baseline] (1.202 ms) : 0, 1202
crashtracking [candidate] (1.19 ms) : 0, 1190
BytebuddyAgent [baseline] (801.178 ms) : 0, 801178
BytebuddyAgent [candidate] (797.629 ms) : 0, 797629
AgentMeter [baseline] (11.361 ms) : 0, 11361
AgentMeter [candidate] (11.296 ms) : 0, 11296
GlobalTracer [baseline] (249.171 ms) : 0, 249171
GlobalTracer [candidate] (247.819 ms) : 0, 247819
IAST [baseline] (27.373 ms) : 0, 27373
IAST [candidate] (26.886 ms) : 0, 26886
AppSec [baseline] (35.903 ms) : 0, 35903
AppSec [candidate] (35.751 ms) : 0, 35751
Debugger [baseline] (65.082 ms) : 0, 65082
Debugger [candidate] (64.331 ms) : 0, 64331
Remote Config [baseline] (537.123 µs) : 0, 537
Remote Config [candidate] (547.732 µs) : 0, 548
Telemetry [baseline] (8.773 ms) : 0, 8773
Telemetry [candidate] (8.702 ms) : 0, 8702
Flare Poller [baseline] (3.46 ms) : 0, 3460
Flare Poller [candidate] (3.462 ms) : 0, 3462
section profiling
crashtracking [baseline] (1.196 ms) : 0, 1196
crashtracking [candidate] (1.178 ms) : 0, 1178
BytebuddyAgent [baseline] (687.711 ms) : 0, 687711
BytebuddyAgent [candidate] (681.852 ms) : 0, 681852
AgentMeter [baseline] (8.543 ms) : 0, 8543
AgentMeter [candidate] (8.56 ms) : 0, 8560
GlobalTracer [baseline] (216.064 ms) : 0, 216064
GlobalTracer [candidate] (216.06 ms) : 0, 216060
AppSec [baseline] (32.465 ms) : 0, 32465
AppSec [candidate] (32.691 ms) : 0, 32691
Debugger [baseline] (67.027 ms) : 0, 67027
Debugger [candidate] (67.33 ms) : 0, 67330
Remote Config [baseline] (639.449 µs) : 0, 639
Remote Config [candidate] (627.83 µs) : 0, 628
Telemetry [baseline] (9.187 ms) : 0, 9187
Telemetry [candidate] (9.005 ms) : 0, 9005
Flare Poller [baseline] (3.749 ms) : 0, 3749
Flare Poller [candidate] (3.835 ms) : 0, 3835
ProfilingAgent [baseline] (99.286 ms) : 0, 99286
ProfilingAgent [candidate] (99.194 ms) : 0, 99194
Profiling [baseline] (99.865 ms) : 0, 99865
Profiling [candidate] (99.77 ms) : 0, 99770
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.08 s) : 0, 1080357
Total [baseline] (8.792 s) : 0, 8792007
Agent [candidate] (1.064 s) : 0, 1064254
Total [candidate] (8.753 s) : 0, 8753199
section iast
Agent [baseline] (1.238 s) : 0, 1238191
Total [baseline] (9.422 s) : 0, 9421877
Agent [candidate] (1.233 s) : 0, 1232782
Total [candidate] (9.441 s) : 0, 9441099
gantt
title insecure-bank - break down per module: candidate=1.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section tracing
crashtracking [baseline] (1.209 ms) : 0, 1209
crashtracking [candidate] (1.202 ms) : 0, 1202
BytebuddyAgent [baseline] (637.714 ms) : 0, 637714
BytebuddyAgent [candidate] (627.885 ms) : 0, 627885
AgentMeter [baseline] (29.58 ms) : 0, 29580
AgentMeter [candidate] (29.085 ms) : 0, 29085
GlobalTracer [baseline] (260.811 ms) : 0, 260811
GlobalTracer [candidate] (257.457 ms) : 0, 257457
AppSec [baseline] (33.545 ms) : 0, 33545
AppSec [candidate] (32.687 ms) : 0, 32687
Debugger [baseline] (64.366 ms) : 0, 64366
Debugger [candidate] (64.776 ms) : 0, 64776
Remote Config [baseline] (624.689 µs) : 0, 625
Remote Config [candidate] (603.871 µs) : 0, 604
Telemetry [baseline] (11.436 ms) : 0, 11436
Telemetry [candidate] (9.813 ms) : 0, 9813
Flare Poller [baseline] (4.658 ms) : 0, 4658
Flare Poller [candidate] (4.564 ms) : 0, 4564
section iast
crashtracking [baseline] (1.214 ms) : 0, 1214
crashtracking [candidate] (1.194 ms) : 0, 1194
BytebuddyAgent [baseline] (800.79 ms) : 0, 800790
BytebuddyAgent [candidate] (796.549 ms) : 0, 796549
AgentMeter [baseline] (11.497 ms) : 0, 11497
AgentMeter [candidate] (11.28 ms) : 0, 11280
GlobalTracer [baseline] (248.271 ms) : 0, 248271
GlobalTracer [candidate] (247.836 ms) : 0, 247836
IAST [baseline] (27.311 ms) : 0, 27311
IAST [candidate] (27.003 ms) : 0, 27003
AppSec [baseline] (35.377 ms) : 0, 35377
AppSec [candidate] (34.104 ms) : 0, 34104
Debugger [baseline] (64.797 ms) : 0, 64797
Debugger [candidate] (66.108 ms) : 0, 66108
Remote Config [baseline] (556.126 µs) : 0, 556
Remote Config [candidate] (554.586 µs) : 0, 555
Telemetry [baseline] (8.764 ms) : 0, 8764
Telemetry [candidate] (8.743 ms) : 0, 8743
Flare Poller [baseline] (3.479 ms) : 0, 3479
Flare Poller [candidate] (3.454 ms) : 0, 3454
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 3 performance regressions! Performance is the same for 18 metrics, 15 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section baseline
no_agent (1.24 ms) : 1227, 1253
. : milestone, 1240,
iast (3.193 ms) : 3149, 3237
. : milestone, 3193,
iast_FULL (5.83 ms) : 5771, 5890
. : milestone, 5830,
iast_GLOBAL (3.438 ms) : 3391, 3486
. : milestone, 3438,
profiling (2.024 ms) : 2006, 2041
. : milestone, 2024,
tracing (1.787 ms) : 1773, 1801
. : milestone, 1787,
section candidate
no_agent (1.192 ms) : 1181, 1204
. : milestone, 1192,
iast (3.311 ms) : 3260, 3361
. : milestone, 3311,
iast_FULL (5.998 ms) : 5937, 6059
. : milestone, 5998,
iast_GLOBAL (3.623 ms) : 3569, 3677
. : milestone, 3623,
profiling (2.114 ms) : 2095, 2132
. : milestone, 2114,
tracing (1.818 ms) : 1802, 1834
. : milestone, 1818,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section baseline
no_agent (18.895 ms) : 18703, 19088
. : milestone, 18895,
appsec (19.118 ms) : 18923, 19314
. : milestone, 19118,
code_origins (17.656 ms) : 17479, 17833
. : milestone, 17656,
iast (17.593 ms) : 17416, 17770
. : milestone, 17593,
profiling (18.626 ms) : 18440, 18812
. : milestone, 18626,
tracing (17.681 ms) : 17504, 17858
. : milestone, 17681,
section candidate
no_agent (18.155 ms) : 17970, 18340
. : milestone, 18155,
appsec (19.731 ms) : 19525, 19937
. : milestone, 19731,
code_origins (17.292 ms) : 17122, 17462
. : milestone, 17292,
iast (17.971 ms) : 17791, 18151
. : milestone, 17971,
profiling (19.71 ms) : 19509, 19911
. : milestone, 19710,
tracing (17.873 ms) : 17695, 18051
. : milestone, 17873,
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.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section baseline
no_agent (1.47 ms) : 1458, 1481
. : milestone, 1470,
appsec (2.512 ms) : 2457, 2567
. : milestone, 2512,
iast (2.246 ms) : 2176, 2315
. : milestone, 2246,
iast_GLOBAL (2.295 ms) : 2225, 2365
. : milestone, 2295,
profiling (2.082 ms) : 2027, 2137
. : milestone, 2082,
tracing (2.055 ms) : 2001, 2108
. : milestone, 2055,
section candidate
no_agent (1.47 ms) : 1459, 1481
. : milestone, 1470,
appsec (3.78 ms) : 3559, 4002
. : milestone, 3780,
iast (2.255 ms) : 2186, 2324
. : milestone, 2255,
iast_GLOBAL (2.295 ms) : 2225, 2365
. : milestone, 2295,
profiling (2.09 ms) : 2034, 2146
. : milestone, 2090,
tracing (2.07 ms) : 2016, 2124
. : milestone, 2070,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.59.0-SNAPSHOT~e55536fe53, baseline=1.60.0-SNAPSHOT~8fbdc86698
dateFormat X
axisFormat %s
section baseline
no_agent (15.508 s) : 15508000, 15508000
. : milestone, 15508000,
appsec (14.942 s) : 14942000, 14942000
. : milestone, 14942000,
iast (18.148 s) : 18148000, 18148000
. : milestone, 18148000,
iast_GLOBAL (17.758 s) : 17758000, 17758000
. : milestone, 17758000,
profiling (14.894 s) : 14894000, 14894000
. : milestone, 14894000,
tracing (14.647 s) : 14647000, 14647000
. : milestone, 14647000,
section candidate
no_agent (15.46 s) : 15460000, 15460000
. : milestone, 15460000,
appsec (14.773 s) : 14773000, 14773000
. : milestone, 14773000,
iast (18.482 s) : 18482000, 18482000
. : milestone, 18482000,
iast_GLOBAL (17.9 s) : 17900000, 17900000
. : milestone, 17900000,
profiling (15.521 s) : 15521000, 15521000
. : milestone, 15521000,
tracing (14.626 s) : 14626000, 14626000
. : milestone, 14626000,
|
Some of this was a bad merge, but I probably missed some in my other PR, too
|
|
||
| @Test | ||
| public void objectEntry() { | ||
| public void newObjectEntry() { |
There was a problem hiding this comment.
This is clean-up of tests that I missed on the original PR.
I'm happy to separate this if that's what the reviewer would prefer.
internal-api/src/test/java/datadog/trace/api/TagMapEntryTest.java
Outdated
Show resolved
Hide resolved
...otstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java
Outdated
Show resolved
Hide resolved
| private final TagMap.Entry spanKindEntry() { | ||
| TagMap.Entry kindEntry = cachedSpanKindEntry; | ||
| if (kindEntry == null) { | ||
| cachedSpanKindEntry = kindEntry = TagMap.Entry.create(Tags.SPAN_KIND, spanKind()); |
There was a problem hiding this comment.
As a side note, this could have been done in the constructor if spanKind had been passed as a parameter instead of being defined as an overridable method.
There was a problem hiding this comment.
Yes, I would very much like to refactor the decorators to make such things possible.
For the moment, I'm just trying to do small isolated changes for quick wins.
But I do think we really need to revisit the decorators, I think are adding a lot of unnecessary overhead.
There was a problem hiding this comment.
That makes sense. I just realized that the current structure conceals invisible contracts, such as the requirement that spanKind not change.
There was a problem hiding this comment.
I tried the approach of doing a virtual call in the constructor, but that had other problems.
It created issues with static field initialization order. If the virtual method refers to a static and comes after the decorator singleton field, then the other static field is null during decorator construction.
So for now, I'm sticking with caching approach, but adding a comment to explain why.
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
What Does This Do
Caches & reuses SpanKind Entry in ClientDecorator
Motivation
Reduce allocation & garbage collection - improves throughput under load
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]