Remove cleanup-on-shutdown for temporary files #8746
+31
−42
Conversation
This file contains hidden or 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
BenchmarksStartupParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 66 metrics, 5 unstable metrics. Startup time reports for petclinicgantt
title petclinic - global startup overhead: candidate=1.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.01 s) : 0, 1009615
Total [baseline] (10.517 s) : 0, 10516564
Agent [candidate] (1.006 s) : 0, 1005569
Total [candidate] (10.502 s) : 0, 10502216
section appsec
Agent [baseline] (1.149 s) : 0, 1148971
Total [baseline] (10.639 s) : 0, 10639476
Agent [candidate] (1.151 s) : 0, 1151117
Total [candidate] (10.666 s) : 0, 10665767
section iast
Agent [baseline] (1.137 s) : 0, 1137046
Total [baseline] (10.855 s) : 0, 10854583
Agent [candidate] (1.139 s) : 0, 1138720
Total [candidate] (10.897 s) : 0, 10896940
section profiling
Agent [baseline] (1.251 s) : 0, 1250947
Total [baseline] (10.795 s) : 0, 10795301
Agent [candidate] (1.261 s) : 0, 1260796
Total [candidate] (10.812 s) : 0, 10811674
gantt
title petclinic - break down per module: candidate=1.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (672.171 ms) : 0, 672171
BytebuddyAgent [candidate] (669.389 ms) : 0, 669389
GlobalTracer [baseline] (241.368 ms) : 0, 241368
GlobalTracer [candidate] (240.581 ms) : 0, 240581
AppSec [baseline] (55.086 ms) : 0, 55086
AppSec [candidate] (54.664 ms) : 0, 54664
Debugger [baseline] (6.183 ms) : 0, 6183
Debugger [candidate] (6.152 ms) : 0, 6152
Remote Config [baseline] (703.161 µs) : 0, 703
Remote Config [candidate] (711.828 µs) : 0, 712
Telemetry [baseline] (10.638 ms) : 0, 10638
Telemetry [candidate] (10.62 ms) : 0, 10620
section appsec
BytebuddyAgent [baseline] (688.123 ms) : 0, 688123
BytebuddyAgent [candidate] (690.039 ms) : 0, 690039
GlobalTracer [baseline] (236.872 ms) : 0, 236872
GlobalTracer [candidate] (237.137 ms) : 0, 237137
IAST [baseline] (22.036 ms) : 0, 22036
IAST [candidate] (21.773 ms) : 0, 21773
AppSec [baseline] (174.903 ms) : 0, 174903
AppSec [candidate] (174.527 ms) : 0, 174527
Debugger [baseline] (5.857 ms) : 0, 5857
Debugger [candidate] (5.912 ms) : 0, 5912
Remote Config [baseline] (647.892 µs) : 0, 648
Remote Config [candidate] (640.458 µs) : 0, 640
Telemetry [baseline] (8.122 ms) : 0, 8122
Telemetry [candidate] (8.544 ms) : 0, 8544
section iast
BytebuddyAgent [baseline] (789.854 ms) : 0, 789854
BytebuddyAgent [candidate] (790.818 ms) : 0, 790818
GlobalTracer [baseline] (230.176 ms) : 0, 230176
GlobalTracer [candidate] (230.299 ms) : 0, 230299
IAST [baseline] (22.807 ms) : 0, 22807
IAST [candidate] (22.839 ms) : 0, 22839
AppSec [baseline] (56.444 ms) : 0, 56444
AppSec [candidate] (56.861 ms) : 0, 56861
Debugger [baseline] (5.882 ms) : 0, 5882
Debugger [candidate] (5.938 ms) : 0, 5938
Remote Config [baseline] (589.313 µs) : 0, 589
Remote Config [candidate] (599.48 µs) : 0, 599
Telemetry [baseline] (7.893 ms) : 0, 7893
Telemetry [candidate] (7.954 ms) : 0, 7954
section profiling
BytebuddyAgent [baseline] (661.153 ms) : 0, 661153
BytebuddyAgent [candidate] (666.536 ms) : 0, 666536
GlobalTracer [baseline] (375.059 ms) : 0, 375059
GlobalTracer [candidate] (377.149 ms) : 0, 377149
AppSec [baseline] (53.888 ms) : 0, 53888
AppSec [candidate] (53.693 ms) : 0, 53693
Debugger [baseline] (6.127 ms) : 0, 6127
Debugger [candidate] (6.169 ms) : 0, 6169
Remote Config [baseline] (648.424 µs) : 0, 648
Remote Config [candidate] (652.489 µs) : 0, 652
Telemetry [baseline] (8.124 ms) : 0, 8124
Telemetry [candidate] (8.258 ms) : 0, 8258
ProfilingAgent [baseline] (95.916 ms) : 0, 95916
ProfilingAgent [candidate] (97.946 ms) : 0, 97946
Profiling [baseline] (95.941 ms) : 0, 95941
Profiling [candidate] (97.972 ms) : 0, 97972
Startup time reports for insecure-bankgantt
title insecure-bank - global startup overhead: candidate=1.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section tracing
Agent [baseline] (1.003 s) : 0, 1002807
Total [baseline] (8.615 s) : 0, 8614731
Agent [candidate] (1.01 s) : 0, 1010120
Total [candidate] (8.633 s) : 0, 8633296
section iast
Agent [baseline] (1.139 s) : 0, 1139004
Total [baseline] (9.206 s) : 0, 9206293
Agent [candidate] (1.134 s) : 0, 1134330
Total [candidate] (9.145 s) : 0, 9144886
section iast_HARDCODED_SECRET_DISABLED
Agent [baseline] (1.133 s) : 0, 1133058
Total [baseline] (9.153 s) : 0, 9153413
Agent [candidate] (1.141 s) : 0, 1141225
Total [candidate] (9.194 s) : 0, 9193567
section iast_TELEMETRY_OFF
Agent [baseline] (1.14 s) : 0, 1139830
Total [baseline] (9.148 s) : 0, 9148418
Agent [candidate] (1.146 s) : 0, 1145865
Total [candidate] (9.185 s) : 0, 9185400
gantt
title insecure-bank - break down per module: candidate=1.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section tracing
BytebuddyAgent [baseline] (668.733 ms) : 0, 668733
BytebuddyAgent [candidate] (673.701 ms) : 0, 673701
GlobalTracer [baseline] (238.696 ms) : 0, 238696
GlobalTracer [candidate] (240.667 ms) : 0, 240667
AppSec [baseline] (54.502 ms) : 0, 54502
AppSec [candidate] (54.759 ms) : 0, 54759
Debugger [baseline] (6.887 ms) : 0, 6887
Debugger [candidate] (6.162 ms) : 0, 6162
Remote Config [baseline] (709.338 µs) : 0, 709
Remote Config [candidate] (696.514 µs) : 0, 697
Telemetry [baseline] (9.843 ms) : 0, 9843
Telemetry [candidate] (10.622 ms) : 0, 10622
section iast
BytebuddyAgent [baseline] (793.003 ms) : 0, 793003
BytebuddyAgent [candidate] (788.265 ms) : 0, 788265
GlobalTracer [baseline] (229.11 ms) : 0, 229110
GlobalTracer [candidate] (229.654 ms) : 0, 229654
IAST [baseline] (22.651 ms) : 0, 22651
IAST [candidate] (22.646 ms) : 0, 22646
AppSec [baseline] (55.678 ms) : 0, 55678
AppSec [candidate] (55.976 ms) : 0, 55976
Debugger [baseline] (5.85 ms) : 0, 5850
Debugger [candidate] (5.893 ms) : 0, 5893
Remote Config [baseline] (581.458 µs) : 0, 581
Remote Config [candidate] (606.02 µs) : 0, 606
Telemetry [baseline] (7.859 ms) : 0, 7859
Telemetry [candidate] (7.917 ms) : 0, 7917
section iast_HARDCODED_SECRET_DISABLED
BytebuddyAgent [baseline] (788.295 ms) : 0, 788295
BytebuddyAgent [candidate] (794.055 ms) : 0, 794055
GlobalTracer [baseline] (229.034 ms) : 0, 229034
GlobalTracer [candidate] (230.354 ms) : 0, 230354
IAST [baseline] (23.26 ms) : 0, 23260
IAST [candidate] (22.682 ms) : 0, 22682
AppSec [baseline] (54.92 ms) : 0, 54920
AppSec [candidate] (56.287 ms) : 0, 56287
Debugger [baseline] (5.843 ms) : 0, 5843
Debugger [candidate] (5.838 ms) : 0, 5838
Remote Config [baseline] (581.832 µs) : 0, 582
Remote Config [candidate] (581.284 µs) : 0, 581
Telemetry [baseline] (7.852 ms) : 0, 7852
Telemetry [candidate] (7.846 ms) : 0, 7846
section iast_TELEMETRY_OFF
BytebuddyAgent [baseline] (791.99 ms) : 0, 791990
BytebuddyAgent [candidate] (796.751 ms) : 0, 796751
GlobalTracer [baseline] (231.054 ms) : 0, 231054
GlobalTracer [candidate] (232.146 ms) : 0, 232146
IAST [baseline] (22.545 ms) : 0, 22545
IAST [candidate] (22.501 ms) : 0, 22501
AppSec [baseline] (56.453 ms) : 0, 56453
AppSec [candidate] (56.596 ms) : 0, 56596
Debugger [baseline] (5.891 ms) : 0, 5891
Debugger [candidate] (5.88 ms) : 0, 5880
Remote Config [baseline] (613.52 µs) : 0, 614
Remote Config [candidate] (596.264 µs) : 0, 596
Telemetry [baseline] (7.763 ms) : 0, 7763
Telemetry [candidate] (7.775 ms) : 0, 7775
LoadParameters
See matching parameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 12 metrics, 18 unstable metrics. Request duration reports for insecure-bankgantt
title insecure-bank - request duration [CI 0.99] : candidate=1.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section baseline
no_agent (382.154 µs) : 362, 402
. : milestone, 382,
iast (532.161 µs) : 509, 556
. : milestone, 532,
iast_FULL (742.644 µs) : 719, 766
. : milestone, 743,
iast_GLOBAL (573.492 µs) : 550, 597
. : milestone, 573,
iast_HARDCODED_SECRET_DISABLED (536.25 µs) : 514, 559
. : milestone, 536,
iast_INACTIVE (477.984 µs) : 455, 501
. : milestone, 478,
iast_TELEMETRY_OFF (520.526 µs) : 498, 543
. : milestone, 521,
tracing (471.834 µs) : 449, 494
. : milestone, 472,
section candidate
no_agent (386.646 µs) : 367, 406
. : milestone, 387,
iast (524.405 µs) : 501, 548
. : milestone, 524,
iast_FULL (745.743 µs) : 722, 769
. : milestone, 746,
iast_GLOBAL (586.751 µs) : 563, 611
. : milestone, 587,
iast_HARDCODED_SECRET_DISABLED (539.178 µs) : 516, 562
. : milestone, 539,
iast_INACTIVE (477.261 µs) : 455, 500
. : milestone, 477,
iast_TELEMETRY_OFF (516.84 µs) : 494, 540
. : milestone, 517,
tracing (476.868 µs) : 455, 499
. : milestone, 477,
Request duration reports for petclinicgantt
title petclinic - request duration [CI 0.99] : candidate=1.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section baseline
no_agent (1.373 ms) : 1354, 1392
. : milestone, 1373,
appsec (1.759 ms) : 1734, 1783
. : milestone, 1759,
appsec_no_iast (1.744 ms) : 1721, 1768
. : milestone, 1744,
code_origins (1.708 ms) : 1681, 1735
. : milestone, 1708,
iast (1.542 ms) : 1518, 1566
. : milestone, 1542,
profiling (1.524 ms) : 1499, 1548
. : milestone, 1524,
tracing (1.515 ms) : 1490, 1540
. : milestone, 1515,
section candidate
no_agent (1.382 ms) : 1361, 1402
. : milestone, 1382,
appsec (1.752 ms) : 1729, 1775
. : milestone, 1752,
appsec_no_iast (1.744 ms) : 1720, 1768
. : milestone, 1744,
code_origins (1.7 ms) : 1672, 1727
. : milestone, 1700,
iast (1.513 ms) : 1489, 1537
. : milestone, 1513,
profiling (1.569 ms) : 1545, 1594
. : milestone, 1569,
tracing (1.505 ms) : 1480, 1530
. : milestone, 1505,
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.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section baseline
no_agent (14.951 s) : 14951000, 14951000
. : milestone, 14951000,
appsec (15.043 s) : 15043000, 15043000
. : milestone, 15043000,
iast (19.111 s) : 19111000, 19111000
. : milestone, 19111000,
iast_GLOBAL (17.78 s) : 17780000, 17780000
. : milestone, 17780000,
profiling (14.858 s) : 14858000, 14858000
. : milestone, 14858000,
tracing (15.085 s) : 15085000, 15085000
. : milestone, 15085000,
section candidate
no_agent (14.971 s) : 14971000, 14971000
. : milestone, 14971000,
appsec (15.028 s) : 15028000, 15028000
. : milestone, 15028000,
iast (18.441 s) : 18441000, 18441000
. : milestone, 18441000,
iast_GLOBAL (17.839 s) : 17839000, 17839000
. : milestone, 17839000,
profiling (15.057 s) : 15057000, 15057000
. : milestone, 15057000,
tracing (15.087 s) : 15087000, 15087000
. : milestone, 15087000,
Execution time for tomcatgantt
title tomcat - execution time [CI 0.99] : candidate=1.49.0-SNAPSHOT~7d91b630c7, baseline=1.49.0-SNAPSHOT~74633afbc6
dateFormat X
axisFormat %s
section baseline
no_agent (1.474 ms) : 1462, 1485
. : milestone, 1474,
appsec (2.389 ms) : 2342, 2437
. : milestone, 2389,
iast (2.176 ms) : 2115, 2237
. : milestone, 2176,
iast_GLOBAL (2.218 ms) : 2158, 2279
. : milestone, 2218,
profiling (2.029 ms) : 1979, 2078
. : milestone, 2029,
tracing (1.99 ms) : 1943, 2037
. : milestone, 1990,
section candidate
no_agent (1.474 ms) : 1463, 1486
. : milestone, 1474,
appsec (2.397 ms) : 2348, 2445
. : milestone, 2397,
iast (2.185 ms) : 2123, 2246
. : milestone, 2185,
iast_GLOBAL (2.213 ms) : 2152, 2274
. : milestone, 2213,
profiling (2.028 ms) : 1978, 2078
. : milestone, 2028,
tracing (1.987 ms) : 1941, 2034
. : milestone, 1987,
|
There was a problem hiding this comment.
Pull Request Overview
This PR removes the shutdown hook–based temporary file cleanup from the TempLocationManager to avoid race conditions with profiling snapshot generation. Key changes include:
- Removing the shutdown hook and self-cleanup parameter from TempLocationManager.
- Updating tests to call cleanup() without the self-cleanup flag.
- Introducing a new ignore(Path) method to allow specific temporary paths to be exempted from cleanup.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| dd-java-agent/agent-profiling/profiling-controller/src/test/java/com/datadog/profiling/controller/TempLocationManagerTest.java | Updated cleanup method calls in tests to remove selfCleanup parameter. |
| dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/TempLocationManager.java | Removed shutdown hook logic and associated self-cleanup code; modified the CleanupHook interface and CleanupVisitor accordingly; added an ignore(Path) method. |
| dd-java-agent/agent-profiling/profiling-controller-openjdk/src/main/java/com/datadog/profiling/controller/openjdk/OpenJdkController.java | Updated to retrieve the TempLocationManager instance before resolving the JFR repository path. |
Comments suppressed due to low confidence (3)
dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/TempLocationManager.java:299
- The removal of the shutdown hook is in line with the PR purpose; please ensure that the cooperative cleanup mechanism adequately handles leftover temporary files in all runtime scenarios.
Runtime.getRuntime().addShutdownHook(selfCleanup);
dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/TempLocationManager.java:365
- Consider adding unit tests and JavaDoc for the new ignore(Path path) method to ensure its behavior is well validated.
public void ignore(Path path) {
dd-java-agent/agent-profiling/profiling-controller/src/main/java/com/datadog/profiling/controller/TempLocationManager.java:105
- [nitpick] The removal of the self-cleanup check in preVisitDirectory affects the exclusion of the JFR repository. Please confirm this change is intentional and update the inline comments to clarify the intended behavior.
if (cleanSelf && JFR_DIR_PATTERN.matcher(dir.getFileName().toString()).matches()) {
MattAlp
approved these changes
Apr 29, 2025
jbachorik
force-pushed
the
jb/avoid_jfr_cleanup
branch
from
6730e44 to
7d91b63
Compare
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
It removes the 'cleanup-on-shutdown' functionality from the
TempLocationManagerMotivation
This cleanup can race against profiling-snapshot-on-shutdown and prevent those profiling snapshots to be sucessfully generated.
All of this for a small benefit of not having a few MiB of temp files for slightly longer time - the cooperative cleanup, when any other java process with the profiler starts up, will take care of those file anyway.
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any usefull labelsclose,fixor any linking keywords when referencing an issue.Use
solvesinstead, and assign the PR milestone to the issueJira ticket: PROF-11665