Skip to content
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

Add specific support for Optional #6406

Merged
merged 2 commits into from
Dec 27, 2023
Merged

Add specific support for Optional #6406

merged 2 commits into from
Dec 27, 2023

Conversation

jpbempel
Copy link
Member

@jpbempel jpbempel commented Dec 20, 2023

What Does This Do

We access directly to the value of an Optional through the java.util.Optional API rather than using the reflection as since JDK 16 it will be prohibited.

Motivation

Avoid using reflection on some JDK classes that is prohibited since JDK 16

Additional Notes

Jira ticket: DEBUG-1980

We access directly to the value of an Optional through the
`java.util.Optional` API rather than using the reflection as since
JDK 16 it will be prohibited.
@jpbempel jpbempel requested a review from a team as a code owner December 20, 2023 16:26
@jpbempel jpbempel requested review from shatzi and removed request for a team December 20, 2023 16:26
@jpbempel jpbempel added comp: debugger Dynamic Instrumentation type: enhancement labels Dec 20, 2023
@pr-commenter
Copy link

pr-commenter bot commented Dec 20, 2023

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master jpbempel/optional
git_commit_date 1703258917 1703263358
git_commit_sha 691d820 1761501
release_version 1.27.0-SNAPSHOT~691d820a0b 1.27.0-SNAPSHOT~1761501028
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1703266068 1703266068
ci_job_id 396043577 396043577
ci_pipeline_id 25575282 25575282
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
module Agent Agent
parent None None
variant iast iast

Summary

Found 0 performance improvements and 1 performance regressions! Performance is the same for 44 metrics, 9 unstable metrics.

scenario Δ mean execution_time candidate mean execution_time baseline mean execution_time
scenario:startup:petclinic:iast:Remote Config worse
[+29.212µs; +75.905µs] or [+5.240%; +13.615%]
610.066µs 557.507µs
Startup time reports for petclinic
gantt
    title petclinic - global startup overhead: candidate=1.27.0-SNAPSHOT~1761501028, baseline=1.27.0-SNAPSHOT~691d820a0b

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.057 s) : 0, 1056536
Total [baseline] (9.346 s) : 0, 9346455
Agent [candidate] (1.047 s) : 0, 1046941
Total [candidate] (9.431 s) : 0, 9431123
section appsec
Agent [baseline] (1.151 s) : 0, 1150888
Total [baseline] (9.393 s) : 0, 9392628
Agent [candidate] (1.145 s) : 0, 1145017
Total [candidate] (9.425 s) : 0, 9425184
section iast
Agent [baseline] (1.168 s) : 0, 1168271
Total [baseline] (9.523 s) : 0, 9522587
Agent [candidate] (1.175 s) : 0, 1175281
Total [candidate] (9.596 s) : 0, 9596061
section profiling
Agent [baseline] (1.25 s) : 0, 1249877
Total [baseline] (9.566 s) : 0, 9566021
Agent [candidate] (1.245 s) : 0, 1244872
Total [candidate] (9.635 s) : 0, 9635036
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.057 s -
Agent appsec 1.151 s 94.352 ms (8.9%)
Agent iast 1.168 s 111.735 ms (10.6%)
Agent profiling 1.25 s 193.341 ms (18.3%)
Total tracing 9.346 s -
Total appsec 9.393 s 46.173 ms (0.5%)
Total iast 9.523 s 176.131 ms (1.9%)
Total profiling 9.566 s 219.565 ms (2.3%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.047 s -
Agent appsec 1.145 s 98.077 ms (9.4%)
Agent iast 1.175 s 128.34 ms (12.3%)
Agent profiling 1.245 s 197.931 ms (18.9%)
Total tracing 9.431 s -
Total appsec 9.425 s -5.939 ms (-0.1%)
Total iast 9.596 s 164.938 ms (1.7%)
Total profiling 9.635 s 203.913 ms (2.2%)
gantt
    title petclinic - break down per module: candidate=1.27.0-SNAPSHOT~1761501028, baseline=1.27.0-SNAPSHOT~691d820a0b

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (654.641 ms) : 0, 654641
BytebuddyAgent [candidate] (647.276 ms) : 0, 647276
GlobalTracer [baseline] (308.307 ms) : 0, 308307
GlobalTracer [candidate] (306.775 ms) : 0, 306775
AppSec [baseline] (50.986 ms) : 0, 50986
AppSec [candidate] (50.837 ms) : 0, 50837
Remote Config [baseline] (671.785 µs) : 0, 672
Remote Config [candidate] (660.54 µs) : 0, 661
Telemetry [baseline] (7.343 ms) : 0, 7343
Telemetry [candidate] (7.16 ms) : 0, 7160
section appsec
BytebuddyAgent [baseline] (652.799 ms) : 0, 652799
BytebuddyAgent [candidate] (648.049 ms) : 0, 648049
GlobalTracer [baseline] (307.307 ms) : 0, 307307
GlobalTracer [candidate] (306.683 ms) : 0, 306683
AppSec [baseline] (148.757 ms) : 0, 148757
AppSec [candidate] (148.496 ms) : 0, 148496
Remote Config [baseline] (644.398 µs) : 0, 644
Remote Config [candidate] (640.195 µs) : 0, 640
Telemetry [baseline] (6.96 ms) : 0, 6960
Telemetry [candidate] (6.885 ms) : 0, 6885
section iast
BytebuddyAgent [baseline] (770.293 ms) : 0, 770293
BytebuddyAgent [candidate] (774.834 ms) : 0, 774834
GlobalTracer [baseline] (285.088 ms) : 0, 285088
GlobalTracer [candidate] (287.434 ms) : 0, 287434
AppSec [baseline] (52.292 ms) : 0, 52292
AppSec [candidate] (49.286 ms) : 0, 49286
IAST [baseline] (18.453 ms) : 0, 18453
IAST [candidate] (21.361 ms) : 0, 21361
Remote Config [baseline] (557.507 µs) : 0, 558
Remote Config [candidate] (610.066 µs) : 0, 610
Telemetry [baseline] (7.324 ms) : 0, 7324
Telemetry [candidate] (7.226 ms) : 0, 7226
section profiling
BytebuddyAgent [baseline] (662.286 ms) : 0, 662286
BytebuddyAgent [candidate] (658.153 ms) : 0, 658153
GlobalTracer [baseline] (378.012 ms) : 0, 378012
GlobalTracer [candidate] (376.896 ms) : 0, 376896
AppSec [baseline] (51.585 ms) : 0, 51585
AppSec [candidate] (51.419 ms) : 0, 51419
Remote Config [baseline] (993.969 µs) : 0, 994
Remote Config [candidate] (1.003 ms) : 0, 1003
Telemetry [baseline] (7.24 ms) : 0, 7240
Telemetry [candidate] (7.201 ms) : 0, 7201
ProfilingAgent [baseline] (95.29 ms) : 0, 95290
ProfilingAgent [candidate] (95.926 ms) : 0, 95926
Profiling [baseline] (95.317 ms) : 0, 95317
Profiling [candidate] (95.952 ms) : 0, 95952
Loading
Startup time reports for insecure-bank
gantt
    title insecure-bank - global startup overhead: candidate=1.27.0-SNAPSHOT~1761501028, baseline=1.27.0-SNAPSHOT~691d820a0b

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.056 s) : 0, 1056163
Total [baseline] (8.715 s) : 0, 8715198
Agent [candidate] (1.049 s) : 0, 1049463
Total [candidate] (8.72 s) : 0, 8719545
section iast
Agent [baseline] (1.166 s) : 0, 1166281
Total [baseline] (9.227 s) : 0, 9226903
Agent [candidate] (1.174 s) : 0, 1174267
Total [candidate] (9.308 s) : 0, 9308432
section iast_TELEMETRY_OFF
Agent [baseline] (1.158 s) : 0, 1158211
Total [baseline] (9.222 s) : 0, 9222036
Agent [candidate] (1.167 s) : 0, 1166865
Total [candidate] (9.289 s) : 0, 9289439
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.056 s -
Agent iast 1.166 s 110.117 ms (10.4%)
Agent iast_TELEMETRY_OFF 1.158 s 102.048 ms (9.7%)
Total tracing 8.715 s -
Total iast 9.227 s 511.704 ms (5.9%)
Total iast_TELEMETRY_OFF 9.222 s 506.838 ms (5.8%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.049 s -
Agent iast 1.174 s 124.803 ms (11.9%)
Agent iast_TELEMETRY_OFF 1.167 s 117.401 ms (11.2%)
Total tracing 8.72 s -
Total iast 9.308 s 588.887 ms (6.8%)
Total iast_TELEMETRY_OFF 9.289 s 569.894 ms (6.5%)
gantt
    title insecure-bank - break down per module: candidate=1.27.0-SNAPSHOT~1761501028, baseline=1.27.0-SNAPSHOT~691d820a0b

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (654.134 ms) : 0, 654134
BytebuddyAgent [candidate] (648.507 ms) : 0, 648507
GlobalTracer [baseline] (308.874 ms) : 0, 308874
GlobalTracer [candidate] (307.553 ms) : 0, 307553
AppSec [baseline] (50.636 ms) : 0, 50636
AppSec [candidate] (51.319 ms) : 0, 51319
Remote Config [baseline] (673.309 µs) : 0, 673
Remote Config [candidate] (664.661 µs) : 0, 665
Telemetry [baseline] (7.285 ms) : 0, 7285
Telemetry [candidate] (7.128 ms) : 0, 7128
section iast
BytebuddyAgent [baseline] (768.942 ms) : 0, 768942
BytebuddyAgent [candidate] (774.817 ms) : 0, 774817
GlobalTracer [baseline] (284.453 ms) : 0, 284453
GlobalTracer [candidate] (287.288 ms) : 0, 287288
AppSec [baseline] (51.045 ms) : 0, 51045
AppSec [candidate] (48.928 ms) : 0, 48928
IAST [baseline] (20.535 ms) : 0, 20535
IAST [candidate] (19.992 ms) : 0, 19992
Remote Config [baseline] (564.113 µs) : 0, 564
Remote Config [candidate] (1.314 ms) : 0, 1314
Telemetry [baseline] (6.452 ms) : 0, 6452
Telemetry [candidate] (7.31 ms) : 0, 7310
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (761.06 ms) : 0, 761060
BytebuddyAgent [candidate] (766.573 ms) : 0, 766573
GlobalTracer [baseline] (284.696 ms) : 0, 284696
GlobalTracer [candidate] (287.943 ms) : 0, 287943
AppSec [baseline] (49.222 ms) : 0, 49222
AppSec [candidate] (49.724 ms) : 0, 49724
IAST [baseline] (21.885 ms) : 0, 21885
IAST [candidate] (19.277 ms) : 0, 19277
Remote Config [baseline] (586.226 µs) : 0, 586
Remote Config [candidate] (631.132 µs) : 0, 631
Telemetry [baseline] (6.458 ms) : 0, 6458
Telemetry [candidate] (8.159 ms) : 0, 8159
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2023-12-22T17:07:01 2023-12-22T17:23:35
git_branch master jpbempel/optional
git_commit_date 1703258917 1703263358
git_commit_sha 691d820 1761501
release_version 1.27.0-SNAPSHOT~691d820a0b 1.27.0-SNAPSHOT~1761501028
start_time 2023-12-22T17:06:48 2023-12-22T17:23:22
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1703266068 1703266068
ci_job_id 396043577 396043577
ci_pipeline_id 25575282 25575282
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant iast iast

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 9 metrics, 13 unstable metrics.

Request duration reports for petclinic
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.27.0-SNAPSHOT~1761501028, baseline=1.27.0-SNAPSHOT~691d820a0b
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.356 ms) : 1337, 1375
.   : milestone, 1356,
appsec (1.767 ms) : 1743, 1792
.   : milestone, 1767,
iast (1.516 ms) : 1492, 1540
.   : milestone, 1516,
profiling (1.516 ms) : 1491, 1540
.   : milestone, 1516,
tracing (1.493 ms) : 1468, 1517
.   : milestone, 1493,
section candidate
no_agent (1.355 ms) : 1336, 1374
.   : milestone, 1355,
appsec (1.758 ms) : 1732, 1783
.   : milestone, 1758,
iast (1.512 ms) : 1488, 1536
.   : milestone, 1512,
profiling (1.511 ms) : 1486, 1536
.   : milestone, 1511,
tracing (1.501 ms) : 1477, 1525
.   : milestone, 1501,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.356 ms [1.337 ms, 1.375 ms] -
appsec 1.767 ms [1.743 ms, 1.792 ms] 411.569 µs (30.4%)
iast 1.516 ms [1.492 ms, 1.54 ms] 160.215 µs (11.8%)
profiling 1.516 ms [1.491 ms, 1.54 ms] 159.889 µs (11.8%)
tracing 1.493 ms [1.468 ms, 1.517 ms] 136.861 µs (10.1%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.355 ms [1.336 ms, 1.374 ms] -
appsec 1.758 ms [1.732 ms, 1.783 ms] 402.253 µs (29.7%)
iast 1.512 ms [1.488 ms, 1.536 ms] 156.74 µs (11.6%)
profiling 1.511 ms [1.486 ms, 1.536 ms] 156.037 µs (11.5%)
tracing 1.501 ms [1.477 ms, 1.525 ms] 146.141 µs (10.8%)
Request duration reports for insecure-bank
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.27.0-SNAPSHOT~1761501028, baseline=1.27.0-SNAPSHOT~691d820a0b
    dateFormat X
    axisFormat %s
section baseline
no_agent (363.197 µs) : 342, 384
.   : milestone, 363,
iast (473.563 µs) : 453, 494
.   : milestone, 474,
iast_FULL (537.101 µs) : 516, 558
.   : milestone, 537,
iast_INACTIVE (442.395 µs) : 422, 463
.   : milestone, 442,
iast_TELEMETRY_OFF (464.744 µs) : 444, 485
.   : milestone, 465,
tracing (437.137 µs) : 417, 458
.   : milestone, 437,
section candidate
no_agent (359.997 µs) : 341, 379
.   : milestone, 360,
iast (464.883 µs) : 444, 485
.   : milestone, 465,
iast_FULL (529.965 µs) : 510, 550
.   : milestone, 530,
iast_INACTIVE (438.781 µs) : 418, 459
.   : milestone, 439,
iast_TELEMETRY_OFF (467.75 µs) : 446, 489
.   : milestone, 468,
tracing (437.0 µs) : 417, 457
.   : milestone, 437,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 363.197 µs [342.484 µs, 383.909 µs] -
iast 473.563 µs [452.711 µs, 494.415 µs] 110.367 µs (30.4%)
iast_FULL 537.101 µs [516.268 µs, 557.934 µs] 173.904 µs (47.9%)
iast_INACTIVE 442.395 µs [421.856 µs, 462.934 µs] 79.198 µs (21.8%)
iast_TELEMETRY_OFF 464.744 µs [444.395 µs, 485.093 µs] 101.547 µs (28.0%)
tracing 437.137 µs [416.534 µs, 457.74 µs] 73.94 µs (20.4%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 359.997 µs [340.537 µs, 379.458 µs] -
iast 464.883 µs [444.323 µs, 485.443 µs] 104.886 µs (29.1%)
iast_FULL 529.965 µs [509.602 µs, 550.327 µs] 169.967 µs (47.2%)
iast_INACTIVE 438.781 µs [418.296 µs, 459.265 µs] 78.783 µs (21.9%)
iast_TELEMETRY_OFF 467.75 µs [446.437 µs, 489.064 µs] 107.753 µs (29.9%)
tracing 437.0 µs [416.52 µs, 457.48 µs] 77.003 µs (21.4%)

Copy link
Contributor

@shatzi shatzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the direction of having a special handles for fetching values from types, I think I want to ensure this done in a way we can expand it.

@@ -55,6 +58,12 @@ public class WellKnownClasses {
"java.time.LocalDateTime",
"java.util.UUID"));

private static Map<String, Function<Object, SpecialField>> specialFields = new HashMap<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why the map is for a type -> single field.
I think it would be better to have a map for [type,field-name] -> Func.
Two things:

  1. we can now just inject a special access to a single field in an type.
  2. support special types with multiple fields

One more thing we can do is convert the return value to be array of fields instead of a single field. but I like the first direction better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is so far no need to handle multiple fields for this mechanism.

WellKnownClasses.getSpecialFieldAccess(target.getClass().getTypeName());
if (specialFieldAccess != null) {
WellKnownClasses.SpecialField specialField = specialFieldAccess.apply(target);
if (specialField != null && specialField.getName().equals(memberName)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find this wired that we first try to access the "special field" even before we check it name. see other suggestion on the getSpecialFieldAccess should also get field name and not just type.

* @return a function to access special field of a type, or null if type is not supported. This is
* used to avoid using reflection to access fields on well known types
*/
public static Function<Object, SpecialField> getSpecialFieldAccess(String type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one thing that I'm not sure how this works is how about inheritance. assume someone derived from Optional, will this stop working and we would use reflection?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional is final

return new SpecialField("value", Object.class.getTypeName(), ((Optional<?>) o).orElse(null));
}

public static class SpecialField {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use CaptureValue? seems very similar...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true. done.

@@ -294,6 +296,8 @@ static class WellKnownClasses {
UUID uuid = UUID.nameUUIDFromBytes("foobar".getBytes());
AtomicLong atomicLong = new AtomicLong(123);
URI uri = URI.create("https://www.datadoghq.com");
Optional<Date> maybeUiid = Optional.of(new Date());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybeDate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@jpbempel jpbempel merged commit 032308c into master Dec 27, 2023
73 checks passed
@jpbempel jpbempel deleted the jpbempel/optional branch December 27, 2023 06:45
@github-actions github-actions bot added this to the 1.27.0 milestone Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants