-
Notifications
You must be signed in to change notification settings - Fork 292
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 collection of local var for method probe #7548
Conversation
Collecting local var was disabled for method probe/unhanded exception because all local vars are not accessible/defined. But for Exception Debugging/Replay this is very useful to have. The solution is to hoist local variable at the top level of the method and initialize them to be able to capture them at any point of the method. we are handling also the case of duplicate local var in different scope but with incompatible types
Debugger benchmarksParameters
See matching parameters
SummaryFound 0 performance improvements and 3 performance regressions! Performance is the same for 6 metrics, 6 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 (289.735 µs) : 258, 322
. : milestone, 290,
basic (284.161 µs) : 275, 293
. : milestone, 284,
loop (10.512 ms) : 10479, 10546
. : milestone, 10512,
section candidate
noprobe (303.919 µs) : 253, 354
. : milestone, 304,
basic (302.381 µs) : 290, 315
. : milestone, 302,
loop (10.523 ms) : 10489, 10557
. : milestone, 10523,
|
BenchmarksStartupParameters
See matching parameters
SummaryFound 1 performance improvements and 0 performance regressions! Performance is the same for 48 metrics, 14 unstable metrics.
Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.053 s) : 0, 1053314
Total [baseline] (10.392 s) : 0, 10391893
Agent [candidate] (1.051 s) : 0, 1051085
Total [candidate] (10.512 s) : 0, 10512357
section appsec
Agent [baseline] (1.193 s) : 0, 1193060
Total [baseline] (10.642 s) : 0, 10642345
Agent [candidate] (1.177 s) : 0, 1177495
Total [candidate] (10.557 s) : 0, 10557302
section iast
Agent [baseline] (1.175 s) : 0, 1175269
Total [baseline] (10.866 s) : 0, 10865529
Agent [candidate] (1.184 s) : 0, 1184464
Total [candidate] (10.85 s) : 0, 10850441
section profiling
Agent [baseline] (1.25 s) : 0, 1249749
Total [baseline] (10.643 s) : 0, 10643037
Agent [candidate] (1.265 s) : 0, 1264809
Total [candidate] (10.694 s) : 0, 10693998
gantt
title petclinic - break down per module: candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (671.998 ms) : 0, 671998
BytebuddyAgent [candidate] (670.518 ms) : 0, 670518
GlobalTracer [baseline] (307.958 ms) : 0, 307958
GlobalTracer [candidate] (307.354 ms) : 0, 307354
AppSec [baseline] (51.604 ms) : 0, 51604
AppSec [candidate] (51.49 ms) : 0, 51490
Remote Config [baseline] (672.872 µs) : 0, 673
Remote Config [candidate] (683.87 µs) : 0, 684
Telemetry [baseline] (7.494 ms) : 0, 7494
Telemetry [candidate] (7.475 ms) : 0, 7475
section appsec
BytebuddyAgent [baseline] (697.433 ms) : 0, 697433
BytebuddyAgent [candidate] (682.945 ms) : 0, 682945
GlobalTracer [baseline] (302.418 ms) : 0, 302418
GlobalTracer [candidate] (302.571 ms) : 0, 302571
AppSec [baseline] (160.029 ms) : 0, 160029
AppSec [candidate] (160.932 ms) : 0, 160932
Remote Config [baseline] (626.783 µs) : 0, 627
Remote Config [candidate] (630.963 µs) : 0, 631
Telemetry [baseline] (8.62 ms) : 0, 8620
Telemetry [candidate] (7.536 ms) : 0, 7536
IAST [baseline] (20.474 ms) : 0, 20474
IAST [candidate] (18.481 ms) : 0, 18481
section iast
BytebuddyAgent [baseline] (781.599 ms) : 0, 781599
BytebuddyAgent [candidate] (786.052 ms) : 0, 786052
GlobalTracer [baseline] (296.569 ms) : 0, 296569
GlobalTracer [candidate] (298.925 ms) : 0, 298925
AppSec [baseline] (50.706 ms) : 0, 50706
AppSec [candidate] (52.535 ms) : 0, 52535
Remote Config [baseline] (576.322 µs) : 0, 576
Remote Config [candidate] (608.571 µs) : 0, 609
Telemetry [baseline] (7.986 ms) : 0, 7986
Telemetry [candidate] (9.671 ms) : 0, 9671
IAST [baseline] (24.256 ms) : 0, 24256
IAST [candidate] (22.973 ms) : 0, 22973
section profiling
BytebuddyAgent [baseline] (665.904 ms) : 0, 665904
BytebuddyAgent [candidate] (673.106 ms) : 0, 673106
GlobalTracer [baseline] (389.344 ms) : 0, 389344
GlobalTracer [candidate] (394.762 ms) : 0, 394762
AppSec [baseline] (52.352 ms) : 0, 52352
AppSec [candidate] (53.14 ms) : 0, 53140
Remote Config [baseline] (688.594 µs) : 0, 689
Remote Config [candidate] (702.87 µs) : 0, 703
Telemetry [baseline] (7.395 ms) : 0, 7395
Telemetry [candidate] (7.47 ms) : 0, 7470
ProfilingAgent [baseline] (96.225 ms) : 0, 96225
ProfilingAgent [candidate] (97.949 ms) : 0, 97949
Profiling [baseline] (96.249 ms) : 0, 96249
Profiling [candidate] (97.973 ms) : 0, 97973
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.053 s) : 0, 1053213
Total [baseline] (8.536 s) : 0, 8536327
Agent [candidate] (1.055 s) : 0, 1054799
Total [candidate] (8.497 s) : 0, 8497196
section iast
Agent [baseline] (1.185 s) : 0, 1184811
Total [baseline] (9.039 s) : 0, 9038817
Agent [candidate] (1.183 s) : 0, 1183019
Total [candidate] (9.044 s) : 0, 9044385
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.176 s) : 0, 1175927
Total [baseline] (8.959 s) : 0, 8958678
Agent [candidate] (1.173 s) : 0, 1173050
Total [candidate] (8.98 s) : 0, 8979509
section iast_TELEMETRY_OFF
Agent [baseline] (1.175 s) : 0, 1175197
Total [baseline] (8.979 s) : 0, 8978594
Agent [candidate] (1.172 s) : 0, 1171627
Total [candidate] (8.973 s) : 0, 8972912
gantt
title insecure-bank - break down per module: candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (672.134 ms) : 0, 672134
BytebuddyAgent [candidate] (674.196 ms) : 0, 674196
GlobalTracer [baseline] (308.106 ms) : 0, 308106
GlobalTracer [candidate] (307.485 ms) : 0, 307485
AppSec [baseline] (51.214 ms) : 0, 51214
AppSec [candidate] (51.327 ms) : 0, 51327
Remote Config [baseline] (664.22 µs) : 0, 664
Remote Config [candidate] (664.497 µs) : 0, 664
Telemetry [baseline] (7.493 ms) : 0, 7493
Telemetry [candidate] (7.455 ms) : 0, 7455
section iast
BytebuddyAgent [baseline] (788.27 ms) : 0, 788270
BytebuddyAgent [candidate] (785.867 ms) : 0, 785867
GlobalTracer [baseline] (298.619 ms) : 0, 298619
GlobalTracer [candidate] (298.826 ms) : 0, 298826
AppSec [baseline] (53.241 ms) : 0, 53241
AppSec [candidate] (51.975 ms) : 0, 51975
IAST [baseline] (22.215 ms) : 0, 22215
IAST [candidate] (23.775 ms) : 0, 23775
Remote Config [baseline] (604.975 µs) : 0, 605
Remote Config [candidate] (605.482 µs) : 0, 605
Telemetry [baseline] (8.183 ms) : 0, 8183
Telemetry [candidate] (8.261 ms) : 0, 8261
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (781.352 ms) : 0, 781352
BytebuddyAgent [candidate] (778.464 ms) : 0, 778464
GlobalTracer [baseline] (296.893 ms) : 0, 296893
GlobalTracer [candidate] (296.638 ms) : 0, 296638
AppSec [baseline] (54.942 ms) : 0, 54942
AppSec [candidate] (48.478 ms) : 0, 48478
IAST [baseline] (21.158 ms) : 0, 21158
IAST [candidate] (24.105 ms) : 0, 24105
Remote Config [baseline] (580.301 µs) : 0, 580
Remote Config [candidate] (601.639 µs) : 0, 602
Telemetry [baseline] (7.39 ms) : 0, 7390
Telemetry [candidate] (11.158 ms) : 0, 11158
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (781.444 ms) : 0, 781444
BytebuddyAgent [candidate] (777.512 ms) : 0, 777512
GlobalTracer [baseline] (296.789 ms) : 0, 296789
GlobalTracer [candidate] (296.574 ms) : 0, 296574
AppSec [baseline] (53.797 ms) : 0, 53797
AppSec [candidate] (50.616 ms) : 0, 50616
IAST [baseline] (20.994 ms) : 0, 20994
IAST [candidate] (24.089 ms) : 0, 24089
Remote Config [baseline] (569.931 µs) : 0, 570
Remote Config [candidate] (598.576 µs) : 0, 599
Telemetry [baseline] (7.969 ms) : 0, 7969
Telemetry [candidate] (8.634 ms) : 0, 8634
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 1 performance regressions! Performance is the same for 10 metrics, 17 unstable metrics.
Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section baseline
no_agent (366.639 µs) : 347, 386
. : milestone, 367,
iast (491.071 µs) : 469, 513
. : milestone, 491,
iast_FULL (552.832 µs) : 532, 574
. : milestone, 553,
iast_GLOBAL (505.801 µs) : 485, 527
. : milestone, 506,
iast_HARDCODED_SECRET_DISABLED (482.318 µs) : 461, 504
. : milestone, 482,
iast_INACTIVE (453.584 µs) : 433, 474
. : milestone, 454,
iast_TELEMETRY_OFF (470.539 µs) : 448, 493
. : milestone, 471,
tracing (443.749 µs) : 423, 465
. : milestone, 444,
section candidate
no_agent (378.723 µs) : 359, 398
. : milestone, 379,
iast (490.523 µs) : 468, 513
. : milestone, 491,
iast_FULL (554.27 µs) : 533, 575
. : milestone, 554,
iast_GLOBAL (511.3 µs) : 490, 533
. : milestone, 511,
iast_HARDCODED_SECRET_DISABLED (487.024 µs) : 465, 509
. : milestone, 487,
iast_INACTIVE (451.453 µs) : 430, 473
. : milestone, 451,
iast_TELEMETRY_OFF (479.981 µs) : 457, 503
. : milestone, 480,
tracing (438.838 µs) : 419, 459
. : milestone, 439,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section baseline
no_agent (1.341 ms) : 1322, 1361
. : milestone, 1341,
appsec (1.74 ms) : 1716, 1763
. : milestone, 1740,
appsec_no_iast (1.736 ms) : 1712, 1761
. : milestone, 1736,
iast (1.484 ms) : 1462, 1507
. : milestone, 1484,
profiling (1.494 ms) : 1470, 1517
. : milestone, 1494,
tracing (1.46 ms) : 1435, 1484
. : milestone, 1460,
section candidate
no_agent (1.336 ms) : 1318, 1354
. : milestone, 1336,
appsec (1.734 ms) : 1711, 1757
. : milestone, 1734,
appsec_no_iast (1.705 ms) : 1680, 1730
. : milestone, 1705,
iast (1.472 ms) : 1449, 1495
. : milestone, 1472,
profiling (1.551 ms) : 1526, 1575
. : milestone, 1551,
tracing (1.482 ms) : 1458, 1507
. : milestone, 1482,
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 biojavagantt
title biojava - execution time [CI 0.99] : candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section baseline
no_agent (14.959 s) : 14959000, 14959000
. : milestone, 14959000,
appsec (15.25 s) : 15250000, 15250000
. : milestone, 15250000,
iast (18.983 s) : 18983000, 18983000
. : milestone, 18983000,
iast_GLOBAL (18.112 s) : 18112000, 18112000
. : milestone, 18112000,
profiling (15.459 s) : 15459000, 15459000
. : milestone, 15459000,
tracing (14.912 s) : 14912000, 14912000
. : milestone, 14912000,
section candidate
no_agent (14.978 s) : 14978000, 14978000
. : milestone, 14978000,
appsec (15.154 s) : 15154000, 15154000
. : milestone, 15154000,
iast (18.786 s) : 18786000, 18786000
. : milestone, 18786000,
iast_GLOBAL (17.702 s) : 17702000, 17702000
. : milestone, 17702000,
profiling (15.077 s) : 15077000, 15077000
. : milestone, 15077000,
tracing (15.37 s) : 15370000, 15370000
. : milestone, 15370000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.39.0-SNAPSHOT~0d1e45ca47, baseline=1.40.0-SNAPSHOT~123a2c8614
dateFormat X
axisFormat %s
section baseline
no_agent (1.455 ms) : 1444, 1466
. : milestone, 1455,
appsec (2.22 ms) : 2184, 2255
. : milestone, 2220,
iast (1.989 ms) : 1945, 2032
. : milestone, 1989,
iast_GLOBAL (2.022 ms) : 1978, 2066
. : milestone, 2022,
profiling (1.857 ms) : 1823, 1892
. : milestone, 1857,
tracing (1.842 ms) : 1809, 1876
. : milestone, 1842,
section candidate
no_agent (1.457 ms) : 1445, 1468
. : milestone, 1457,
appsec (2.21 ms) : 2175, 2244
. : milestone, 2210,
iast (1.972 ms) : 1929, 2015
. : milestone, 1972,
iast_GLOBAL (2.013 ms) : 1970, 2057
. : milestone, 2013,
profiling (1.849 ms) : 1814, 1884
. : milestone, 1849,
tracing (1.839 ms) : 1806, 1873
. : milestone, 1839,
|
2fed31d
to
633fa81
Compare
@@ -40,6 +41,26 @@ public void ensureSafeLoadClass() { | |||
assertEquals(ASMHelperTest.class, clazz); | |||
} | |||
|
|||
@Test | |||
public void isStoreCompatibleType() { |
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.
should we also validate it return false?
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.
done
@@ -2298,7 +2381,7 @@ private TestSnapshotListener installProbes( | |||
Config config = mock(Config.class); | |||
when(config.isDebuggerEnabled()).thenReturn(true); | |||
when(config.isDebuggerClassFileDumpEnabled()).thenReturn(true); | |||
when(config.isDebuggerVerifyByteCode()).thenReturn(true); | |||
when(config.isDebuggerVerifyByteCode()).thenReturn(false); |
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.
why this return false now?
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.
debug left over
@@ -451,6 +458,76 @@ private void instrumentMethodEnter() { | |||
methodNode.instructions.insert(methodEnterLabel, insnList); | |||
} | |||
|
|||
private Collection<LocalVariableNode> initLocalVars(InsnList insnList) { |
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.
nir: rename to initAndHoistLocalVars to better reflect what this does
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.
done
continue; | ||
} | ||
if (!checkDuplicateLocalVar(localVar, localVarsByName)) { | ||
duplicateNames.add(localVar.name); |
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.
do we want to debug log message that this variables was not hoisted? might be a good for support cases. there is already report warning inside checkDuplicatedLocalVar - but I believe it would read better for people not deep in the code.
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.
redundant with the warning
What Does This Do
Collecting local var was disabled for method probe/unhanded exception because all local vars are not accessible/defined. But for Exception Debugging/Replay this is very useful to have. The solution is to hoist local variable at the top level of the method and initialize them to be able to capture them at any point of the method.
we are handling also the case of duplicate local var in different scope but with incompatible types
Motivation
Collecting local vars even if uncaught exception
Additional Notes
Contributor Checklist
type:
and (comp:
orinst:
) labels in addition to any usefull labelsclose
,fix
or any linking keywords when referencing an issue.Use
solves
instead, and assign the PR milestone to the issueJira ticket: DEBUG-2778