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

Replace StringBuilder#append(String) with StringBuilder#append(char) #8008

Merged
merged 2 commits into from
Nov 26, 2024

Conversation

smola
Copy link
Member

@smola smola commented Nov 25, 2024

What Does This Do

Replace StringBuilder#append(String) with StringBuilder#append(char) when a single character is used.

Motivation

Detected with java-best-practices/sb-append-char static analysis rule.

Additional Notes

Contributor Checklist

Jira ticket: [PROJ-IDENT]

@smola smola added tag: no release notes Changes to exclude from release notes type: refactoring labels Nov 25, 2024
@pr-commenter
Copy link

pr-commenter bot commented Nov 25, 2024

Debugger benchmarks

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
ci_job_date 1732528930 1732529321
end_time 2024-11-25T10:03:26 2024-11-25T10:09:58
git_branch master smola/single-char
git_commit_sha b36006c 0fcd18e
start_time 2024-11-25T10:02:11 2024-11-25T10:08:42
See matching parameters
Baseline Candidate
ci_job_id 718499533 718499533
ci_pipeline_id 49833603 49833603
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
git_commit_date 1732527941 1732527941

Summary

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

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:loop better
[-723.086µs; -684.346µs] or [-6.638%; -6.282%]
better
[-754.246µs; -685.296µs] or [-6.812%; -6.190%]
better
[-750.200µs; -631.285µs] or [-6.734%; -5.667%]
better
[-934.660µs; -508.794µs] or [-8.098%; -4.408%]
better
[+4.723op/s; +7.182op/s] or [+5.290%; +8.044%]
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
[-61.067µs; +30.030µs] or [-21.687%; +10.665%]
unstable
[-80.728µs; +39.283µs] or [-24.754%; +12.045%]
unstable
[-89.696µs; +48.576µs] or [-26.246%; +14.214%]
unstable
[-1035.227µs; +372.891µs] or [-109.493%; +39.440%]
same
scenario:basic same same unstable
[-17.052µs; +22.418µs] or [-5.246%; +6.897%]
unstable
[-277.770µs; +94.856µs] or [-44.603%; +15.232%]
unstable
[-155.976op/s; +155.976op/s] or [-6.239%; +6.239%]
Request duration reports for reports
gantt
    title reports - request duration [CI 0.99] : candidate=None, baseline=None
    dateFormat X
    axisFormat %s
section baseline
noprobe (326.122 µs) : 249, 403
.   : milestone, 326,
basic (311.992 µs) : 296, 328
.   : milestone, 312,
loop (11.072 ms) : 11034, 11110
.   : milestone, 11072,
section candidate
noprobe (305.4 µs) : 282, 329
.   : milestone, 305,
basic (317.267 µs) : 309, 325
.   : milestone, 317,
loop (10.352 ms) : 10327, 10377
.   : milestone, 10352,
Loading
  • baseline results
Scenario Request median duration [CI 0.99]
noprobe 326.122 µs [248.761 µs, 403.483 µs]
basic 311.992 µs [296.415 µs, 327.569 µs]
loop 11.072 ms [11.034 ms, 11.11 ms]
  • candidate results
Scenario Request median duration [CI 0.99]
noprobe 305.4 µs [281.685 µs, 329.114 µs]
basic 317.267 µs [309.31 µs, 325.224 µs]
loop 10.352 ms [10.327 ms, 10.377 ms]

@pr-commenter
Copy link

pr-commenter bot commented Nov 25, 2024

Benchmarks

Startup

Load

Dacapo

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master smola/single-char
git_commit_date 1732532650 1732535609
git_commit_sha f2d21ae 7d452a8
release_version 1.43.0-SNAPSHOT~f2d21ae588 1.44.0-SNAPSHOT~7d452a8e9a
See matching parameters
Baseline Candidate
application biojava biojava
ci_job_date 1732537497 1732537497
ci_job_id 718676643 718676643
ci_pipeline_id 49844883 49844883
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 biojava
gantt
    title biojava - execution time [CI 0.99] : candidate=1.44.0-SNAPSHOT~7d452a8e9a, baseline=1.43.0-SNAPSHOT~f2d21ae588
    dateFormat X
    axisFormat %s
section baseline
no_agent (14.942 s) : 14942000, 14942000
.   : milestone, 14942000,
appsec (15.077 s) : 15077000, 15077000
.   : milestone, 15077000,
iast (18.604 s) : 18604000, 18604000
.   : milestone, 18604000,
iast_GLOBAL (17.737 s) : 17737000, 17737000
.   : milestone, 17737000,
profiling (15.099 s) : 15099000, 15099000
.   : milestone, 15099000,
tracing (15.131 s) : 15131000, 15131000
.   : milestone, 15131000,
section candidate
no_agent (14.928 s) : 14928000, 14928000
.   : milestone, 14928000,
appsec (15.063 s) : 15063000, 15063000
.   : milestone, 15063000,
iast (18.723 s) : 18723000, 18723000
.   : milestone, 18723000,
iast_GLOBAL (18.252 s) : 18252000, 18252000
.   : milestone, 18252000,
profiling (14.935 s) : 14935000, 14935000
.   : milestone, 14935000,
tracing (14.844 s) : 14844000, 14844000
.   : milestone, 14844000,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 14.942 s [14.942 s, 14.942 s] -
appsec 15.077 s [15.077 s, 15.077 s] 135.0 ms (0.9%)
iast 18.604 s [18.604 s, 18.604 s] 3.662 s (24.5%)
iast_GLOBAL 17.737 s [17.737 s, 17.737 s] 2.795 s (18.7%)
profiling 15.099 s [15.099 s, 15.099 s] 157.0 ms (1.1%)
tracing 15.131 s [15.131 s, 15.131 s] 189.0 ms (1.3%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 14.928 s [14.928 s, 14.928 s] -
appsec 15.063 s [15.063 s, 15.063 s] 135.0 ms (0.9%)
iast 18.723 s [18.723 s, 18.723 s] 3.795 s (25.4%)
iast_GLOBAL 18.252 s [18.252 s, 18.252 s] 3.324 s (22.3%)
profiling 14.935 s [14.935 s, 14.935 s] 7.0 ms (0.0%)
tracing 14.844 s [14.844 s, 14.844 s] -84.0 ms (-0.6%)
Execution time for tomcat
gantt
    title tomcat - execution time [CI 0.99] : candidate=1.44.0-SNAPSHOT~7d452a8e9a, baseline=1.43.0-SNAPSHOT~f2d21ae588
    dateFormat X
    axisFormat %s
section baseline
no_agent (1.469 ms) : 1457, 1480
.   : milestone, 1469,
appsec (2.341 ms) : 2299, 2383
.   : milestone, 2341,
iast (2.092 ms) : 2039, 2145
.   : milestone, 2092,
iast_GLOBAL (2.136 ms) : 2083, 2189
.   : milestone, 2136,
profiling (1.956 ms) : 1914, 1999
.   : milestone, 1956,
tracing (1.927 ms) : 1887, 1968
.   : milestone, 1927,
section candidate
no_agent (1.473 ms) : 1462, 1485
.   : milestone, 1473,
appsec (2.35 ms) : 2308, 2392
.   : milestone, 2350,
iast (2.088 ms) : 2035, 2141
.   : milestone, 2088,
iast_GLOBAL (2.133 ms) : 2079, 2186
.   : milestone, 2133,
profiling (1.941 ms) : 1899, 1983
.   : milestone, 1941,
tracing (1.929 ms) : 1889, 1970
.   : milestone, 1929,
Loading
  • baseline results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.469 ms [1.457 ms, 1.48 ms] -
appsec 2.341 ms [2.299 ms, 2.383 ms] 872.174 µs (59.4%)
iast 2.092 ms [2.039 ms, 2.145 ms] 623.223 µs (42.4%)
iast_GLOBAL 2.136 ms [2.083 ms, 2.189 ms] 666.96 µs (45.4%)
profiling 1.956 ms [1.914 ms, 1.999 ms] 487.606 µs (33.2%)
tracing 1.927 ms [1.887 ms, 1.968 ms] 458.633 µs (31.2%)
  • candidate results
Variant Execution Time [CI 0.99] Δ no_agent
no_agent 1.473 ms [1.462 ms, 1.485 ms] -
appsec 2.35 ms [2.308 ms, 2.392 ms] 876.601 µs (59.5%)
iast 2.088 ms [2.035 ms, 2.141 ms] 614.556 µs (41.7%)
iast_GLOBAL 2.133 ms [2.079 ms, 2.186 ms] 659.499 µs (44.8%)
profiling 1.941 ms [1.899 ms, 1.983 ms] 467.505 µs (31.7%)
tracing 1.929 ms [1.889 ms, 1.97 ms] 455.973 µs (30.9%)

Copy link
Contributor

@PerfectSlayer PerfectSlayer left a comment

Choose a reason for hiding this comment

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

I hope you used a tool to make all the changes 😅

@smola
Copy link
Member Author

smola commented Nov 25, 2024

@PerfectSlayer Sure, just used sed 😉

Anyway, the debugger benchmarks will need to be checked, these results are weird.

@smola smola force-pushed the smola/single-char branch from 5a15ca9 to 0fcd18e Compare November 25, 2024 09:45
@smola smola marked this pull request as ready for review November 25, 2024 11:53
@smola smola requested review from a team as code owners November 25, 2024 11:53
@smola smola requested review from shatzi, nikita-tkachenko-datadog and mcculls and removed request for a team November 25, 2024 11:53
@jpbempel
Copy link
Member

The rule is kind of useless. even if append on char is marginally faster that string, before make it noticeable you need a very critical million iterations to see a difference, and most of the time on a constant.
I found those rules more counter-productive than relevant. just my 2 cents

@smola
Copy link
Member Author

smola commented Nov 26, 2024

Ack. I'll merge this as a one-off, since they won't be harmful anyway.

@smola smola merged commit 92e8023 into master Nov 26, 2024
103 checks passed
@smola smola deleted the smola/single-char branch November 26, 2024 08:10
@github-actions github-actions bot added this to the 1.44.0 milestone Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tag: no release notes Changes to exclude from release notes type: refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants