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.

Sign up for GitHub

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

Jump to bottom

Fix local var hoisting #7624

Merged
merged 2 commits into from
Sep 17, 2024
Merged

Fix local var hoisting #7624

merged 2 commits into from
Sep 17, 2024

Conversation

jpbempel
Copy link
Member

@jpbempel jpbempel commented Sep 16, 2024 •
edited by jira bot
Loading

What Does This Do

Rewrite the algorithm for hoisting local vars:
Go through all local vars defined in the method and determine by name all local var that are hoistable.
local vars are hoistable if there is no conflict by slot or by name with another.
If conflict by slot and same name we cancel hoisting for this name. if
name is different we allocating a new slot for one of the local.
If conflict by name and type are not compatible we cancel hoisting for this name. If type compatible, we can add in hoistable list. Then we are considering all hoistable variables by name and if there are multiple occurrence for a name, we are merging those variables in a new slot and rewrite var instructions to use this new slot. if only one variable we just assign a new slot and rewrite the instructions in the range.

Motivation

Fixing issues since #7548

Additional Notes

Tested against exploration tests

Contributor Checklist

Jira ticket: DEBUG-2841

@jpbempel jpbempel added the comp: debugger Dynamic Instrumentation label Sep 16, 2024
@jpbempel jpbempel requested a review from a team as a code owner September 16, 2024 09:19
@jpbempel jpbempel requested review from evanchooly and removed request for a team September 16, 2024 09:19
@pr-commenter
Copy link

pr-commenter bot commented Sep 16, 2024 •
edited
Loading

Debugger benchmarks

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
ci_job_date 1726494356 1726494732
end_time 2024-09-16T13:47:12 2024-09-16T13:53:26
git_branch master jpbempel/fix-overlapping-slot
git_commit_sha bb8b0d2 7b3812f
start_time 2024-09-16T13:45:57 2024-09-16T13:52:13
See matching parameters
Baseline Candidate
ci_job_id 640507479 640507479
ci_pipeline_id 44411482 44411482
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
git_commit_date 1726493741 1726493741

Summary

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

See unchanged results
scenario Δ mean agg_http_req_duration_min Δ mean agg_http_req_duration_p50 Δ mean agg_http_req_duration_p75 Δ mean agg_http_req_duration_p99 Δ mean throughput
scenario:noprobe unstable
[-33.015µs; +15.529µs] or [-12.320%; +5.795%]		 unstable
[-48.908µs; +28.982µs] or [-15.875%; +9.407%]		 unstable
[-61.135µs; +40.634µs] or [-18.977%; +12.613%]		 unstable
[-119.864µs; +94.852µs] or [-18.824%; +14.896%]		 unstable
[-60.677op/s; +202.925op/s] or [-2.306%; +7.711%]
scenario:basic unsure
[+0.818µs; +13.474µs] or [+0.305%; +5.022%]		 same same unstable
[-69.270µs; +21.246µs] or [-12.623%; +3.872%]		 unstable
[-238.668op/s; +103.715op/s] or [-9.069%; +3.941%]
scenario:loop unsure
[-83.764µs; -67.918µs] or [-0.823%; -0.667%]		 unsure
[-133.488µs; -56.653µs] or [-1.290%; -0.547%]		 unsure
[-144.544µs; -31.102µs] or [-1.385%; -0.298%]		 same same
Request duration reports for reports 
gantt
    title reports - request duration [CI 0.99] : candidate=None, baseline=None
    dateFormat X
    axisFormat %s
section baseline
noprobe (308.078 µs) : 266, 350
.   : milestone, 308,
basic (296.15 µs) : 286, 307
.   : milestone, 296,
loop (10.348 ms) : 10309, 10386
.   : milestone, 10348,
section candidate
noprobe (298.116 µs) : 266, 330
.   : milestone, 298,
basic (301.355 µs) : 291, 311
.   : milestone, 301,
loop (10.253 ms) : 10220, 10285
.   : milestone, 10253,
Loading
  • baseline results
Scenario Request median duration [CI 0.99]
noprobe 308.078 µs [266.22 µs, 349.937 µs]
basic 296.15 µs [285.527 µs, 306.773 µs]
loop 10.348 ms [10.309 ms, 10.386 ms]
  • candidate results
Scenario Request median duration [CI 0.99]
noprobe 298.116 µs [266.377 µs, 329.854 µs]
basic 301.355 µs [291.332 µs, 311.378 µs]
loop 10.253 ms [10.22 ms, 10.285 ms]

@pr-commenter
Copy link

pr-commenter bot commented Sep 16, 2024 •
edited
Loading

Benchmarks

Startup

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master jpbempel/fix-overlapping-slot
git_commit_date 1726564137 1726569568
git_commit_sha 1429d59 5fc2763
release_version 1.40.0-SNAPSHOT~1429d59b51 1.40.0-SNAPSHOT~5fc276336e
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1726572008 1726572008
ci_job_id 641889976 641889976
ci_pipeline_id 44488952 44488952
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 0 performance regressions! Performance is the same for 48 metrics, 15 unstable metrics.

Startup time reports for petclinic 
gantt
    title petclinic - global startup overhead: candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.064 s) : 0, 1063637
Total [baseline] (10.341 s) : 0, 10340928
Agent [candidate] (1.07 s) : 0, 1070287
Total [candidate] (10.331 s) : 0, 10331412
section appsec
Agent [baseline] (1.199 s) : 0, 1199234
Total [baseline] (10.641 s) : 0, 10641002
Agent [candidate] (1.198 s) : 0, 1198301
Total [candidate] (10.632 s) : 0, 10631738
section iast
Agent [baseline] (1.193 s) : 0, 1193064
Total [baseline] (10.87 s) : 0, 10870310
Agent [candidate] (1.191 s) : 0, 1191055
Total [candidate] (10.774 s) : 0, 10773980
section profiling
Agent [baseline] (1.262 s) : 0, 1262408
Total [baseline] (10.571 s) : 0, 10570777
Agent [candidate] (1.26 s) : 0, 1259719
Total [candidate] (10.602 s) : 0, 10601699
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.064 s -
Agent appsec 1.199 s 135.597 ms (12.7%)
Agent iast 1.193 s 129.427 ms (12.2%)
Agent profiling 1.262 s 198.771 ms (18.7%)
Total tracing 10.341 s -
Total appsec 10.641 s 300.074 ms (2.9%)
Total iast 10.87 s 529.382 ms (5.1%)
Total profiling 10.571 s 229.848 ms (2.2%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.07 s -
Agent appsec 1.198 s 128.014 ms (12.0%)
Agent iast 1.191 s 120.768 ms (11.3%)
Agent profiling 1.26 s 189.432 ms (17.7%)
Total tracing 10.331 s -
Total appsec 10.632 s 300.326 ms (2.9%)
Total iast 10.774 s 442.567 ms (4.3%)
Total profiling 10.602 s 270.287 ms (2.6%) 
gantt
    title petclinic - break down per module: candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (678.657 ms) : 0, 678657
BytebuddyAgent [candidate] (683.219 ms) : 0, 683219
GlobalTracer [baseline] (310.728 ms) : 0, 310728
GlobalTracer [candidate] (313.138 ms) : 0, 313138
AppSec [baseline] (52.455 ms) : 0, 52455
AppSec [candidate] (52.018 ms) : 0, 52018
Remote Config [baseline] (673.668 µs) : 0, 674
Remote Config [candidate] (664.0 µs) : 0, 664
Telemetry [baseline] (7.518 ms) : 0, 7518
Telemetry [candidate] (7.527 ms) : 0, 7527
section appsec
BytebuddyAgent [baseline] (701.686 ms) : 0, 701686
BytebuddyAgent [candidate] (701.008 ms) : 0, 701008
GlobalTracer [baseline] (303.532 ms) : 0, 303532
GlobalTracer [candidate] (303.668 ms) : 0, 303668
AppSec [baseline] (160.823 ms) : 0, 160823
AppSec [candidate] (160.043 ms) : 0, 160043
Remote Config [baseline] (628.848 µs) : 0, 629
Remote Config [candidate] (640.508 µs) : 0, 641
Telemetry [baseline] (9.13 ms) : 0, 9130
Telemetry [candidate] (9.397 ms) : 0, 9397
IAST [baseline] (19.674 ms) : 0, 19674
IAST [candidate] (19.884 ms) : 0, 19884
section iast
BytebuddyAgent [baseline] (795.461 ms) : 0, 795461
BytebuddyAgent [candidate] (792.624 ms) : 0, 792624
GlobalTracer [baseline] (299.042 ms) : 0, 299042
GlobalTracer [candidate] (299.544 ms) : 0, 299544
AppSec [baseline] (52.625 ms) : 0, 52625
AppSec [candidate] (51.971 ms) : 0, 51971
Remote Config [baseline] (627.072 µs) : 0, 627
Remote Config [candidate] (635.75 µs) : 0, 636
Telemetry [baseline] (8.05 ms) : 0, 8050
Telemetry [candidate] (7.332 ms) : 0, 7332
IAST [baseline] (23.577 ms) : 0, 23577
IAST [candidate] (25.242 ms) : 0, 25242
section profiling
ProfilingAgent [baseline] (96.05 ms) : 0, 96050
ProfilingAgent [candidate] (95.887 ms) : 0, 95887
BytebuddyAgent [baseline] (673.573 ms) : 0, 673573
BytebuddyAgent [candidate] (672.199 ms) : 0, 672199
GlobalTracer [baseline] (393.537 ms) : 0, 393537
GlobalTracer [candidate] (392.975 ms) : 0, 392975
AppSec [baseline] (53.121 ms) : 0, 53121
AppSec [candidate] (52.623 ms) : 0, 52623
Remote Config [baseline] (658.367 µs) : 0, 658
Remote Config [candidate] (657.222 µs) : 0, 657
Telemetry [baseline] (7.428 ms) : 0, 7428
Telemetry [candidate] (7.404 ms) : 0, 7404
Profiling [baseline] (96.075 ms) : 0, 96075
Profiling [candidate] (95.911 ms) : 0, 95911
Loading
Startup time reports for insecure-bank 
gantt
    title insecure-bank - global startup overhead: candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51

    dateFormat X
    axisFormat %s
section tracing
Agent [baseline] (1.065 s) : 0, 1064899
Total [baseline] (8.531 s) : 0, 8530746
Agent [candidate] (1.071 s) : 0, 1070986
Total [candidate] (8.547 s) : 0, 8546811
section iast
Agent [baseline] (1.184 s) : 0, 1183818
Total [baseline] (9.014 s) : 0, 9013642
Agent [candidate] (1.189 s) : 0, 1188720
Total [candidate] (9.065 s) : 0, 9065251
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.207 s) : 0, 1206930
Total [baseline] (8.981 s) : 0, 8980880
Agent [candidate] (1.207 s) : 0, 1206854
Total [candidate] (9.028 s) : 0, 9028182
section iast_TELEMETRY_OFF
Agent [baseline] (1.193 s) : 0, 1193400
Total [baseline] (9.012 s) : 0, 9012011
Agent [candidate] (1.202 s) : 0, 1202484
Total [candidate] (9.062 s) : 0, 9062150
Loading
  • baseline results
Module Variant Duration Δ tracing
Agent tracing 1.065 s -
Agent iast 1.184 s 118.919 ms (11.2%)
Agent iast_HARDCODED_SECRET_DISABLED 1.207 s 142.031 ms (13.3%)
Agent iast_TELEMETRY_OFF 1.193 s 128.501 ms (12.1%)
Total tracing 8.531 s -
Total iast 9.014 s 482.896 ms (5.7%)
Total iast_HARDCODED_SECRET_DISABLED 8.981 s 450.134 ms (5.3%)
Total iast_TELEMETRY_OFF 9.012 s 481.265 ms (5.6%)
  • candidate results
Module Variant Duration Δ tracing
Agent tracing 1.071 s -
Agent iast 1.189 s 117.734 ms (11.0%)
Agent iast_HARDCODED_SECRET_DISABLED 1.207 s 135.868 ms (12.7%)
Agent iast_TELEMETRY_OFF 1.202 s 131.498 ms (12.3%)
Total tracing 8.547 s -
Total iast 9.065 s 518.439 ms (6.1%)
Total iast_HARDCODED_SECRET_DISABLED 9.028 s 481.37 ms (5.6%)
Total iast_TELEMETRY_OFF 9.062 s 515.339 ms (6.0%) 
gantt
    title insecure-bank - break down per module: candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51

    dateFormat X
    axisFormat %s
section tracing
BytebuddyAgent [baseline] (680.184 ms) : 0, 680184
BytebuddyAgent [candidate] (683.494 ms) : 0, 683494
GlobalTracer [baseline] (310.322 ms) : 0, 310322
GlobalTracer [candidate] (313.32 ms) : 0, 313320
AppSec [baseline] (52.559 ms) : 0, 52559
AppSec [candidate] (52.312 ms) : 0, 52312
Remote Config [baseline] (665.956 µs) : 0, 666
Remote Config [candidate] (669.998 µs) : 0, 670
Telemetry [baseline] (7.53 ms) : 0, 7530
Telemetry [candidate] (7.499 ms) : 0, 7499
section iast
BytebuddyAgent [baseline] (787.627 ms) : 0, 787627
BytebuddyAgent [candidate] (791.212 ms) : 0, 791212
GlobalTracer [baseline] (298.021 ms) : 0, 298021
GlobalTracer [candidate] (298.771 ms) : 0, 298771
AppSec [baseline] (53.252 ms) : 0, 53252
AppSec [candidate] (53.303 ms) : 0, 53303
IAST [baseline] (23.428 ms) : 0, 23428
IAST [candidate] (23.723 ms) : 0, 23723
Remote Config [baseline] (634.239 µs) : 0, 634
Remote Config [candidate] (649.615 µs) : 0, 650
Telemetry [baseline] (7.286 ms) : 0, 7286
Telemetry [candidate] (7.408 ms) : 0, 7408
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (803.972 ms) : 0, 803972
BytebuddyAgent [candidate] (804.295 ms) : 0, 804295
GlobalTracer [baseline] (302.972 ms) : 0, 302972
GlobalTracer [candidate] (303.147 ms) : 0, 303147
AppSec [baseline] (52.993 ms) : 0, 52993
AppSec [candidate] (53.8 ms) : 0, 53800
IAST [baseline] (25.202 ms) : 0, 25202
IAST [candidate] (23.778 ms) : 0, 23778
Remote Config [baseline] (609.012 µs) : 0, 609
Remote Config [candidate] (604.588 µs) : 0, 605
Telemetry [baseline] (7.325 ms) : 0, 7325
Telemetry [candidate] (7.339 ms) : 0, 7339
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (793.187 ms) : 0, 793187
BytebuddyAgent [candidate] (799.521 ms) : 0, 799521
GlobalTracer [baseline] (301.058 ms) : 0, 301058
GlobalTracer [candidate] (303.15 ms) : 0, 303150
AppSec [baseline] (55.8 ms) : 0, 55800
AppSec [candidate] (55.211 ms) : 0, 55211
IAST [baseline] (21.628 ms) : 0, 21628
IAST [candidate] (21.047 ms) : 0, 21047
Remote Config [baseline] (607.462 µs) : 0, 607
Remote Config [candidate] (610.009 µs) : 0, 610
Telemetry [baseline] (7.387 ms) : 0, 7387
Telemetry [candidate] (9.096 ms) : 0, 9096
Loading

Load

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
end_time 2024-09-17T10:50:45 2024-09-17T10:57:33
git_branch master jpbempel/fix-overlapping-slot
git_commit_date 1726564137 1726569568
git_commit_sha 1429d59 5fc2763
release_version 1.40.0-SNAPSHOT~1429d59b51 1.40.0-SNAPSHOT~5fc276336e
start_time 2024-09-17T10:50:31 2024-09-17T10:57:20
See matching parameters
Baseline Candidate
application insecure-bank insecure-bank
ci_job_date 1726570997 1726570997
ci_job_id 641889977 641889977
ci_pipeline_id 44488952 44488952
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 10 metrics, 18 unstable metrics.

Request duration reports for insecure-bank 
gantt
    title insecure-bank - request duration [CI 0.99] : candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51
    dateFormat X
    axisFormat %s
section baseline
no_agent (368.999 µs) : 349, 389
.   : milestone, 369,
iast (479.511 µs) : 458, 501
.   : milestone, 480,
iast_FULL (548.947 µs) : 528, 570
.   : milestone, 549,
iast_GLOBAL (500.268 µs) : 479, 522
.   : milestone, 500,
iast_HARDCODED_SECRET_DISABLED (482.782 µs) : 460, 505
.   : milestone, 483,
iast_INACTIVE (445.784 µs) : 424, 468
.   : milestone, 446,
iast_TELEMETRY_OFF (470.789 µs) : 448, 494
.   : milestone, 471,
tracing (449.692 µs) : 426, 473
.   : milestone, 450,
section candidate
no_agent (364.055 µs) : 344, 384
.   : milestone, 364,
iast (478.958 µs) : 458, 500
.   : milestone, 479,
iast_FULL (548.264 µs) : 527, 570
.   : milestone, 548,
iast_GLOBAL (497.128 µs) : 476, 518
.   : milestone, 497,
iast_HARDCODED_SECRET_DISABLED (479.225 µs) : 458, 500
.   : milestone, 479,
iast_INACTIVE (442.76 µs) : 422, 464
.   : milestone, 443,
iast_TELEMETRY_OFF (476.066 µs) : 454, 499
.   : milestone, 476,
tracing (435.426 µs) : 416, 455
.   : milestone, 435,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 368.999 µs [349.033 µs, 388.964 µs] -
iast 479.511 µs [458.215 µs, 500.806 µs] 110.512 µs (29.9%)
iast_FULL 548.947 µs [527.807 µs, 570.086 µs] 179.948 µs (48.8%)
iast_GLOBAL 500.268 µs [478.754 µs, 521.782 µs] 131.269 µs (35.6%)
iast_HARDCODED_SECRET_DISABLED 482.782 µs [460.298 µs, 505.266 µs] 113.783 µs (30.8%)
iast_INACTIVE 445.784 µs [423.804 µs, 467.765 µs] 76.786 µs (20.8%)
iast_TELEMETRY_OFF 470.789 µs [447.598 µs, 493.98 µs] 101.79 µs (27.6%)
tracing 449.692 µs [426.017 µs, 473.366 µs] 80.693 µs (21.9%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 364.055 µs [344.328 µs, 383.782 µs] -
iast 478.958 µs [457.759 µs, 500.157 µs] 114.903 µs (31.6%)
iast_FULL 548.264 µs [526.998 µs, 569.529 µs] 184.209 µs (50.6%)
iast_GLOBAL 497.128 µs [476.16 µs, 518.095 µs] 133.073 µs (36.6%)
iast_HARDCODED_SECRET_DISABLED 479.225 µs [458.319 µs, 500.131 µs] 115.17 µs (31.6%)
iast_INACTIVE 442.76 µs [421.994 µs, 463.527 µs] 78.705 µs (21.6%)
iast_TELEMETRY_OFF 476.066 µs [453.607 µs, 498.525 µs] 112.011 µs (30.8%)
tracing 435.426 µs [415.517 µs, 455.336 µs] 71.371 µs (19.6%)
Request duration reports for petclinic 
gantt
    title petclinic - request duration [CI 0.99] : candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.345 ms) : 1326, 1365
.   : milestone, 1345,
appsec (1.732 ms) : 1708, 1756
.   : milestone, 1732,
appsec_no_iast (1.734 ms) : 1710, 1758
.   : milestone, 1734,
iast (1.462 ms) : 1440, 1484
.   : milestone, 1462,
profiling (1.476 ms) : 1452, 1500
.   : milestone, 1476,
tracing (1.463 ms) : 1439, 1488
.   : milestone, 1463,
section candidate
no_agent (1.326 ms) : 1306, 1346
.   : milestone, 1326,
appsec (1.714 ms) : 1691, 1738
.   : milestone, 1714,
appsec_no_iast (1.717 ms) : 1693, 1741
.   : milestone, 1717,
iast (1.482 ms) : 1459, 1505
.   : milestone, 1482,
profiling (1.467 ms) : 1442, 1491
.   : milestone, 1467,
tracing (1.462 ms) : 1438, 1487
.   : milestone, 1462,
Loading
  • baseline results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.345 ms [1.326 ms, 1.365 ms] -
appsec 1.732 ms [1.708 ms, 1.756 ms] 386.33 µs (28.7%)
appsec_no_iast 1.734 ms [1.71 ms, 1.758 ms] 388.377 µs (28.9%)
iast 1.462 ms [1.44 ms, 1.484 ms] 116.339 µs (8.6%)
profiling 1.476 ms [1.452 ms, 1.5 ms] 130.398 µs (9.7%)
tracing 1.463 ms [1.439 ms, 1.488 ms] 117.95 µs (8.8%)
  • candidate results
Variant Request duration [CI 0.99] Δ no_agent
no_agent 1.326 ms [1.306 ms, 1.346 ms] -
appsec 1.714 ms [1.691 ms, 1.738 ms] 388.353 µs (29.3%)
appsec_no_iast 1.717 ms [1.693 ms, 1.741 ms] 391.081 µs (29.5%)
iast 1.482 ms [1.459 ms, 1.505 ms] 156.228 µs (11.8%)
profiling 1.467 ms [1.442 ms, 1.491 ms] 140.758 µs (10.6%)
tracing 1.462 ms [1.438 ms, 1.487 ms] 136.392 µs (10.3%)

Dacapo

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master jpbempel/fix-overlapping-slot
git_commit_date 1726564137 1726569568
git_commit_sha 1429d59 5fc2763
release_version 1.40.0-SNAPSHOT~1429d59b51 1.40.0-SNAPSHOT~5fc276336e
See matching parameters
Baseline Candidate
application biojava biojava
ci_job_date 1726571527 1726571527
ci_job_id 641889978 641889978
ci_pipeline_id 44488952 44488952
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
variant appsec appsec

Summary

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

Execution time for tomcat 
gantt
    title tomcat - execution time [CI 0.99] : candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.469 ms) : 1458, 1481
.   : milestone, 1469,
appsec (2.311 ms) : 2270, 2352
.   : milestone, 2311,
iast (2.074 ms) : 2023, 2125
.   : milestone, 2074,
iast_GLOBAL (2.122 ms) : 2071, 2173
.   : milestone, 2122,
profiling (1.932 ms) : 1892, 1971
.   : milestone, 1932,
tracing (1.911 ms) : 1872, 1949
.   : milestone, 1911,
section candidate
no_agent (1.474 ms) : 1463, 1486
.   : milestone, 1474,
appsec (2.307 ms) : 2266, 2348
.   : milestone, 2307,
iast (2.071 ms) : 2019, 2122
.   : milestone, 2071,
iast_GLOBAL (2.123 ms) : 2071, 2174
.   : milestone, 2123,
profiling (1.93 ms) : 1890, 1970
.   : milestone, 1930,
tracing (1.916 ms) : 1877, 1954
.   : milestone, 1916,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.469 ms [1.458 ms, 1.481 ms] -
appsec 2.311 ms [2.27 ms, 2.352 ms] 841.538 µs (57.3%)
iast 2.074 ms [2.023 ms, 2.125 ms] 604.731 µs (41.2%)
iast_GLOBAL 2.122 ms [2.071 ms, 2.173 ms] 652.334 µs (44.4%)
profiling 1.932 ms [1.892 ms, 1.971 ms] 462.043 µs (31.4%)
tracing 1.911 ms [1.872 ms, 1.949 ms] 441.059 µs (30.0%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.474 ms [1.463 ms, 1.486 ms] -
appsec 2.307 ms [2.266 ms, 2.348 ms] 832.756 µs (56.5%)
iast 2.071 ms [2.019 ms, 2.122 ms] 596.27 µs (40.4%)
iast_GLOBAL 2.123 ms [2.071 ms, 2.174 ms] 648.433 µs (44.0%)
profiling 1.93 ms [1.89 ms, 1.97 ms] 455.645 µs (30.9%)
tracing 1.916 ms [1.877 ms, 1.954 ms] 441.631 µs (30.0%)
Execution time for biojava 
gantt
    title biojava - execution time [CI 0.99] : candidate=1.40.0-SNAPSHOT~5fc276336e, baseline=1.40.0-SNAPSHOT~1429d59b51
    dateFormat X
    axisFormat %s
section baseline
no_agent (15.038 s) : 15038000, 15038000
.   : milestone, 15038000,
appsec (15.191 s) : 15191000, 15191000
.   : milestone, 15191000,
iast (18.94 s) : 18940000, 18940000
.   : milestone, 18940000,
iast_GLOBAL (17.965 s) : 17965000, 17965000
.   : milestone, 17965000,
profiling (15.334 s) : 15334000, 15334000
.   : milestone, 15334000,
tracing (15.262 s) : 15262000, 15262000
.   : milestone, 15262000,
section candidate
no_agent (15.044 s) : 15044000, 15044000
.   : milestone, 15044000,
appsec (15.286 s) : 15286000, 15286000
.   : milestone, 15286000,
iast (19.202 s) : 19202000, 19202000
.   : milestone, 19202000,
iast_GLOBAL (18.043 s) : 18043000, 18043000
.   : milestone, 18043000,
profiling (15.041 s) : 15041000, 15041000
.   : milestone, 15041000,
tracing (15.074 s) : 15074000, 15074000
.   : milestone, 15074000,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 15.038 s [15.038 s, 15.038 s] -
appsec 15.191 s [15.191 s, 15.191 s] 153.0 ms (1.0%)
iast 18.94 s [18.94 s, 18.94 s] 3.902 s (25.9%)
iast_GLOBAL 17.965 s [17.965 s, 17.965 s] 2.927 s (19.5%)
profiling 15.334 s [15.334 s, 15.334 s] 296.0 ms (2.0%)
tracing 15.262 s [15.262 s, 15.262 s] 224.0 ms (1.5%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 15.044 s [15.044 s, 15.044 s] -
appsec 15.286 s [15.286 s, 15.286 s] 242.0 ms (1.6%)
iast 19.202 s [19.202 s, 19.202 s] 4.158 s (27.6%)
iast_GLOBAL 18.043 s [18.043 s, 18.043 s] 2.999 s (19.9%)
profiling 15.041 s [15.041 s, 15.041 s] -3.0 ms (-0.0%)
tracing 15.074 s [15.074 s, 15.074 s] 30.0 ms (0.2%)

@jpbempel jpbempel force-pushed the jpbempel/fix-overlapping-slot branch from 3512d83 to c3ae681 Compare September 16, 2024 13:34
@jpbempel
Fix local var hoisting
7b3812f 
Rewrite the algorithm for hoisting local vars:
Go through all local vars defined in the method and determine by name
all local var that are hoistable.
local vars are hoistable if there is no conflict by slot or by name
with another.
If conflict by slot and same name we cancel hoisting for this name. if
 name is different we allocating a new slot for one of the local.
If conflict by name and type are not compatible we cancel hoisting for
this name. If type compatible, we can add in hoistable list.
Then we are considering all hoistable variables by name and if there
are multiple occurrence for a name, we are merging those variables
in a new slot and rewrite var instructions to use this new slot.
if only one variable we just assign a new slot and rewrite the
instructions in the range.
@jpbempel jpbempel force-pushed the jpbempel/fix-overlapping-slot branch from c3ae681 to 7b3812f Compare September 16, 2024 13:35
shatzi
shatzi approved these changes Sep 16, 2024
View reviewed changes
...debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java
varInsnNode.var = newSlot;
}
}
if (insn instanceof IincInsnNode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

does this cover all the rewrite needed be done?
for example, while searching what iinc does, I found wide - which also can get variable index.

Copy link
Member Author

@jpbempel jpbempel Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

wide is very special instruction as this is a composite one. it modifies an existing instruction like iload or iinc to handle index more than 255 or incrementation > 128.
in ASM library decoding this instruction will result in either a VarInsnNode or IIncInsnNode because index are represented with int anyway. It means that we are covering all instructions that way.
Here the handling of wide instruction by ASM: https://github.com/llbit/ow2-asm/blob/a695934043d6f8f0aee3f6867c8dd167afd4aed8/src/org/objectweb/asm/ClassReader.java#L1066-L1073

...debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java
newVarNode = hoistableVars.get(0);
int oldIndex = newVarNode.index;
newVarNode.index = newVar(getType(newVarNode.desc)); // new slot for the local variable
rewriteLocalVarInsn(newVarNode, oldIndex, newVarNode.index);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that matter. but it seems we are rewrite (assign new slot) for variables that we should not.
For every hoist-able variable, even if there is no collision we rewrite it and sign it to a new slot.

Maybe add a logic that would create find new slots only for variable only if there is a conflict to be solved?

Copy link
Member Author

Choose a reason for hiding this comment

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

the LocalVariableTable is first optional, and then non-exhaustive. It does not prevent slot to be reused outside of what is declared in the LVT. By hoisting we extending the scope of the slot and we may conflict with another usage that we could not anticipated just by looking at the LVT. Here we are conservative and assign in any case a new slot that way we are isolating the variable for possible conflict outside of the LVT.

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java Outdated
assertCaptureLocals(snapshot.getCaptures().getReturn(), "localVarL0", "int", "0");
assertCaptureLocals(snapshot.getCaptures().getReturn(), "localVarL1", "int", "1");
assertCaptureLocals(snapshot.getCaptures().getReturn(), "localVarL2", "int", "2");
assertCaptureLocals(snapshot.getCaptures().getReturn(), "localVarL3", "int", "3");
assertCaptureLocals(snapshot.getCaptures().getReturn(), "localVarL4", "int", "4");
Copy link
Contributor

Choose a reason for hiding this comment

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

why localVarL4 cannot be hoisted?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java
TestSnapshotListener listener = installProbes(CLASS_NAME, probe);
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
int result = Reflect.onClass(testClass).call("main", "duplicateLocalDifferentScope").get();
assertEquals(28, result);
Copy link
Contributor

Choose a reason for hiding this comment

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

should we also asset 'ch' is not hoisted?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

@jpbempel jpbempel merged commit bb5d791 into master Sep 17, 2024
99 checks passed
@jpbempel jpbempel deleted the jpbempel/fix-overlapping-slot branch September 17, 2024 11:56
@github-actions github-actions bot added this to the 1.40.0 milestone Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shatzi shatzi shatzi approved these changes

@evanchooly evanchooly Awaiting requested review from evanchooly evanchooly is a code owner automatically assigned from DataDog/debugger-java

Assignees
No one assigned
Labels
comp: debugger Dynamic Instrumentation
Projects
None yet
Milestone
1.40.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jpbempel @shatzi