From bec9c4d6bf0e511bd3e5bb211b81654a1816d68c Mon Sep 17 00:00:00 2001 From: Vasili Gulevich Date: Sun, 26 Nov 2023 15:48:53 +0000 Subject: [PATCH] [GTK] Optimize TreeItem constructor by removing ID system #882 The ID system tracking association of every TreeItem with GTK model entry had been using GTK.gtk_tree_store_set() to persist a key in GTK model. This operation has linear execution time over model size. IDs were replaced with tree paths and strong references, which, if implemented right, have logarithmic execution time on balanced trees and constant execution time on wide trees. 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: 141_061_117ns -> 4_369_889ns 100_000 elements: 10_761_449_641ns -> 181_898_611ns (-98%) --- .../gtk/org/eclipse/swt/widgets/Tree.java | 559 +++++++----------- .../org/eclipse/swt/widgets/TreeColumn.java | 10 +- .../gtk/org/eclipse/swt/widgets/TreeItem.java | 201 ++++--- .../Test_org_eclipse_swt_widgets_Tree.java | 2 +- .../Test_org_eclipse_swt_widgets_Tree.java | 1 - 5 files changed, 334 insertions(+), 439 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 608cda18fb0..a2639edb965 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 @@ -15,6 +15,8 @@ import java.util.*; +import java.util.List; +import java.util.stream.*; import org.eclipse.swt.*; import org.eclipse.swt.events.*; @@ -87,7 +89,7 @@ public class Tree extends Composite { int columnCount, sortDirection; int selectionCountOnPress,selectionCountOnRelease; long ignoreCell; - TreeItem[] items; + final List roots = new ArrayList<>(); int nextId; TreeColumn [] columns; TreeColumn sortColumn; @@ -180,78 +182,29 @@ void _addListener (int eventType, Listener listener) { } } -TreeItem _getItem (long iter) { - int id = getId (iter, true); - if (items [id] != null) return items [id]; +TreeItem _getItemByIter (long iter) { + if (iter == 0) return null; long path = GTK.gtk_tree_model_get_path (modelHandle, iter); - int depth = GTK.gtk_tree_path_get_depth (path); - int [] indices = new int [depth]; - C.memmove (indices, GTK.gtk_tree_path_get_indices (path), 4*depth); - long parentIter = 0; - if (depth > 1) { - GTK.gtk_tree_path_up (path); - parentIter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - GTK.gtk_tree_model_get_iter (modelHandle, parentIter, path); - } - items [id] = new TreeItem (this, parentIter, SWT.NONE, indices [indices.length -1], iter); - GTK.gtk_tree_path_free (path); - if (parentIter != 0) OS.g_free (parentIter); - return items [id]; -} - -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) { - TreeItem [] newItems = new TreeItem [newSize]; - System.arraycopy (items, 0, newItems, 0, items.length); - items = newItems; -} - -int findAvailableId() { - // Adapt to cases where items[] array was resized since last search - // This also fixes cases where +1 below went too far - if (nextId >= items.length) - nextId = 0; - - // Search from 'nextId' to end - for (int id = nextId; id < items.length; id++) { - if (items [id] == null) return id; - } - - // Search from begin to nextId - for (int id = 0; id < nextId; id++) { - if (items [id] == null) return id; - } - - // Still not found; no empty spots remaining - int newId = items.length; - if (drawCount <= 0) { - reallocateIds (items.length + 4); - } else { - // '.setRedraw(false)' is typically used during bulk operations. - // Reallocate to 1.5x the old size to avoid frequent reallocations. - reallocateIds ((items.length + 1) * 3 / 2); + if (path == 0) error (SWT.ERROR_NO_HANDLES); + try { + return _getItemByPath(path); + } finally { + GTK.gtk_tree_path_free (path); } - - return newId; } -int getId (long iter, boolean queryModel) { - if (queryModel) { - int[] value = new int[1]; - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, value, -1); - if (value [0] != -1) return value [0]; +TreeItem _getItemByPath (long path) { + if (path == 0) return null; + int depth; + int [] indices; + depth = GTK.gtk_tree_path_get_depth (path); + indices = new int [depth]; + C.memmove (indices, GTK.gtk_tree_path_get_indices (path), 4*depth); + TreeItem item = depth > 0 ? getItem (indices [0]) : null; + for (int i = 1; i < depth; i++) { + item = item.getItem( indices [i]); } - - int id = findAvailableId(); - nextId = id + 1; - - GTK.gtk_tree_store_set (modelHandle, iter, ID_COLUMN, id, -1); - return id; + return item; } static int checkStyle (int style) { @@ -275,7 +228,7 @@ static int checkStyle (int style) { @Override long cellDataProc (long tree_column, long cell, long tree_model, long iter, long data) { if (cell == ignoreCell) return 0; - TreeItem item = _getItem (iter); + TreeItem item = _getItemByIter (iter); if (item != null) OS.g_object_set_qdata (cell, Display.SWT_OBJECT_INDEX2, item.handle); boolean isPixbuf = GTK.GTK_IS_CELL_RENDERER_PIXBUF (cell); boolean isText = GTK.GTK_IS_CELL_RENDERER_TEXT (cell); @@ -568,21 +521,12 @@ int calculateWidth (long column, long iter, boolean recurse) { */ public void clear(int index, boolean all) { checkWidget (); - clear (0, index, all); + TreeItem item = roots.get(index); + if (item == null) return; + item.clear(); + if (all) item.clearAll(all); } -void clear (long parentIter, int index, boolean all) { - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - GTK.gtk_tree_model_iter_nth_child(modelHandle, iter, parentIter, index); - int[] value = new int[1]; - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, value, -1); - if (value [0] != -1) { - TreeItem item = items [value [0]]; - item.clear (); - } - if (all) clearAll (all, iter); - OS.g_free (iter); -} /** * Clears all the items in the receiver. The text, icon and other @@ -605,24 +549,12 @@ void clear (long parentIter, int index, boolean all) { */ public void clearAll (boolean all) { checkWidget (); - clearAll (all, 0); -} -void clearAll (boolean all, long parentIter) { - int length = GTK.gtk_tree_model_iter_n_children (modelHandle, parentIter); - if (length == 0) return; - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - boolean valid = GTK.gtk_tree_model_iter_children (modelHandle, iter, parentIter); - int[] value = new int[1]; - while (valid) { - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, value, -1); - if (value [0] != -1) { - TreeItem item = items [value [0]]; - item.clear (); + for (TreeItem item: roots) { + if (item != null) { + item.clear(); + if (all) item.clearAll(all); } - if (all) clearAll (all, iter); - valid = GTK.gtk_tree_model_iter_next (modelHandle, iter); } - OS.g_free (iter); } @Override @@ -658,11 +590,11 @@ Point computeSizeInPixels (int wHint, int hHint, boolean changed) { // Initialize to height of root items & header size.y = getItemCount() * itemHeight + getHeaderHeight(); - for (TreeItem item : items) { + getKnownItemsRecursively().forEach(item -> { if (item != null && item.isExpanded) { size.y += GTK.gtk_tree_model_iter_n_children (modelHandle, item.handle) * itemHeight; } - } + }); } Rectangle trim = computeTrimInPixels (0, 0, size.x, size.y); @@ -684,48 +616,41 @@ Point computeSizeInPixels (int wHint, int hHint, boolean changed) { return size; } -void copyModel (long oldModel, int oldStart, long newModel, int newStart, long oldParent, long newParent, int modelLength) { +void copyModel (long oldModel, int oldStart, long newModel, int newStart, TreeItem parentItem, long newParent, int modelLength) { long iter = OS.g_malloc(GTK.GtkTreeIter_sizeof ()); long value = OS.g_malloc (OS.GValue_sizeof ()); // GValue needs to be initialized with G_VALUE_INIT, which is zeroes C.memset (value, 0, OS.GValue_sizeof ()); + long oldParent = parentItem != null ? parentItem.handle : 0; if (GTK.gtk_tree_model_iter_children (oldModel, iter, oldParent)) { long [] oldItems = new long [GTK.gtk_tree_model_iter_n_children (oldModel, oldParent)]; int oldIndex = 0; - int [] intBuffer = new int [1]; do { long newIterator = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); if (newIterator == 0) error (SWT.ERROR_NO_HANDLES); GTK.gtk_tree_store_append (newModel, newIterator, newParent); - GTK.gtk_tree_model_get (oldModel, iter, ID_COLUMN, intBuffer, -1); - int index = intBuffer[0]; - TreeItem item = null; - if (index != -1) { - item = items [index]; - if (item != null) { - long oldIterator = item.handle; - oldItems[oldIndex++] = oldIterator; - - // Copy header fields - for (int iColumn = 0; iColumn < FIRST_COLUMN; iColumn++) { - GTK.gtk_tree_model_get_value (oldModel, oldIterator, iColumn, value); - GTK.gtk_tree_store_set_value (newModel, newIterator, iColumn, value); - OS.g_value_unset (value); - } + TreeItem item = parentItem != null ? parentItem._peekItem(oldIndex) : roots.get(oldIndex); + if (item != null) { + long oldIterator = item.handle; + oldItems[oldIndex++] = oldIterator; + + // Copy header fields + for (int iColumn = 0; iColumn < FIRST_COLUMN; iColumn++) { + GTK.gtk_tree_model_get_value (oldModel, oldIterator, iColumn, value); + GTK.gtk_tree_store_set_value (newModel, newIterator, iColumn, value); + OS.g_value_unset (value); + } - // Copy requested columns - for (int iOffset = 0; iOffset < modelLength - FIRST_COLUMN; iOffset++) { - GTK.gtk_tree_model_get_value (oldModel, oldIterator, oldStart + iOffset, value); - GTK.gtk_tree_store_set_value (newModel, newIterator, newStart + iOffset, value); - OS.g_value_unset (value); - } + // Copy requested columns + for (int iOffset = 0; iOffset < modelLength - FIRST_COLUMN; iOffset++) { + GTK.gtk_tree_model_get_value (oldModel, oldIterator, oldStart + iOffset, value); + GTK.gtk_tree_store_set_value (newModel, newIterator, newStart + iOffset, value); + OS.g_value_unset (value); } - } else { - GTK.gtk_tree_store_set (newModel, newIterator, ID_COLUMN, -1, -1); } // recurse through children - copyModel(oldModel, oldStart, newModel, newStart, iter, newIterator, modelLength); + copyModel(oldModel, oldStart, newModel, newStart, item, newIterator, modelLength); if (item!= null) { item.handle = newIterator; @@ -746,6 +671,7 @@ void copyModel (long oldModel, int oldStart, long newModel, int newStart, long o OS.g_free (iter); } + void createColumn (TreeColumn column, int index) { /* * Bug in ATK. For some reason, ATK segments fault if @@ -773,7 +699,7 @@ void createColumn (TreeColumn column, int index) { long [] types = getColumnTypes (columnCount + 4); // grow by 4 rows at a time long newModel = GTK.gtk_tree_store_newv (types.length, types); if (newModel == 0) error (SWT.ERROR_NO_HANDLES); - copyModel (oldModel, FIRST_COLUMN, newModel, FIRST_COLUMN, (long )0, (long )0, modelLength); + copyModel (oldModel, FIRST_COLUMN, newModel, FIRST_COLUMN, null, (long )0, modelLength); GTK.gtk_tree_view_set_model (handle, newModel); setModel (newModel); } @@ -961,8 +887,7 @@ void createItem (TreeColumn column, int index) { OS.pango_font_description_free (fontDesc); } if (columnCount >= 1) { - for (int i=0; i { if (item != null) { Font [] cellFont = item.cellFont; if (cellFont != null) { @@ -981,7 +906,7 @@ void createItem (TreeColumn column, int index) { } } - } + }); } updateHeaderCSS(); @@ -991,43 +916,10 @@ void createItem (TreeColumn column, int index) { * The fastest way to insert many items is documented in {@link TreeItem#TreeItem(org.eclipse.swt.widgets.Tree,int,int)} * and {@link TreeItem#setItemCount} */ -void createItem (TreeItem item, long parentIter, int index) { - /* - * Try to achieve maximum possible performance in bulk insert scenarios. - * Even a single call to 'gtk_tree_model_iter_n_children' already - * reduces performance 3x, so try to avoid any unneeded API calls. - */ - if (index == 0) { - item.handle = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (item.handle == 0) error(SWT.ERROR_NO_HANDLES); - GTK.gtk_tree_store_prepend (modelHandle, item.handle, parentIter); - } else if (index == -1) { - item.handle = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (item.handle == 0) error(SWT.ERROR_NO_HANDLES); - GTK.gtk_tree_store_append (modelHandle, item.handle, parentIter); - } else { - int count = GTK.gtk_tree_model_iter_n_children (modelHandle, parentIter); - if (!(0 <= index && index <= count)) error (SWT.ERROR_INVALID_RANGE); - - item.handle = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (item.handle == 0) error(SWT.ERROR_NO_HANDLES); - - /* - * Feature in GTK. It is much faster to append to a tree store - * than to insert at the end using gtk_tree_store_insert(). - */ - if (index == count) { - GTK.gtk_tree_store_append (modelHandle, item.handle, parentIter); - } else { - GTK.gtk_tree_store_insert (modelHandle, item.handle, parentIter, index); - } - } - - int id = getId (item.handle, false); - items [id] = item; +void itemCreated (TreeItem item) { modelChanged = true; - if (parentIter == 0 ) { + if (item.getParentItem() == null) { /* If this was the first root item fire an EmptinessChanged event. */ @@ -1151,7 +1043,6 @@ void createRenderers (long columnHandle, int modelIndex, boolean check, int colu @Override void createWidget (int index) { super.createWidget (index); - items = new TreeItem [4]; columns = new TreeColumn [4]; columnCount = 0; // In GTK 3 font description is inherited from parent widget which is not how SWT has always worked, @@ -1238,35 +1129,33 @@ void destroyItem (TreeColumn column) { long [] types = getColumnTypes (1); long newModel = GTK.gtk_tree_store_newv (types.length, types); if (newModel == 0) error (SWT.ERROR_NO_HANDLES); - copyModel(oldModel, column.modelIndex, newModel, FIRST_COLUMN, (long )0, (long )0, FIRST_COLUMN + CELL_TYPES); + copyModel(oldModel, column.modelIndex, newModel, FIRST_COLUMN, null, (long )0, FIRST_COLUMN + CELL_TYPES); GTK.gtk_tree_view_set_model (handle, newModel); setModel (newModel); createColumn (null, 0); } else { - for (int i=0; i { + long iter = item.handle; + int modelIndex = column.modelIndex; + GTK.gtk_tree_store_set (modelHandle, iter, modelIndex + CELL_PIXBUF, (long )0, -1); + GTK.gtk_tree_store_set (modelHandle, iter, modelIndex + CELL_TEXT, (long )0, -1); + GTK.gtk_tree_store_set (modelHandle, iter, modelIndex + CELL_FOREGROUND, (long )0, -1); + GTK.gtk_tree_store_set (modelHandle, iter, modelIndex + CELL_BACKGROUND, (long )0, -1); + GTK.gtk_tree_store_set (modelHandle, iter, modelIndex + CELL_FONT, (long )0, -1); + + Font [] cellFont = item.cellFont; + if (cellFont != null) { + if (columnCount == 0) { + item.cellFont = null; + } else { + Font [] temp = new Font [columnCount]; + System.arraycopy (cellFont, 0, temp, 0, indexCopy); + System.arraycopy (cellFont, indexCopy + 1, temp, indexCopy, columnCount - indexCopy); + item.cellFont = temp; } } - } + }); if (index == 0) { // first column must be left aligned and must show check box TreeColumn firstColumn = columns [0]; @@ -1285,18 +1174,11 @@ void destroyItem (TreeColumn column) { } -void destroyItem (TreeItem item) { - long selection = GTK.gtk_tree_view_get_selection (handle); - OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - GTK.gtk_tree_store_remove (modelHandle, item.handle); - OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - modelChanged = true; - +void itemDestroyed (TreeItem item) { /* If this was the last root item fire an EmptinessChanged event. */ - int roots = GTK.gtk_tree_model_iter_n_children (modelHandle, 0); - if (roots == 0) { + if (roots.isEmpty()) { Event event = new Event (); event.detail = 1; sendEvent (SWT.EmptinessChanged, event); @@ -1584,16 +1466,11 @@ TreeItem getFocusItem () { long [] path = new long [1]; GTK.gtk_tree_view_get_cursor (handle, path, null); if (path [0] == 0) return null; - TreeItem item = null; - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (GTK.gtk_tree_model_get_iter (modelHandle, iter, path [0])) { - int [] index = new int [1]; - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, index, -1); - if (index [0] != -1) item = items [index [0]]; //TODO should we be creating this item when index is -1? + try { + return _getItemByPath (path [0]); + } finally { + GTK.gtk_tree_path_free (path [0]); } - OS.g_free (iter); - GTK.gtk_tree_path_free (path [0]); - return item; } /** @@ -1745,10 +1622,27 @@ public boolean getHeaderVisible () { public TreeItem getItem (int index) { checkWidget(); if (index < 0) error (SWT.ERROR_INVALID_RANGE); + if (index >= roots.size ()) error (SWT.ERROR_INVALID_RANGE); + + return _getItem(null, 0, roots, index); +} + +TreeItem _getItem(TreeItem parentItem, long parentIter, List siblings, int index) { + TreeItem result = siblings.get(index); + if (result != null) return result; + + // TODO: shorter iteration when index - 1 exists + 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); + if (!GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, parentIter, index)) error (SWT.ERROR_INVALID_RANGE); + result = new TreeItem(this, parentItem, SWT.NONE, index, iter); + TreeItem existing = siblings.set(index, result); + if (existing != null) { + existing.dispose(); + throw new AssertionError("Out-of-bound item creation"); + } + return result; } finally { OS.g_free (iter); } @@ -1815,7 +1709,7 @@ TreeItem getItemInPixels (Point point) { } } if (!overExpander) { - item = _getItem (iter); + item = _getItemByIter (iter); } } OS.g_free (iter); @@ -1923,22 +1817,28 @@ int getItemHeightInPixels () { */ public TreeItem [] getItems () { checkWidget(); - return getItems (0); + return _getItems(null, roots, 0); } -TreeItem [] getItems (long parent) { - ArrayList result = new ArrayList<>(); +TreeItem [] _getItems (TreeItem parentItem, List items, long parentIter) { + int i = 0; long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + if (iter == 0) error (SWT.ERROR_NO_HANDLES); try { - boolean valid = GTK.gtk_tree_model_iter_children (modelHandle, iter, parent); + boolean valid = GTK.gtk_tree_model_iter_children (modelHandle, iter, parentIter); while (valid) { - result.add (_getItem (parent, iter, result.size ())); + if (items.size() <= i || items.get(i) == null) { + new TreeItem(this, parentItem, SWT.NONE, i, iter); + if (items.get(i) == null) throw new AssertionError("Item constructor should update sibling list"); + } + i++; valid = GTK.gtk_tree_model_iter_next (modelHandle, iter); } } finally { OS.g_free (iter); } - return result.toArray (new TreeItem [result.size()]); + assert items.indexOf(null) < 0 : "All items should be hydrated"; + return items.toArray(new TreeItem[items.size()]); } /** @@ -2029,7 +1929,7 @@ public TreeItem[] getSelection () { long data = OS.g_list_data (list); long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); if (GTK.gtk_tree_model_get_iter (modelHandle, iter, data)) { - treeSelection [length] = _getItem (iter); + treeSelection [length] = _getItemByIter (iter); length++; } list = OS.g_list_next (list); @@ -2165,7 +2065,7 @@ public TreeItem getTopItem () { item = null; long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof()); if (GTK.gtk_tree_model_get_iter (modelHandle, iter, path [0])) { - item = _getItem (iter); + item = _getItemByIter (iter); } OS.g_free (iter); GTK.gtk_tree_path_free (path [0]); @@ -2186,7 +2086,7 @@ TreeItem _getCachedTopItem() { long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); long data = OS.g_list_data (list); if (GTK.gtk_tree_model_get_iter (modelHandle, iter, data)) { - treeSelection = _getItem (iter); + treeSelection = _getItemByIter (iter); } OS.g_free (iter); GTK.gtk_tree_path_free (data); @@ -2202,7 +2102,7 @@ TreeItem _getCachedTopItem() { TreeItem item = null; long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof()); if (GTK.gtk_tree_model_get_iter_first (modelHandle, iter)) { - item = _getItem (iter); + item = _getItemByIter (iter); } OS.g_free (iter); return item; @@ -2546,11 +2446,7 @@ long gtk_row_has_child_toggled (long model, long path, long iter) { * and use this callback, as it is invoked when a row has * gotten the first child row or lost its last child row. */ - int [] index = new int [1]; - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, index, -1); - if (index [0] >= items.length) return 0; - TreeItem item = items [index [0]]; - if (item == null) return 0; + TreeItem item = _getItemByPath(path); int childCount = GTK.gtk_tree_model_iter_n_children (modelHandle, item.handle); if (childCount != 0 && item.isExpanded) { OS.g_signal_handlers_block_matched (handle, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, TEST_EXPAND_ROW); @@ -2578,9 +2474,7 @@ long gtk_start_interactive_search(long widget) { @Override long gtk_test_collapse_row (long tree, long iter, long path) { - int [] index = new int [1]; - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, index, -1); - TreeItem item = items [index [0]]; + TreeItem item = _getItemByIter(iter); Event event = new Event (); event.item = item; boolean oldModelChanged = modelChanged; @@ -2618,9 +2512,7 @@ long gtk_test_collapse_row (long tree, long iter, long path) { @Override long gtk_test_expand_row (long tree, long iter, long path) { - int [] index = new int [1]; - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, index, -1); - TreeItem item = items [index [0]]; + TreeItem item = _getItemByPath(path); Event event = new Event (); event.item = item; boolean oldModelChanged = modelChanged; @@ -2667,7 +2559,7 @@ long gtk_toggled (long renderer, long pathStr) { TreeItem item = null; long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof()); if (GTK.gtk_tree_model_get_iter (modelHandle, iter, path)) { - item = _getItem (iter); + item = _getItemByIter (iter); } OS.g_free (iter); GTK.gtk_tree_path_free (path); @@ -2881,43 +2773,21 @@ void register () { display.addWidget (modelHandle, this); } -void releaseItem (TreeItem item, boolean release) { - int [] index = new int [1]; - GTK.gtk_tree_model_get (modelHandle, item.handle, ID_COLUMN, index, -1); - if (index [0] == -1) return; - if (release) item.release (false); - items [index [0]] = null; -} - -void releaseItems (long parentIter) { - int[] index = new int [1]; - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - boolean valid = GTK.gtk_tree_model_iter_children (modelHandle, iter, parentIter); - while (valid) { - releaseItems (iter); - if (!isDisposed ()) { - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, index, -1); - if (index [0] != -1) { - TreeItem item = items [index [0]]; - if (item != null) releaseItem (item, true); - } +void _releaseItems (long parentIter, List items) { + for (TreeItem item: items) { + if (item != null && !isDisposed ()) { + item.release(false); } - valid = GTK.gtk_tree_model_iter_next (modelHandle, iter); } - OS.g_free (iter); } @Override void releaseChildren (boolean destroy) { - if (items != null) { - for (int i=0; i { + if (item != null && !item.isDisposed ()) { + item.release (false); } - items = null; - } + }); if (columns != null) { for (int i=0; i end) return; - int itemCount = GTK.gtk_tree_model_iter_n_children (modelHandle, parentIter); - if (!(0 <= start && start <= end && end < itemCount)) { - error (SWT.ERROR_INVALID_RANGE); - } - long selection = GTK.gtk_tree_view_get_selection (handle); - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (iter == 0) error (SWT.ERROR_NO_HANDLES); - try { - for (int i = start; i <= end; i++) { - GTK.gtk_tree_model_iter_nth_child (modelHandle, iter, parentIter, start); - int[] value = new int[1]; - GTK.gtk_tree_model_get (modelHandle, iter, ID_COLUMN, value, -1); - TreeItem item = value [0] != -1 ? items [value [0]] : null; - if (item != null && !item.isDisposed ()) { - /* - * Bug 182598 - assertion failed in gtktreestore.c - * Removing an item while its data is being set will invalidate - * it, which will cause a crash in GTK. - */ - if(item.settingData) { - throwCannotRemoveItem(i); - } - item.dispose (); - } else { - OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - GTK.gtk_tree_store_remove (modelHandle, iter); - OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - } - } - } finally { - OS.g_free (iter); - } -} - /** * Removes all of the items from the receiver. * @@ -2991,6 +2825,7 @@ public void removeAll () { checkWidget (); checkSetDataInProcessBeforeRemoval(); + long selection = GTK.gtk_tree_view_get_selection (handle); OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); @@ -2998,11 +2833,13 @@ public void removeAll () { OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - for (int i=0; i(roots)) { + if (item != null && !item.isDisposed() ) { + item.release(false); + } } - items = new TreeItem[4]; + + roots.clear(); if (!searchEnabled ()) { GTK.gtk_tree_view_set_search_column (handle, -1); @@ -3066,7 +2903,7 @@ void sendMeasureEvent (long cell, long width, long height) { if (!ignoreSize && GTK.GTK_IS_CELL_RENDERER_TEXT (cell) && hooks (SWT.MeasureItem)) { long iter = OS.g_object_get_qdata (cell, Display.SWT_OBJECT_INDEX2); TreeItem item = null; - if (iter != 0) item = _getItem (iter); + if (iter != 0) item = _getItemByIter (iter); if (item != null && !item.isDisposed()) { int columnIndex = 0; if (columnCount > 0) { @@ -3152,7 +2989,7 @@ void rendererRender (long cell, long cr, long snapshot, long widget, long backgr TreeItem item = null; boolean wasSelected = false; long iter = OS.g_object_get_qdata (cell, Display.SWT_OBJECT_INDEX2); - if (iter != 0) item = _getItem (iter); + if (iter != 0) item = _getItemByIter (iter); long columnHandle = OS.g_object_get_qdata (cell, Display.SWT_OBJECT_INDEX1); int columnIndex = 0; if (columnCount > 0) { @@ -3442,12 +3279,7 @@ void resetCustomDraw () { @Override void reskinChildren (int flags) { - if (items != null) { - for (int i=0; i item.reskinChildren (flags)); if (columns != null) { for (int i=0; i items, int count) { + setRedraw(false); + try { + assert GTK.gtk_tree_model_iter_n_children (modelHandle, parentIter) == items.size (); + for (int currentSize = items.size (); currentSize > count; currentSize = items.size ()) { + TreeItem item = items.get (currentSize - 1); + if (item != null) { + item.dispose (); + } else { + items.remove (currentSize - 1); + } + if (currentSize - 1 != items.size () ) throw new AssertionError ("Items should cleanup sibling list on disposal"); } - OS.g_free (iters); - } else { - for (int i=itemCount; i count) { + GTK.gtk_tree_model_iter_nth_child (modelHandle, modificationIter, parentIter, count); + items.subList (count, items.size ()).clear (); + long selection = GTK.gtk_tree_view_get_selection (parent.handle); + for (int i = count; i < oldSize; i++) { + OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); + GTK.gtk_tree_store_remove (modelHandle, modificationIter); + OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); + } + } else if (items.size () < count) { + if (items.size () > 0) { + GTK.gtk_tree_model_iter_nth_child (modelHandle, modificationIter, parentIter, oldSize - 1); + } + items.addAll(Collections.nCopies (count - items.size(), null)); + long createdIter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + if (createdIter == 0) error (SWT.ERROR_NO_HANDLES); + try { + for (int i = oldSize; i < count; i++) { + GTK.gtk_tree_store_insert_after (modelHandle, createdIter, parentIter, i == 0 ? 0 : modificationIter); + new TreeItem (this, parentItem, SWT.NONE, i, createdIter); + C.memmove (modificationIter, createdIter, GTK.GtkTreeIter_sizeof ()); + } + } finally { + OS.g_free (createdIter); + } + } + assert GTK.gtk_tree_model_iter_n_children (modelHandle, parentIter) == count; + } finally { + OS.g_free (modificationIter); } + } finally { + setRedraw(true); } - if (!isVirtual) setRedraw (true); modelChanged = true; } @@ -3550,9 +3399,9 @@ void setItemCount (long parentIter, int count) { * @since 3.2 */ public void setItemCount (int count) { + if (count < 0) count = 0; checkWidget (); - count = Math.max (0, count); - setItemCount (0, count); + _setItemCount (null, 0, roots, count); } /** @@ -3876,11 +3725,7 @@ void setModel (long newModel) { @Override void setOrientation (boolean create) { super.setOrientation (create); - if (items != null) { - for (int i=0; i item.setOrientation (create)); if (columns != null) { for (int i=0; i { if (item != null && item.settingData) { - throwCannotRemoveItem(i); + throwCannotRemoveItem(); } - } + }); } -private void throwCannotRemoveItem(int i) { - String message = "Cannot remove item with index " + i + "."; +private void throwCannotRemoveItem() { + String message = "Cannot remove item with index."; throw new SWTException(message); } @@ -4341,4 +4185,11 @@ public void dispose() { headerCSSProvider = 0; } } + +Stream getKnownItemsRecursively() { + if (roots == null) return Stream.empty(); // called from constructor + return roots.stream() + .filter(Objects::nonNull). + flatMap( item -> Stream.concat(Stream.of(item), item.getKnownChildrenRecursively())); +} } diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java index 6208ef498a2..eebdf53e78b 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TreeColumn.java @@ -478,12 +478,10 @@ public void pack () { width = requisition.width; } if ((parent.style & SWT.VIRTUAL) != 0) { - for (int i=0; i item.cached ? parent.calculateWidth (handle, item.handle, true) : 0) + .max () + .orElse (0)); } else { long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); if (GTK.gtk_tree_model_get_iter_first (parent.modelHandle, iter)) { 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 15ca6fd4ec3..d9f4ebf3a57 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 @@ -14,6 +14,10 @@ package org.eclipse.swt.widgets; +import java.util.*; +import java.util.List; +import java.util.stream.*; + import org.eclipse.swt.*; import org.eclipse.swt.graphics.*; import org.eclipse.swt.internal.*; @@ -45,6 +49,8 @@ public class TreeItem extends Item { Font[] cellFont; String [] strings; boolean cached, grayed, isExpanded, updated, settingData; + private final List items = new ArrayList<>(); + private final TreeItem parentItem; static final int EXPANDER_EXTRA_PADDING = 4; /** @@ -70,7 +76,7 @@ public class TreeItem extends Item { * @see Widget#getStyle */ public TreeItem (Tree parent, int style) { - this (checkNull (parent), 0, style, -1, 0); + this (checkNull (parent), null, style, -1, 0); } /** @@ -103,7 +109,7 @@ public TreeItem (Tree parent, int style) { * @see Tree#setRedraw */ public TreeItem (Tree parent, int style, int index) { - this (checkNull (parent), 0, style, checkIndex (index), 0); + this (checkNull (parent), null, style, checkIndex (index), 0); } /** @@ -129,7 +135,7 @@ public TreeItem (Tree parent, int style, int index) { * @see Widget#getStyle */ public TreeItem (TreeItem parentItem, int style) { - this (checkNull (parentItem).parent, parentItem.handle, style, -1, 0); + this (checkNull (parentItem).parent, parentItem, style, -1, 0); } /** @@ -158,19 +164,56 @@ public TreeItem (TreeItem parentItem, int style) { * @see Tree#setRedraw */ public TreeItem (TreeItem parentItem, int style, int index) { - this (checkNull (parentItem).parent, parentItem.handle, style, checkIndex (index), 0); + this (checkNull (parentItem).parent, parentItem, style, checkIndex (index), 0); } -TreeItem (Tree parent, long parentIter, int style, int index, long iter) { +TreeItem (Tree parent, TreeItem parentItem, int style, int index, long iter) { super (parent, style); this.parent = parent; - if (iter == 0) { - parent.createItem (this, parentIter, index); - } else { - assert handle == 0; - handle = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (handle == 0) error(SWT.ERROR_NO_HANDLES); - C.memmove(handle, iter, GTK.GtkTreeIter_sizeof ()); + this.parentItem = parentItem; + long parentIter = parentItem != null ? parentItem.handle : 0; + if (handle != 0) throw new AssertionError("A new instance can't have a preallocated handle"); + handle = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); + if (handle == 0) error(SWT.ERROR_NO_HANDLES); + + try { + List siblings = getSiblings (); + if (iter == 0) { + if (index == -1) { + index = siblings.size(); + } + /* + * Try to achieve maximum possible performance in bulk insert scenarios. + * Even a single call to 'gtk_tree_model_iter_n_children' already + * reduces performance 3x, so try to avoid any unneeded API calls. + */ + if (index < 0 || index > siblings.size ()) { + error (SWT.ERROR_INVALID_RANGE); + } else if (index == 0) { + GTK.gtk_tree_store_prepend (parent.modelHandle, handle, parentIter); + siblings.add (0, this); + } else if (siblings.size() == index) { + GTK.gtk_tree_store_append (parent.modelHandle, handle, parentIter); + siblings.add (this); + } else { + /* + * Feature in GTK. It is much faster to append to a tree store + * than to insert at the end using gtk_tree_store_insert(). + */ + GTK.gtk_tree_store_insert (parent.modelHandle, handle, parentIter, index); + siblings.add(index, this); + } + parent.itemCreated(this); + } else { + if (index < 0 || index >= siblings.size ()) error (SWT.ERROR_INVALID_RANGE); + C.memmove(handle, iter, GTK.GtkTreeIter_sizeof ()); + siblings.set(index, this); + } + assert treeStoreIndexOf() == index; + } catch (Throwable e) { + OS.g_free (handle); + handle = 0; + throw e; } } @@ -320,7 +363,12 @@ void clear () { */ public void clear (int index, boolean all) { checkWidget (); - parent.clear (handle, index, all); + TreeItem item = items.get (index); + if (item == null) return; + item.clear(); + if (all) { + item.clearAll (all); + } } /** @@ -344,13 +392,24 @@ public void clear (int index, boolean all) { */ public void clearAll (boolean all) { checkWidget (); - parent.clearAll (all, handle); + for (TreeItem item: items) { + if (item != null) { + item.clear(); + if (all) item.clearAll(all); + } + } } @Override void destroyWidget () { - parent.releaseItem (this, false); - parent.destroyItem (this); + getSiblings ().remove (this); + parent.itemDestroyed (this); + long selection = GTK.gtk_tree_view_get_selection (parent.handle); + OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); + GTK.gtk_tree_store_remove (parent.modelHandle, handle); + OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); + parent.modelChanged = true; + releaseHandle (); } @@ -770,7 +829,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 items.size(); } /** @@ -793,15 +852,13 @@ public int getItemCount () { public TreeItem getItem (int index) { checkWidget(); if (index < 0) error (SWT.ERROR_INVALID_RANGE); + if (index >= items.size()) error (SWT.ERROR_INVALID_RANGE); if (!parent.checkData (this)) error (SWT.ERROR_WIDGET_DISPOSED); + return parent._getItem(this, handle, items, 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); - } +TreeItem _peekItem(int index) { + return items.get(index); } /** @@ -823,7 +880,7 @@ public TreeItem getItem (int index) { public TreeItem [] getItems () { checkWidget(); if (!parent.checkData (this)) error (SWT.ERROR_WIDGET_DISPOSED); - return parent.getItems (handle); + return parent._getItems(this, items, handle); } @Override @@ -863,19 +920,7 @@ public Tree getParent () { */ public TreeItem getParentItem () { checkWidget(); - long path = GTK.gtk_tree_model_get_path (parent.modelHandle, handle); - TreeItem item = null; - int depth = GTK.gtk_tree_path_get_depth (path); - if (depth > 1) { - GTK.gtk_tree_path_up (path); - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (GTK.gtk_tree_model_get_iter (parent.modelHandle, iter, path)) { - item = parent._getItem (iter); - } - OS.g_free (iter); - } - GTK.gtk_tree_path_free (path); - return item; + return parentItem; } @Override @@ -1035,34 +1080,29 @@ public int indexOf (TreeItem item) { checkWidget(); if (item == null) error (SWT.ERROR_NULL_ARGUMENT); if (item.isDisposed()) error (SWT.ERROR_INVALID_ARGUMENT); - int index = -1; - boolean isParent = false; - long currentPath = GTK.gtk_tree_model_get_path (parent.modelHandle, handle); - long parentPath = GTK.gtk_tree_model_get_path (parent.modelHandle, item.handle); - int depth = GTK.gtk_tree_path_get_depth (parentPath); - if (depth > 1 && GTK.gtk_tree_path_up(parentPath)) { - if (GTK.gtk_tree_path_compare(currentPath, parentPath) == 0) isParent = true; - } - GTK.gtk_tree_path_free (currentPath); - GTK.gtk_tree_path_free (parentPath); - if (!isParent) return index; - long path = GTK.gtk_tree_model_get_path (parent.modelHandle, item.handle); - if (depth > 1) { + return items.indexOf (item); +} + +private int treeStoreIndexOf() { + long path = GTK.gtk_tree_model_get_path (parent.modelHandle, handle); + try { + int depth = GTK.gtk_tree_path_get_depth (path); long indices = GTK.gtk_tree_path_get_indices (path); if (indices != 0) { - int[] temp = new int[depth]; + int[] temp = new int [depth]; C.memmove (temp, indices, 4 * temp.length); - index = temp[temp.length - 1]; + return temp[temp.length - 1]; } + return -1; + } finally { + GTK.gtk_tree_path_free (path); } - GTK.gtk_tree_path_free (path); - return index; } @Override void releaseChildren (boolean destroy) { if (destroy) { - parent.releaseItems (handle); + parent._releaseItems (handle, items); } super.releaseChildren (destroy); } @@ -1085,6 +1125,10 @@ void releaseWidget () { @Override public void dispose () { + if (settingData) { + String message = "Cannot remove item."; + throw new SWTException(message); + } // Workaround to Bug489751, avoid selecting next node when selected node is disposed. Tree tmpParent = null; if (parent != null && parent.getItemCount() > 0 && parent.getSelectionCount() == 0) { @@ -1106,25 +1150,7 @@ public void dispose () { */ public void removeAll () { checkWidget (); - long modelHandle = parent.modelHandle; - int length = GTK.gtk_tree_model_iter_n_children (modelHandle, handle); - if (length == 0) return; - long iter = OS.g_malloc (GTK.GtkTreeIter_sizeof ()); - if (iter == 0) error (SWT.ERROR_NO_HANDLES); - long selection = GTK.gtk_tree_view_get_selection (parent.handle); - int [] value = new int [1]; - while (GTK.gtk_tree_model_iter_children (modelHandle, iter, handle)) { - GTK.gtk_tree_model_get (modelHandle, iter, Tree.ID_COLUMN, value, -1); - TreeItem item = value [0] != -1 ? parent.items [value [0]] : null; - if (item != null && !item.isDisposed ()) { - item.dispose (); - } else { - OS.g_signal_handlers_block_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - GTK.gtk_tree_store_remove (modelHandle, iter); - OS.g_signal_handlers_unblock_matched (selection, OS.G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, CHANGED); - } - } - OS.g_free (iter); + setItemCount(0); } /** @@ -1634,9 +1660,9 @@ public void setImage (Image [] images) { * @since 3.2 */ public void setItemCount (int count) { + if (count < 0) error (SWT.ERROR_INVALID_RANGE); checkWidget (); - count = Math.max (0, count); - parent.setItemCount (handle, count); + parent._setItemCount(this, this.handle, items, count); } /** @@ -1715,4 +1741,25 @@ public void setText (String [] strings) { if (string != null) setText (i, string); } } + +List getSiblings() { + if (parentItem != null) { + return parentItem.items; + } else { + return parent.roots; + } +} + +static final void ensureSizeAtLeast(List list, int size, T fill) { + int extra = size - list.size(); + if (extra > 0) { + list.addAll(Collections.nCopies(extra, fill)); + } +} + +Stream getKnownChildrenRecursively() { + return items.stream() + .filter(Objects::nonNull). + flatMap( item -> Stream.concat(Stream.of(item), item.getKnownChildrenRecursively())); +} } 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..7bc58327865 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 @@ -1143,7 +1143,7 @@ public void test_setItemCount_indexOf() { // Issue 333 { item_0.setItemCount(5); // This causes cached index to be reset - item_0.indexOf(item_0_4); // This causes index to be cached again + assertEquals(4, item_0.indexOf(item_0_4)); // This causes index to be cached again item_0.setItemCount(10); // This causes index to be recalculated assertEquals(4, item_0.indexOf(item_0_4)); } 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 4b759bce208..4fa7319f377 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 @@ -221,7 +221,6 @@ public void dispose() { }); } - @Test public void getForeground() { assertMaximumDegree(1.2, n -> {