From cc810eacdc96777df3f3da7ee45ea5adabf96bdc Mon Sep 17 00:00:00 2001 From: Vasili Gulevich Date: Sun, 26 Nov 2023 18:16:12 +0000 Subject: [PATCH] [GTK] Speedup TreeItem retrieval by removing redundant checks #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%) --- .../gtk/org/eclipse/swt/widgets/Tree.java | 46 ++++++++----------- .../gtk/org/eclipse/swt/widgets/TreeItem.java | 11 +++-- ...Test_org_eclipse_swt_widgets_TreeItem.java | 9 ++++ .../Test_org_eclipse_swt_widgets_Tree.java | 12 +++++ 4 files changed, 48 insertions(+), 30 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java index ba1c4db4ec0..608cda18fb0 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java @@ -14,6 +14,8 @@ package org.eclipse.swt.widgets; +import java.util.*; + import org.eclipse.swt.*; import org.eclipse.swt.events.*; import org.eclipse.swt.graphics.*; @@ -197,16 +199,10 @@ TreeItem _getItem (long iter) { return items [id]; } -TreeItem _getItem (long parentIter, int index) { - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - try { - GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, parentIter, index); - int id = getId (iter, true); - if (items [id] != null) return items [id]; - return items [id] = new TreeItem (this, parentIter, SWT.NONE, index, iter); - } finally { - OS.g_free (iter); - } +TreeItem _getItem (long parentIter, long iter, int index) { + int id = getId (iter, true); + if (items [id] != null) return items [id]; + return items [id] = new TreeItem (this, parentIter, SWT.NONE, index, iter); } void reallocateIds(int newSize) { @@ -1748,10 +1744,14 @@ public boolean getHeaderVisible () { */ public TreeItem getItem (int index) { checkWidget(); - if (!(0 <= index && index < GTK.gtk_tree_model_iter_n_children (modelHandle, 0))) { - error (SWT.ERROR_INVALID_RANGE); + if (index < 0) error (SWT.ERROR_INVALID_RANGE); + long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + try { + if (!GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, 0, index)) error (SWT.ERROR_INVALID_RANGE); + return _getItem (0, iter, index); + } finally { + OS.g_free (iter); } - return _getItem (0, index); } /** @@ -1927,26 +1927,18 @@ int getItemHeightInPixels () { } TreeItem [] getItems (long parent) { - int length = GTK.gtk_tree_model_iter_n_children (modelHandle, parent); - TreeItem[] result = new TreeItem [length]; - if (length == 0) return result; - if ((style & SWT.VIRTUAL) != 0) { - for (int i=0; i result = new ArrayList<>(); + long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + try { boolean valid = GTK.gtk_tree_model_iter_children (modelHandle, iter, parent); while (valid) { - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, index, -1); - result [i++] = items [index [0]]; + result.add (_getItem (parent, iter, result.size ())); valid = GTK.gtk_tree_model_iter_next (modelHandle, iter); } + } finally { OS.g_free (iter); } - return result; + return result.toArray (new TreeItem [result.size()]); } /** diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java index ce9e334b476..15ca6fd4ec3 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItem.java @@ -794,9 +794,14 @@ public TreeItem getItem (int index) { checkWidget(); if (index < 0) error (SWT.ERROR_INVALID_RANGE); if (!parent.checkData (this)) error (SWT.ERROR_WIDGET_DISPOSED); - int itemCount = GTK.gtk_tree_model_iter_n_children (parent.modelHandle, handle); - if (index >= itemCount) error (SWT.ERROR_INVALID_RANGE); - return parent._getItem (handle, index); + + long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + try { + if (!GTK.gtk_tree_model_iter_nth_child (parent.modelHandle, iter, handle, index)) error (SWT.ERROR_INVALID_RANGE); + return parent._getItem (handle, iter, index); + } finally { + OS.g_free (iter); + } } /** diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java index 8174a4ec3e7..58312e12e65 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_TreeItem.java @@ -1174,6 +1174,15 @@ public void test_setTextILjava_lang_String(){ } +@Test +public void test_removeAll() { + TreeItem item = new TreeItem(treeItem, SWT.NONE); + assertEquals(1, treeItem.getItemCount()); + treeItem.removeAll(); + assertEquals(0, treeItem.getItemCount()); + assertTrue(item.isDisposed()); +} + /* custom */ TreeItem treeItem; Tree tree; diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/performance/Test_org_eclipse_swt_widgets_Tree.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/performance/Test_org_eclipse_swt_widgets_Tree.java index e6458f8ca85..4b759bce208 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/performance/Test_org_eclipse_swt_widgets_Tree.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/performance/Test_org_eclipse_swt_widgets_Tree.java @@ -210,6 +210,18 @@ public void secondTraverse() { }); } + @Test + public void dispose() { + assertMaximumDegree(1.2, n -> { + Tree tree = buildSubject(n, this::initializeItem); + breadthFirstTraverse(tree, item -> { + item.setExpanded(true); + }); + return measureNanos(() -> tree.dispose()); + }); + } + + @Test public void getForeground() { assertMaximumDegree(1.2, n -> {