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 bdb5c2b9ea4..87c988337f2 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 @@ -111,6 +111,8 @@ public class Tree extends Composite { private long headerCSSProvider; + private TreeItemCache itemCache = null; + static final int ID_COLUMN = 0; static final int CHECKED_COLUMN = 1; static final int GRAYED_COLUMN = 2; @@ -4346,4 +4348,11 @@ public void dispose() { headerCSSProvider = 0; } } + +TreeItemCache getItemCache(TreeItem treeItem) { + if (itemCache == null || itemCache.owner != treeItem) { + itemCache = new TreeItemCache(treeItem); + } + return itemCache; +} } 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 6c127c3f23f..8ca291a76f3 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 @@ -166,6 +166,9 @@ public TreeItem (TreeItem parentItem, int style, int index) { this.parent = parent; if (create) { parent.createItem (this, parentIter, index); + if (parentIter != 0) { + parent._getItem(parentIter).resetCache(); + } } else { handle = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); GTK.gtk_tree_model_iter_nth_child (parent.modelHandle, handle, parentIter, index); @@ -768,7 +771,7 @@ Rectangle getImageBoundsInPixels (int index) { public int getItemCount () { checkWidget(); if (!parent.checkData (this)) error (SWT.ERROR_WIDGET_DISPOSED); - return GTK.gtk_tree_model_iter_n_children (parent.modelHandle, handle); + return getCache().getItemCount(); } /** @@ -791,10 +794,8 @@ public int getItemCount () { 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); + if (index >= getCache().getItemCount()) error (SWT.ERROR_INVALID_RANGE); + return getCache().getItem(index); } /** @@ -1099,6 +1100,7 @@ public void dispose () { */ public void removeAll () { checkWidget (); + resetCache(); long modelHandle = parent.modelHandle; int length = GTK.gtk_tree_model_iter_n_children (modelHandle, handle); if (length == 0) return; @@ -1629,6 +1631,7 @@ public void setImage (Image [] images) { public void setItemCount (int count) { checkWidget (); count = Math.max (0, count); + resetCache(); parent.setItemCount (handle, count); } @@ -1708,4 +1711,14 @@ public void setText (String [] strings) { if (string != null) setText (i, string); } } + +private void resetCache() { + getCache().reset(); +} + +private TreeItemCache getCache() { + return parent.getItemCache(this); +} + + } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItemCache.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItemCache.java new file mode 100644 index 00000000000..68f369c439e --- /dev/null +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeItemCache.java @@ -0,0 +1,69 @@ +/******************************************************************************* + * Copyright (c) 2023, 2023 IBM Corporation and others. + * + * This program and the accompanying materials + * are made available under the terms of the Eclipse Public License 2.0 + * which accompanies this distribution, and is available at + * https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Vasili Gulevich - initial API and implementation + *******************************************************************************/ +package org.eclipse.swt.widgets; + +import java.util.*; + +import org.eclipse.swt.internal.gtk.*; + +/** Volatile information about a TreeItem, that takes long to compute + * @since 3.125**/ +class TreeItemCache { + final TreeItem owner; + private TreeItem[] allChildren; + private final ArrayList sparseChildren = new ArrayList<>(); + private int itemCount = -1; + + TreeItemCache(TreeItem owner) { + this.owner = Objects.requireNonNull(owner); + } + + int getItemCount() { + if (itemCount < 0) { + itemCount = GTK.gtk_tree_model_iter_n_children (owner.parent.modelHandle, owner.handle); + } + return itemCount; + } + + TreeItem getItem(int index) { + if (sparseChildren.size() > index && sparseChildren.get(index) != null) { + return sparseChildren.get(index); + } else { + int newSize = Math.max(index + 1, itemCount); + int extraSize = newSize - sparseChildren.size(); + if (extraSize > 0) { + sparseChildren.addAll(Collections.nCopies(extraSize, null)); + } + TreeItem result = owner.parent._getItem(owner.handle, index); + sparseChildren.set(index, result); + return result; + } + } + + TreeItem[] getItems() { + if (allChildren == null) { + allChildren = owner.parent.getItems(owner.handle); + sparseChildren.clear(); + sparseChildren.addAll(Arrays.asList(allChildren)); + itemCount = allChildren.length; + } + return allChildren; + } + + public void reset() { + itemCount = -1; + allChildren = null; + sparseChildren.clear(); + } +} diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_Tree.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_Tree.java index 0ad17d53a07..d87091da7de 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_Tree.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_Tree.java @@ -21,8 +21,12 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import java.util.ArrayDeque; import java.util.ArrayList; +import java.util.Deque; import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.function.IntFunction; import org.eclipse.swt.SWT; import org.eclipse.swt.events.TreeListener; @@ -35,6 +39,7 @@ import org.eclipse.swt.widgets.TreeItem; import org.eclipse.test.Screenshots; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TestName; @@ -1190,4 +1195,177 @@ public void test_setItemCount_itemCount2() { }); } +private double measureGetItemNanos(int childCount) { + tree.setItemCount(1); + TreeItem parent = tree.getItem(0); + parent.setItemCount(childCount); + return measureNanos(() -> parent.getItem(childCount - 1)); +} + +/** + * Execution time of TreeItem.getItem(int index) should not depend on index + * @see https://github.com/eclipse-platform/eclipse.platform.swt/issues/882 +*/ +@Test +public void test_getItemNoGrowth() { + testTreeRegularAndVirtual(() -> { + assertConstant("getItem() execution time", this::measureGetItemNanos); + }); +} + +private double measureGetItemCountNanos(int childCount) { + tree.setItemCount(1); + TreeItem parent = tree.getItem(0); + parent.setItemCount(childCount); + return measureNanos(parent::getItemCount); +} + +/** + * Execution time of TreeItem.getItemCount() should not depend on child count. + * @see https://github.com/eclipse-platform/eclipse.platform.swt/issues/882 +*/ +@Test +public void test_getItemCountNoGrowth() { + testTreeRegularAndVirtual(() -> { + assertConstant("itemCount execution time", this::measureGetItemCountNanos); + }); +} + + +void buildBinaryTree(TreeItem parent, int totalChildCount) { + if (totalChildCount <= 0) { + return; + } + int leftCount = totalChildCount / 2; + int rightCount = totalChildCount - leftCount; + if (leftCount > 1) { + buildBinaryTree(new TreeItem(parent, SWT.NONE), leftCount - 1); + } + if (rightCount > 1) { + buildBinaryTree(new TreeItem(parent, SWT.NONE), rightCount - 1); + } +} + +void depthFirstTraverse(TreeItem parent) { + int count = parent.getItemCount(); + for (int i = 0; i < count ; i++) { + depthFirstTraverse(parent.getItem(i)); + } +} + +private void breadthFirstTraverse(TreeItem parent) { + Deque queue = new ArrayDeque<>(); + queue.add(parent); + while (!queue.isEmpty()) { + parent = queue.removeFirst(); + int count = parent.getItemCount(); + for (int i = 0; i < count ; i++) { + queue.add(parent.getItem(i)); + } + } +} + + +/** + * Measures time required to do one operation + * @return nanoseconds + */ +private double measureNanos(Runnable operation) { + // warmup and calibration - we measure, how many iterations we can approximately do in a second + long warmupStop = System.nanoTime() + TimeUnit.SECONDS.toNanos(1); + long iterationCount = 0; + while (System.nanoTime() < warmupStop) { + System.nanoTime(); + operation.run(); + iterationCount++; + } + if (iterationCount < 100) { + iterationCount = 100; + } + + long start = System.nanoTime(); + for (int i = 0; i < iterationCount; i++) { + operation.run(); + } + long stop = System.nanoTime(); + long elapsed = stop-start; + return ((double)elapsed) / iterationCount; +} + +private double measureBinaryDepthFirstTraverse(int totalChildCount) { + TreeItem root = new TreeItem(tree, SWT.NONE); + buildBinaryTree(root, totalChildCount - 1); + return measureNanos(() -> depthFirstTraverse(root)); +} + +private void assertConstant(String message, IntFunction function) { + function.apply(1000); // warmmup + double elapsed_100 = function.apply(100); + double elapsed_100000 = function.apply(100000); + double ratio = elapsed_100000 / elapsed_100; + String error = String.format( "%s should be constant. But:\nTime for 100 elements: %f ns\nTime for 100000 elements: %f ns\nRatio: %f\n", message, elapsed_100, elapsed_100000, ratio); + assertTrue(error, (elapsed_100000 <= 10 && elapsed_100 <= 10) || ratio < 2); +} + +private void assertLinear(String message, IntFunction function) { + function.apply(1000); // warmmup + double elapsed_100 = function.apply(100); + double elapsed_100000 = function.apply(100000); + double ratio = elapsed_100000 / elapsed_100; + String error = String.format( "%s should be linear. But:\nTime for 100 elements: %f ns\nTime for 100000 elements: %f ns\nRatio: %f\n", message, elapsed_100, elapsed_100000, ratio); + assertTrue(error, (elapsed_100000 <= 100 && elapsed_100 <= 100) || ratio < 2000); +} + +@Test +public void test_binaryDepthFirstTraversalLinearGrowth() { + testTreeRegularAndVirtual(() -> { + assertLinear("Depth first traversal", this::measureBinaryDepthFirstTraverse); + }); +} + +private void buildWideTree(TreeItem root, int totalChildCount) { + int parentCount = (int) Math.sqrt(totalChildCount); + int childCount = parentCount - 1; + + root.setItemCount(parentCount); + totalChildCount -= parentCount; + for (TreeItem parent: root.getItems()) { + parent.setItemCount(childCount); + totalChildCount -= childCount; + } + if (totalChildCount > 0) { + TreeItem parent = new TreeItem(root, SWT.NONE); + if (totalChildCount > 1) { + parent.setItemCount(totalChildCount - 1); + } + } +} + +private double measureWideDepthFirstTraverse(int totalChildCount) { + TreeItem root = new TreeItem(tree, SWT.NONE); + buildWideTree(root, totalChildCount - 1); + return measureNanos(() -> depthFirstTraverse(root)); +} + +@Test +@Ignore("https://github.com/eclipse-platform/eclipse.platform.swt/issues/882 Wide tree depth first traversal is still bad") +public void test_wideDepthFirstTraversalLinearGrowth() { + testTreeRegularAndVirtual(() -> { + assertLinear("Depth first traversal", this::measureWideDepthFirstTraverse); + }); +} + +private double measureWideBreadthFirstTraverse(int totalChildCount) { + TreeItem root = new TreeItem(tree, SWT.NONE); + buildWideTree(root, totalChildCount - 1); + return measureNanos(() -> breadthFirstTraverse(root)); +} + +@Test +public void test_wideBreadthFirstTraversalLinearGrowth() { + testTreeRegularAndVirtual(() -> { + assertLinear("Depth first traversal", this::measureWideBreadthFirstTraverse); + }); +} + }