[GTK] Optimize TreeItem.getItem(int) by caching TreeItem.getItems() #882 #908
Conversation
This file contains 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
Testing asymptotic execution times for operations over large trees.
iterator eclipse-platform#882 This change eliminates a single hotspot. The overall complexity of tree building and element reveal is still quadratic. Performance improvement is proven by running org.eclipse.swt.tests.junit.performance.Test_org_eclipse_swt_widgets_Tree.jfaceReveal() In test jfaceReveal[Shape: STAR, virtual: true]: 10_000 elements: 620_682_717ns -> 501_378_662ns 100_000 elements: 62_804_214_489ns -> 52_781_899_817ns (-16%)
…-platform#882 As we already do iteration of O(N) complexity, and it detects out-of-bounds problems, eliminate redundant model size check computed in O(N). Performance improvement is proven by running org.eclipse.swt.tests.junit.performance.Test_org_eclipse_swt_widgets_Tree.jfaceReveal() In test jfaceReveal[Shape: STAR, virtual: true]: 10_000 elements: 501_378_662ns -> 218_583_468ns 100_000 elements: 52_781_899_817ns -> 20_872_245_796ns (-60%)
…clipse-platform#882 Performance improvement is proven by running relevant tests from org.eclipse.swt.tests.junit.performance.Test_org_eclipse_swt_widgets_Tree: In test jfaceReveal[Shape: STAR, virtual: true]: 10_000 elements: 218_583_468ns -> 141_061_117ns 100_000 elements: 20_872_245_796ns -> 10_761_449_641ns (-48%) In test dispose[Shape: STAR, virtual: true]: 10_000 elements: 5_637_088ns -> 6_222_363ns 100_000 elements: 62_422_153ns -> 56_442_156ns
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.
DO NOT MERGE - this is a demonstration, not an actual solution. See below.
This is a follow up to #903 and #904.
This PR eliminates explicit iterations over
Treefrom execution profile whenTreeItem.getItems()is called beforeTreeItem.getItem(int). This is the case for JFace lazy TreeViewer.Performance improvement is proven by running relevant tests from
org.eclipse.swt.tests.junit.performance.Test_org_eclipse_swt_widgets_Tree:
In test jfaceReveal[Shape: STAR, virtual: true]:
10_000 elements: 218_583_468ns -> 141_061_117ns
100_000 elements: 20_872_245_796ns -> 10_761_449_641ns (-48%)
In test dispose[Shape: STAR, virtual: true]:
10_000 elements: 5_637_088ns -> 6_222_363ns
100_000 elements: 62_422_153ns -> 56_442_156ns
Profile before the change:
Profile after the change:
The solution here is ideologically incorrect, as it targets a particular way of tree traversal, specific to JFAce lazy tree. However, a more general and self-consistent approach, that moves ownership of a TreeItem from a Tree to a parent item, prototyped in master...basilevs:eclipse.platform.swt:issue_882_gtk_direct_access is a much larger change. I suggest not to merge this PR and just use it as a starting point for discussion on removal of ID system from SWT GTK Tree.
The removal itself is done in #918