Performance: Cache method calls in loops and optimize string concaten… #3515

vogella · 2025-11-07T17:54:36Z

Apply low-risk performance optimizations across multiple bundles.

After review, this PR contains only the legitimate performance improvements:

Repeated method call caching:

MenuManagerShowProcessor: Cache menuModel.getChildren() to avoid repeated method calls in loop condition
WorkbenchPage: Cache getChildren() results to avoid repeated method calls in loops

String concatenation optimizations:

WizardsRegistryReader: Use StringBuilder for path construction in loop instead of repeated string concatenation

All changes are non-API breaking and maintain identical behavior while reducing unnecessary object allocations and method calls.

Note: Changes that provided no actual benefit (caching simple field accesses like List.size(), String.length(), or unnecessary variable extractions) have been removed.

github-actions · 2025-11-07T18:05:16Z

Test Results

3 018 files 3 018 suites 2h 16m 45s ⏱️
8 234 tests 7 985 ✅ 249 💤 0 ❌
23 622 runs 22 828 ✅ 794 💤 0 ❌

Results for commit 17b524d.

♻️ This comment has been updated with latest results.

BeckerWdf · 2025-11-10T06:58:31Z

bundles/org.eclipse.jface/src/org/eclipse/jface/action/ContributionManager.java

 	 */
 	public int indexOf(String id) {
-		for (int i = 0; i < contributions.size(); i++) {
+		int size = contributions.size();


isn't this something the compilier anyway optimizes?

Not likely, the size could change in the loop. I personally prefer this style:

for (int i = 0, size = contributions.size(); i < size; i++) {

Assuming the intent is not to deal with the size changing in the loop.

BeckerWdf · 2025-11-10T07:00:10Z

bundles/org.eclipse.jface/src/org/eclipse/jface/bindings/BindingManager.java

-					- currentTriggerSequence.format().length();
+			int bestLength = bestTriggerSequence.format().length();
+			int currentLength = currentTriggerSequence.format().length();
+			compareTo = bestLength - currentLength;


why should this be faster? the number of method calls does not change.

vogella · 2025-11-10T07:52:03Z

Thanks for the early feedback but I would prefer if draft reviews are not pro-actively reviewd. For example, the currentTriggerSequence.format().length(); change is obviously non-sense, I only did not yet find the time to update the PR.

So better spend time in reviewing non-draft PR to avoid spending time on something that is not yet ready to be reviewed.

vogella · 2025-11-10T08:57:27Z

These changes won't be noticeable to end users. They're technically correct optimizations, but the performance gain is likely unmeasurable. The main value is in code quality and avoiding unnecessary allocations, not in user-perceivable performance improvement.

laeubi · 2025-11-10T09:00:46Z

WizardsRegistryReader: Use StringBuilder for path construction in loop instead of repeated string concatenation

We have

just as you asked here #3517 I would suggest to enable such cleanups on a more general scope.

vogella · 2025-11-10T09:06:23Z

WizardsRegistryReader: Use StringBuilder for path construction in loop instead of repeated string concatenation

We have
just as you asked here #3517 I would suggest to enable such cleanups on a more general scope.

I tried this once in the past and it create tons of silly changes. I don't think this is one of the clean-up I would like to activate. The changes here are more dedicated.

laeubi · 2025-11-10T09:09:13Z

BTW we also have

vogella · 2025-11-10T09:12:29Z

Please put suggestions for new clean-up into this issue: #3517

laeubi · 2025-11-10T09:15:39Z

I tried this once in the past and it create tons of silly changes.

Pick one example, report it to JDT and it can be fixed (maybe even with the help of the AI).

@jjohnstn did a great job lately, but without using the cleanups and reporting issues things will never improve.

The changes here are more dedicated.

Dedicated to what? You said its all likely not noticeable by the user so claim they improve "performance" is maybe bit to big thing to claim.

But if the real goal is as claimed here

The main value is in code quality and avoiding unnecessary allocations

then it would be much more valuable to:

Use this particular example
Apply the cleanup and see if it works or not
report any issues and let them be fixed
loop to 1 until happy
1. enable the cleanup to improve on a larger scale.

vogella · 2025-11-10T09:21:11Z

I tried this once in the past and it create tons of silly changes.

Pick one example, report it to JDT and it can be fixed (maybe even with the help of the AI).

The conversion was not wrong but for places which not performance relevant like debug output and increased the code complexity IMHO. Like always you are free to investigate yourself (to which you usually reply you do not have the time for that which is valid and also the reason why I do not plan to work on this as I also do not have the time for this).

laeubi · 2025-11-10T09:41:46Z

The conversion was not wrong but for places which not performance relevant like debug output and increased the code complexity IMHO

This is a valid concern that could probably be addressed

Like always you are free to investigate yourself

You asked for where you are could have used cleanups and I just gave you examples, apart from that I always have incrementally enabled cleanups and investigate what are useful and reporting bugs:

https://github.com/eclipse-jdt/eclipse.jdt.ui/issues?q=is%3Aissue%20state%3Aclosed%20author%3Alaeubi%20cleanup

there is even one (open) enhancement request:

Enhance Cleanup to replace deprecated method calls to support interface methods eclipse-jdt/eclipse.jdt.ui#2232

So yes everyone is busy but especially then we should not have 100'd of fragmented micro optimizations.

vogella · 2025-11-11T09:06:16Z

Trying to address the flaky Mac test here: #3532

concatenation Apply low-risk performance optimizations across multiple bundles: Loop optimizations - cache method results: - StyledString: Cache otherRuns.size() before loop - ContributionManager: Cache contributions.size() in indexOf() and duplicate removal - ContentProposalAdapter: Cache autoActivateString.length() in loop - DialogSettings: Cache s.length() before character iteration - MenuManagerShowProcessor: Cache menuModel.getChildren() to avoid repeated calls - WorkbenchPage: Cache getChildren() results to avoid expensive repeated calls String concatenation optimizations: - WizardsRegistryReader: Use StringBuilder for path construction in loop - NewContentTypeDialog: Use StringBuilder for name generation in while loop Repeated expensive calls: - BindingManager: Cache format().length() results in comparison - MenuManagerShowProcessor: Cache getChildren() in loop condition and body All changes are non-API breaking and maintain identical behavior while reducing unnecessary object allocations and method calls.

vogella marked this pull request as draft November 7, 2025 17:54

vogella force-pushed the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch 2 times, most recently from ea72901 to 3b853bf Compare November 7, 2025 18:24

BeckerWdf reviewed Nov 10, 2025

View reviewed changes

vogella force-pushed the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch 3 times, most recently from 7d5510c to 2ddd35f Compare November 10, 2025 08:56

laeubi mentioned this pull request Nov 10, 2025

More clean-up actions #3517

Open

vogella force-pushed the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch from 2ddd35f to 856c71a Compare November 10, 2025 09:07

vogella force-pushed the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch 3 times, most recently from 9a47563 to b795d93 Compare November 11, 2025 06:38

vogella force-pushed the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch from b795d93 to 59bb59e Compare November 11, 2025 14:31

vogella marked this pull request as ready for review November 11, 2025 16:13

vogella marked this pull request as draft November 11, 2025 16:13

vogella force-pushed the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch 3 times, most recently from 1a7a7d8 to e8a2f85 Compare November 13, 2025 09:49

vogella force-pushed the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch from e8a2f85 to 17b524d Compare November 13, 2025 17:14

vogella marked this pull request as ready for review November 13, 2025 19:38

vogella merged commit 343fd57 into eclipse-platform:master Nov 14, 2025
18 checks passed

vogella deleted the claude/low-risk-perf-improvements-011CUpuMnXBm4FNfKKNjyHUZ branch November 14, 2025 00:05

Performance: Cache method calls in loops and optimize string concaten… #3515

Performance: Cache method calls in loops and optimize string concaten… #3515

Conversation

vogella commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

BeckerWdf Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

merks Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

BeckerWdf Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

vogella commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vogella commented Nov 10, 2025

Uh oh!

laeubi commented Nov 10, 2025

Uh oh!

vogella commented Nov 10, 2025

Uh oh!

laeubi commented Nov 10, 2025

Uh oh!

vogella commented Nov 10, 2025

Uh oh!

laeubi commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vogella commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laeubi commented Nov 10, 2025

Uh oh!

vogella commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vogella commented Nov 7, 2025 •

edited

Loading

github-actions bot commented Nov 7, 2025 •

edited

Loading

vogella commented Nov 10, 2025 •

edited

Loading

laeubi commented Nov 10, 2025 •

edited

Loading

vogella commented Nov 10, 2025 •

edited

Loading

vogella commented Nov 11, 2025 •

edited

Loading