From 42dd69529632c57467d7a8028485c74bcc97aceb 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 iterations #882 TreeItem and Tree have been checking indexes for bounds during indexed access and traversal. Such checks are slow as node size computation is an O(N) operation. As Tree already does iteration when retrieving a TreeItem by its index, and such iteration detects out-of-bounds problems, preliminary model bounds check is redundant. Also, bounds check is avoidable during iteration over a node - instead of an index, GTK iterator applies directly. Performance improvement proved 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..4dfa8df4ba1 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 -> {