Skip to content

Conversation

@ivancea
Copy link
Contributor

@ivancea ivancea commented May 30, 2025

Replace non-multithreaded uses of StringBuffer to StringBuilder.

From the StringBuffer javadoc:

As of release JDK 5, this class has been supplemented with an equivalent class designed for use by a single thread, StringBuilder. The StringBuilder class should generally be used in preference to this one, as it supports all of the same operations but it is faster, as it performs no synchronization.

Just in case, and out of curiosity, made a little microbenchmark to check the difference both between a StringBuffer and a StringBuilder, and between a synchronized and non-synchronized method. The results are "as expected", and tell me that this won't affect negatively:

Benchmark                    (type)  (useSynchronized)   Score  Error  Units
TestsBenchmark.run   empty_function              false   0.089  0.004  ns/op
TestsBenchmark.run   empty_function               true   5.777  0.109  ns/op
TestsBenchmark.run  string_building              false  10.191  0.476  ns/op
TestsBenchmark.run  string_building               true  28.733  1.210  ns/op

The main reason to do this isn't performance anyway, but to avoid using this class when it's not needed.

@ivancea ivancea added >non-issue Team:Core/Infra Meta label for core/infra team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Security Meta label for security team auto-backport Automatically create backport pull requests when merged v8.19.0 v9.1.0 v9.0.3 labels May 30, 2025
@ivancea ivancea marked this pull request as ready for review May 30, 2025 13:34
@ivancea ivancea requested a review from a team as a code owner May 30, 2025 13:34
@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label and removed Team:Core/Infra Meta label for core/infra team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Security Meta label for security team labels May 30, 2025
@ivancea ivancea added :Analytics/SQL SQL querying Team:Core/Infra Meta label for core/infra team Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Security Meta label for security team labels May 30, 2025
@elasticsearchmachine elasticsearchmachine removed Team:Core/Infra Meta label for core/infra team Team:Security Meta label for security team needs:triage Requires assignment of a team area label labels May 30, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@ivancea
Copy link
Contributor Author

ivancea commented May 30, 2025

(Failing test is unrelated)

Copy link
Contributor

@craigtaverner craigtaverner left a comment

Choose a reason for hiding this comment

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

Wow! I thought this had been done ages ago. I'm a little surprised to see these left-over bits.

@ivancea ivancea enabled auto-merge (squash) June 2, 2025 10:23
@ivancea ivancea merged commit d597e50 into elastic:main Jun 2, 2025
16 of 18 checks passed
@ivancea ivancea deleted the string-buffer-to-builder branch June 2, 2025 11:29
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.19
9.0

ivancea added a commit to ivancea/elasticsearch that referenced this pull request Jun 2, 2025
ivancea added a commit to ivancea/elasticsearch that referenced this pull request Jun 2, 2025
mridula-s109 pushed a commit to mridula-s109/elasticsearch that referenced this pull request Jun 3, 2025
joshua-adams-1 pushed a commit to joshua-adams-1/elasticsearch that referenced this pull request Jun 3, 2025
Samiul-TheSoccerFan pushed a commit to Samiul-TheSoccerFan/elasticsearch that referenced this pull request Jun 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/SQL SQL querying auto-backport Automatically create backport pull requests when merged >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.0.3 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants