Resolve editor and view references without repeated linear scans - #4231
Open
vogella wants to merge 1 commit into
Open
Resolve editor and view references without repeated linear scans#4231vogella wants to merge 1 commit into
vogella wants to merge 1 commit into
Conversation
getOrderedEditorReferences, getSortedEditorReferences and getSortedParts resolved every model element by scanning the reference list from the start, and then guarded against duplicates with List.contains, so both grew quadratically with the number of open editors and views. They now build an identity map once per call and use a set for the duplicate check. The resulting order is unchanged. ToolItemUpdater.updateContributionItems rescheduled the same runnable through Display.timerExec once per matching item, which cancels and recreates the timer on every iteration although only the last call has any effect. Schedule it once per batch instead. Both are cleanups without a measured runtime effect. The nested scans only matter with many open parts, and the redundant timer scheduling only with many contributed tool items, neither of which the current tests cover.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors part-reference resolution in the workbench to avoid repeated linear scans when mapping E4 model parts (MPart) to their corresponding EditorReference/ViewReference, and reduces redundant Display.timerExec(...) rescheduling in tool item enablement updates. The changes fit within Eclipse Platform UI’s workbench internals, primarily improving algorithmic complexity and code clarity in hot-ish paths.
Changes:
- Replace repeated
O(n)scans +List.contains(...)duplicate guards with a per-callIdentityHashMap<MPart, …>for lookups and aSetfor duplicate filtering, keeping ordering behavior consistent. - Optimize placeholder visibility filtering by precomputing rendered placeholders into an identity-based set.
- Schedule
ToolItemUpdater’s delayed runnable once per update batch instead of once per matching item.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/WorkbenchPage.java | Uses identity maps/sets to resolve editor/view references without repeated scans and quadratic duplicate checks. |
| bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/ToolItemUpdater.java | Avoids redundant timerExec rescheduling by scheduling once per batch when updates are queued. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Two cleanups found while profiling the editor open and switch paths, offered as code quality rather than as a speedup.
getOrderedEditorReferences, getSortedEditorReferences and getSortedParts resolved every model element by scanning the reference list from the start and then guarded against duplicates with List.contains, so both parts grew quadratically with the number of open editors and views. They now build an identity map once per call and use a set for the duplicate check, with the resulting order unchanged. ToolItemUpdater.updateContributionItems rescheduled the same runnable through Display.timerExec once per matching item, cancelling and recreating the timer on every iteration although only the last call has any effect, so it is now scheduled once per batch.
Neither change is measurable in the existing tests. I compared both variants with interleaved runs of the editor open, close and switch tests and the difference stayed within the noise, which is expected: the nested scans only matter with many open parts and the redundant timer scheduling only with many contributed tool items, and those tests have neither. Please take them on readability and complexity grounds, not on performance.
Verified with org.eclipse.ui.tests, org.eclipse.e4.ui.tests, org.eclipse.e4.ui.workbench.addons.swt.test and org.eclipse.jface.tests: 1719 tests, no failures.