Remove explicit capture of fields #7282
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jpbempel
force-pushed
the
jpbempel/drop-field-capture
branch
from
0657df9
to
7058cbb
Compare
jpbempel
changed the title
jpbempel
force-pushed
the
jpbempel/drop-field-capture
branch
from
7058cbb
to
c7b8be2
Compare
Previously we were capturing directly fields of the current class in a specific way and use them for condition evaluation, while this was captured as argument. Now we only capture this and all evaluations are based on implicit this dereferencing when needed. NB: Serialization of complex classes no longer capture static fields
jpbempel
force-pushed
the
jpbempel/drop-field-capture
branch
from
c7b8be2
to
8712cc2
Compare
Debugger benchmarksParameters
See matching parameters
SummaryFound 5 performance improvements and 0 performance regressions! Performance is the same for 2 metrics, 8 unstable metrics.
See unchanged results
Request duration reports for reportsgantt
title reports - request duration [CI 0.99] : candidate=None, baseline=None
dateFormat X
axisFormat %s
section baseline
noprobe (303.308 µs) : 255, 352
. : milestone, 303,
basic (298.371 µs) : 286, 310
. : milestone, 298,
loop (11.421 ms) : 11372, 11470
. : milestone, 11421,
section candidate
noprobe (299.49 µs) : 248, 351
. : milestone, 299,
basic (298.073 µs) : 285, 312
. : milestone, 298,
loop (10.93 ms) : 10901, 10960
. : milestone, 10930,
|
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 52 metrics, 11 unstable metrics. Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.063 s) : 0, 1062750
Total [baseline] (8.504 s) : 0, 8503653
Agent [candidate] (1.065 s) : 0, 1064732
Total [candidate] (8.512 s) : 0, 8512257
section iast
Agent [baseline] (1.179 s) : 0, 1178758
Total [baseline] (8.949 s) : 0, 8949280
Agent [candidate] (1.18 s) : 0, 1180161
Total [candidate] (8.97 s) : 0, 8969635
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.172 s) : 0, 1172091
Total [baseline] (8.926 s) : 0, 8925530
Agent [candidate] (1.178 s) : 0, 1177518
Total [candidate] (8.94 s) : 0, 8940119
section iast_TELEMETRY_OFF
Agent [baseline] (1.165 s) : 0, 1164872
Total [baseline] (8.918 s) : 0, 8917765
Agent [candidate] (1.164 s) : 0, 1164394
Total [candidate] (8.978 s) : 0, 8977991
gantt
title insecure-bank - break down per module: candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (665.339 ms) : 0, 665339
BytebuddyAgent [candidate] (665.963 ms) : 0, 665963
GlobalTracer [baseline] (304.579 ms) : 0, 304579
GlobalTracer [candidate] (305.703 ms) : 0, 305703
AppSec [baseline] (49.935 ms) : 0, 49935
AppSec [candidate] (50.124 ms) : 0, 50124
Remote Config [baseline] (671.577 µs) : 0, 672
Remote Config [candidate] (673.219 µs) : 0, 673
Telemetry [baseline] (7.64 ms) : 0, 7640
Telemetry [candidate] (7.667 ms) : 0, 7667
section iast
BytebuddyAgent [baseline] (785.777 ms) : 0, 785777
BytebuddyAgent [candidate] (786.365 ms) : 0, 786365
GlobalTracer [baseline] (297.858 ms) : 0, 297858
GlobalTracer [candidate] (297.832 ms) : 0, 297832
AppSec [baseline] (47.259 ms) : 0, 47259
AppSec [candidate] (47.929 ms) : 0, 47929
IAST [baseline] (26.762 ms) : 0, 26762
IAST [candidate] (26.867 ms) : 0, 26867
Remote Config [baseline] (564.988 µs) : 0, 565
Remote Config [candidate] (562.487 µs) : 0, 562
Telemetry [baseline] (6.992 ms) : 0, 6992
Telemetry [candidate] (7.0 ms) : 0, 7000
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (779.282 ms) : 0, 779282
BytebuddyAgent [candidate] (784.724 ms) : 0, 784724
GlobalTracer [baseline] (295.585 ms) : 0, 295585
GlobalTracer [candidate] (296.58 ms) : 0, 296580
AppSec [baseline] (47.432 ms) : 0, 47432
AppSec [candidate] (47.613 ms) : 0, 47613
IAST [baseline] (27.886 ms) : 0, 27886
IAST [candidate] (26.56 ms) : 0, 26560
Remote Config [baseline] (579.713 µs) : 0, 580
Remote Config [candidate] (590.808 µs) : 0, 591
Telemetry [baseline] (7.8 ms) : 0, 7800
Telemetry [candidate] (7.873 ms) : 0, 7873
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (774.882 ms) : 0, 774882
BytebuddyAgent [candidate] (775.492 ms) : 0, 775492
GlobalTracer [baseline] (294.218 ms) : 0, 294218
GlobalTracer [candidate] (294.715 ms) : 0, 294715
AppSec [baseline] (47.157 ms) : 0, 47157
AppSec [candidate] (46.634 ms) : 0, 46634
IAST [baseline] (27.735 ms) : 0, 27735
IAST [candidate] (25.861 ms) : 0, 25861
Remote Config [baseline] (585.78 µs) : 0, 586
Remote Config [candidate] (589.414 µs) : 0, 589
Telemetry [baseline] (6.791 ms) : 0, 6791
Telemetry [candidate] (7.598 ms) : 0, 7598
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.065 s) : 0, 1064624
Total [baseline] (10.288 s) : 0, 10288442
Agent [candidate] (1.063 s) : 0, 1062851
Total [candidate] (10.321 s) : 0, 10320806
section appsec
Agent [baseline] (1.184 s) : 0, 1183723
Total [baseline] (10.533 s) : 0, 10532576
Agent [candidate] (1.192 s) : 0, 1191798
Total [candidate] (10.493 s) : 0, 10492546
section iast
Agent [baseline] (1.17 s) : 0, 1169692
Total [baseline] (10.696 s) : 0, 10695804
Agent [candidate] (1.18 s) : 0, 1179539
Total [candidate] (10.78 s) : 0, 10780180
section profiling
Agent [baseline] (1.275 s) : 0, 1275281
Total [baseline] (10.638 s) : 0, 10637970
Agent [candidate] (1.265 s) : 0, 1264841
Total [candidate] (10.626 s) : 0, 10625867
gantt
title petclinic - break down per module: candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (665.825 ms) : 0, 665825
BytebuddyAgent [candidate] (664.632 ms) : 0, 664632
GlobalTracer [baseline] (305.487 ms) : 0, 305487
GlobalTracer [candidate] (305.353 ms) : 0, 305353
AppSec [baseline] (50.262 ms) : 0, 50262
AppSec [candidate] (50.023 ms) : 0, 50023
Remote Config [baseline] (681.181 µs) : 0, 681
Remote Config [candidate] (663.563 µs) : 0, 664
Telemetry [baseline] (7.684 ms) : 0, 7684
Telemetry [candidate] (7.632 ms) : 0, 7632
section appsec
BytebuddyAgent [baseline] (676.304 ms) : 0, 676304
BytebuddyAgent [candidate] (682.253 ms) : 0, 682253
GlobalTracer [baseline] (298.321 ms) : 0, 298321
GlobalTracer [candidate] (300.449 ms) : 0, 300449
AppSec [baseline] (153.692 ms) : 0, 153692
AppSec [candidate] (154.095 ms) : 0, 154095
IAST [baseline] (20.902 ms) : 0, 20902
IAST [candidate] (20.321 ms) : 0, 20321
Remote Config [baseline] (623.991 µs) : 0, 624
Remote Config [candidate] (627.988 µs) : 0, 628
Telemetry [baseline] (9.605 ms) : 0, 9605
Telemetry [candidate] (9.215 ms) : 0, 9215
section iast
BytebuddyAgent [baseline] (779.518 ms) : 0, 779518
BytebuddyAgent [candidate] (784.539 ms) : 0, 784539
GlobalTracer [baseline] (295.502 ms) : 0, 295502
GlobalTracer [candidate] (297.07 ms) : 0, 297070
AppSec [baseline] (47.171 ms) : 0, 47171
AppSec [candidate] (47.564 ms) : 0, 47564
IAST [baseline] (25.675 ms) : 0, 25675
IAST [candidate] (28.42 ms) : 0, 28420
Remote Config [baseline] (571.71 µs) : 0, 572
Remote Config [candidate] (573.998 µs) : 0, 574
Telemetry [baseline] (7.778 ms) : 0, 7778
Telemetry [candidate] (7.771 ms) : 0, 7771
section profiling
BytebuddyAgent [baseline] (669.059 ms) : 0, 669059
BytebuddyAgent [candidate] (662.545 ms) : 0, 662545
GlobalTracer [baseline] (391.754 ms) : 0, 391754
GlobalTracer [candidate] (389.176 ms) : 0, 389176
AppSec [baseline] (52.085 ms) : 0, 52085
AppSec [candidate] (51.864 ms) : 0, 51864
Remote Config [baseline] (650.664 µs) : 0, 651
Remote Config [candidate] (663.699 µs) : 0, 664
Telemetry [baseline] (7.429 ms) : 0, 7429
Telemetry [candidate] (7.32 ms) : 0, 7320
ProfilingAgent [baseline] (96.564 ms) : 0, 96564
ProfilingAgent [candidate] (96.109 ms) : 0, 96109
Profiling [baseline] (96.589 ms) : 0, 96589
Profiling [candidate] (96.134 ms) : 0, 96134
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 11 metrics, 17 unstable metrics. Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section baseline
no_agent (1.327 ms) : 1307, 1347
. : milestone, 1327,
appsec (1.689 ms) : 1665, 1714
. : milestone, 1689,
appsec_no_iast (1.705 ms) : 1681, 1729
. : milestone, 1705,
iast (1.475 ms) : 1453, 1498
. : milestone, 1475,
profiling (1.501 ms) : 1476, 1527
. : milestone, 1501,
tracing (1.482 ms) : 1458, 1506
. : milestone, 1482,
section candidate
no_agent (1.35 ms) : 1331, 1369
. : milestone, 1350,
appsec (1.73 ms) : 1706, 1753
. : milestone, 1730,
appsec_no_iast (1.713 ms) : 1690, 1736
. : milestone, 1713,
iast (1.472 ms) : 1449, 1495
. : milestone, 1472,
profiling (1.488 ms) : 1464, 1513
. : milestone, 1488,
tracing (1.443 ms) : 1419, 1467
. : milestone, 1443,
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section baseline
no_agent (366.555 µs) : 347, 386
. : milestone, 367,
iast (473.432 µs) : 453, 494
. : milestone, 473,
iast_FULL (545.55 µs) : 524, 567
. : milestone, 546,
iast_GLOBAL (505.512 µs) : 483, 528
. : milestone, 506,
iast_HARDCODED_SECRET_DISABLED (478.39 µs) : 456, 500
. : milestone, 478,
iast_INACTIVE (449.742 µs) : 427, 472
. : milestone, 450,
iast_TELEMETRY_OFF (468.183 µs) : 447, 490
. : milestone, 468,
tracing (438.444 µs) : 418, 459
. : milestone, 438,
section candidate
no_agent (367.746 µs) : 347, 389
. : milestone, 368,
iast (476.575 µs) : 455, 498
. : milestone, 477,
iast_FULL (541.098 µs) : 520, 562
. : milestone, 541,
iast_GLOBAL (507.118 µs) : 485, 529
. : milestone, 507,
iast_HARDCODED_SECRET_DISABLED (472.553 µs) : 451, 494
. : milestone, 473,
iast_INACTIVE (448.897 µs) : 427, 470
. : milestone, 449,
iast_TELEMETRY_OFF (468.47 µs) : 447, 490
. : milestone, 468,
tracing (435.664 µs) : 415, 456
. : milestone, 436,
DacapoParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 0 unstable metrics. Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section baseline
no_agent (1.455 ms) : 1444, 1466
. : milestone, 1455,
appsec (2.212 ms) : 2177, 2247
. : milestone, 2212,
iast (1.961 ms) : 1920, 2002
. : milestone, 1961,
iast_GLOBAL (2.006 ms) : 1965, 2047
. : milestone, 2006,
profiling (1.846 ms) : 1813, 1880
. : milestone, 1846,
tracing (1.838 ms) : 1805, 1871
. : milestone, 1838,
section candidate
no_agent (1.452 ms) : 1441, 1463
. : milestone, 1452,
appsec (2.203 ms) : 2168, 2237
. : milestone, 2203,
iast (1.956 ms) : 1915, 1997
. : milestone, 1956,
iast_GLOBAL (2.002 ms) : 1960, 2045
. : milestone, 2002,
profiling (1.858 ms) : 1822, 1893
. : milestone, 1858,
tracing (1.83 ms) : 1798, 1863
. : milestone, 1830,
Execution time for biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.38.0-SNAPSHOT~32bad461e7, baseline=1.38.0-SNAPSHOT~5a09529999
dateFormat X
axisFormat %s
section baseline
no_agent (14.991 s) : 14991000, 14991000
. : milestone, 14991000,
appsec (15.143 s) : 15143000, 15143000
. : milestone, 15143000,
iast (18.694 s) : 18694000, 18694000
. : milestone, 18694000,
iast_GLOBAL (17.638 s) : 17638000, 17638000
. : milestone, 17638000,
profiling (15.497 s) : 15497000, 15497000
. : milestone, 15497000,
tracing (15.022 s) : 15022000, 15022000
. : milestone, 15022000,
section candidate
no_agent (15.065 s) : 15065000, 15065000
. : milestone, 15065000,
appsec (15.083 s) : 15083000, 15083000
. : milestone, 15083000,
iast (18.672 s) : 18672000, 18672000
. : milestone, 18672000,
iast_GLOBAL (17.848 s) : 17848000, 17848000
. : milestone, 17848000,
profiling (15.877 s) : 15877000, 15877000
. : milestone, 15877000,
tracing (15.091 s) : 15091000, 15091000
. : milestone, 15091000,
|
shatzi
approved these changes
Jul 8, 2024
| @@ -377,8 +357,8 @@ public String toString() { | |||
| + locals | |||
| + ", throwable=" | |||
| + throwable | |||
| + ", fields=" | |||
| + fields | |||
| + ", staticFields=" | |||
| import java.util.*; | ||
|
|
||
| public class RefResolverHelper { | ||
|
|
||
| public static ValueReferenceResolver createResolver(Object instance) { | ||
| /* |
| strList.add("foo"); | ||
| } | ||
| } | ||
| ValueReferenceResolver ctx = |
There was a problem hiding this comment.
why not use createResolver(new Object)? - as it seems to do the same thing...
public static ValueReferenceResolver createResolver(Object instance) { ... }
| @@ -857,7 +857,7 @@ private void collectStaticFields(InsnList insnList) { | |||
| // stack: [capturedcontext] | |||
| } | |||
|
|
|||
| private void collectFields(InsnList insnList) { | |||
| private void collectSpecialFields(InsnList insnList) { | |||
There was a problem hiding this comment.
nit: rename to collectCorrleationInfo?
| insnList.add(new InsnNode(Opcodes.DUP)); | ||
| // stack: [capturedcontext, capturedcontext] | ||
| ldc(insnList, fieldsToCapture.size()); |
| @@ -48,31 +49,23 @@ void testPredicatedRef() { | |||
|
|
|||
| RuntimeException runtimeException = | |||
| assertThrows(RuntimeException.class, () -> isEmptyInvalid.evaluate(ctx)); | |||
| assertEquals("Cannot find symbol: x", runtimeException.getMessage()); | |||
| assertEquals("Cannot dereference to field: x", runtimeException.getMessage()); | |||
There was a problem hiding this comment.
not sure Cannot dereference to field is right...
I think Cannot dereference field: is better
| fromJsonFields(jsonReader, capturedContext); | ||
| continue; | ||
| } | ||
| // if ("this".equals(argName)) { |
| @@ -234,7 +239,7 @@ private void fromJsonFields(JsonReader jsonReader, CapturedContext capturedConte | |||
| } | |||
| case FIELDS: | |||
remote unnecessary comments and dead code
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This Do
Previously we were capturing directly fields of the current class in a specific way and use them for condition evaluation, while this was captured as argument. Now we only capture this and all evaluations are based on implicit this dereferencing when needed. NB: Serialization of complex classes no longer capture static fields
Motivation
Remove useless captures & performance
Additional Notes
Jira ticket: DEBUG-2399