Skip to content

Commit

Permalink
[GTK] Speedup TreeItem retrieval by removing redundant iterations #882
Browse files Browse the repository at this point in the history
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%)
  • Loading branch information
basilevs authored and SyntevoAlex committed Jan 7, 2024
1 parent df73b36 commit 42dd695
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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<length; i++) {
result [i] = _getItem (parent, i);
}
} else {
int i = 0;
int[] index = new int [1];
long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ());
ArrayList<TreeItem> 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 ()]);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 -> {
Expand Down

0 comments on commit 42dd695

Please sign in to comment.