From 0ab446140dd0ed6c00b61ffe8b0d17ba33d48d7e Mon Sep 17 00:00:00 2001 From: sbesombes Date: Tue, 6 Apr 2021 21:00:27 +0200 Subject: [PATCH 1/4] Speed up inserting items in sorted wxDataViewCtrl in wxGTK Avoid resorting the tree on each insertion, this results in horrible performance even with a few thousand items. Instead, sort all the new items only once at the end. --- src/gtk/dataview.cpp | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index 935f9ef11c..e789597f9e 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -244,6 +244,9 @@ public: bool IsSorted() const { return m_sort_column >= 0; } + void SetAllowSort( bool allowed ) { m_allow_sort = allowed; } + bool GetAllowSort() const { return m_allow_sort; } + // Should we be sorted either because we have a configured sort column or // because we have a default sort order? bool ShouldBeSorted() const @@ -286,6 +289,7 @@ private: GtkSortType m_sort_order; wxDataViewColumn *m_dataview_sort_column; int m_sort_column; + bool m_allow_sort; GtkTargetEntry m_dragSourceTargetEntry; wxCharBuffer m_dragSourceTargetEntryTarget; @@ -352,7 +356,7 @@ public: m_children.Add( id ); - if (m_internal->ShouldBeSorted()) + if (m_internal->GetAllowSort() && m_internal->ShouldBeSorted()) { gs_internal = m_internal; m_children.Sort( &wxGtkTreeModelChildCmp ); @@ -399,7 +403,7 @@ public: { m_children.Insert( id, pos ); - if (m_internal->ShouldBeSorted()) + if (m_internal->GetAllowSort() && m_internal->ShouldBeSorted()) { gs_internal = m_internal; m_children.Sort( &wxGtkTreeModelChildCmp ); @@ -451,6 +455,9 @@ public: wxDataViewItem &GetItem() { return m_item; } wxDataViewCtrlInternal *GetInternal() { return m_internal; } + void AllowSort() { m_internal->SetAllowSort(true); } + void ForbidSort() { m_internal->SetAllowSort(false); } + void Resort(); private: @@ -3719,11 +3726,17 @@ void wxDataViewCtrlInternal::BuildBranch( wxGtkTreeModelNode *node ) wxDataViewItemArray children; unsigned int count = m_wx_model->GetChildren( node->GetItem(), children ); + // node will be sorted after last child is inserted + node->ForbidSort(); + unsigned int pos; for (pos = 0; pos < count; pos++) { wxDataViewItem child = children[pos]; + if (pos==count-1) + node->AllowSort(); + if (m_wx_model->IsContainer( child )) node->AddNode( new wxGtkTreeModelNode( node, child, this ) ); else @@ -3731,6 +3744,7 @@ void wxDataViewCtrlInternal::BuildBranch( wxGtkTreeModelNode *node ) // Don't send any events here } + node->AllowSort(); // in case there is no child } } From 8c9c16a578a16559e5eb0c564436850e7c7e6275 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 7 Apr 2021 01:57:37 +0200 Subject: [PATCH 2/4] Initialize new m_allow_sort member Its initial value is otherwise undefined. --- src/gtk/dataview.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index e789597f9e..677288903d 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -3631,6 +3631,7 @@ wxDataViewCtrlInternal::wxDataViewCtrlInternal( wxDataViewCtrl *owner, wxDataVie m_root = NULL; m_sort_order = GTK_SORT_ASCENDING; m_sort_column = -1; + m_allow_sort = true; m_dataview_sort_column = NULL; m_dragDataObject = NULL; From d173c3f74b0c7072f62bf2e24155d89a7cc60a58 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 7 Apr 2021 02:01:47 +0200 Subject: [PATCH 3/4] Rename the new sort-related methods Don't use SetAllowSort() in one class and {Allow,Forbid}Sort() in another one, both could be fine on their own but not together. Also simplify code by forbidding sorting only if necessary (and add a comment explaining why do we do it), which removes the need for re-allowing it from two different places. --- src/gtk/dataview.cpp | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index 677288903d..42e60bbff6 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -244,8 +244,8 @@ public: bool IsSorted() const { return m_sort_column >= 0; } - void SetAllowSort( bool allowed ) { m_allow_sort = allowed; } - bool GetAllowSort() const { return m_allow_sort; } + void AllowSort(bool allowed) { m_allow_sort = allowed; } + bool IsSortAllowed() const { return m_allow_sort; } // Should we be sorted either because we have a configured sort column or // because we have a default sort order? @@ -356,7 +356,7 @@ public: m_children.Add( id ); - if (m_internal->GetAllowSort() && m_internal->ShouldBeSorted()) + if ( m_internal->IsSortAllowed() && m_internal->ShouldBeSorted() ) { gs_internal = m_internal; m_children.Sort( &wxGtkTreeModelChildCmp ); @@ -403,7 +403,7 @@ public: { m_children.Insert( id, pos ); - if (m_internal->GetAllowSort() && m_internal->ShouldBeSorted()) + if (m_internal->IsSortAllowed() && m_internal->ShouldBeSorted() ) { gs_internal = m_internal; m_children.Sort( &wxGtkTreeModelChildCmp ); @@ -455,8 +455,7 @@ public: wxDataViewItem &GetItem() { return m_item; } wxDataViewCtrlInternal *GetInternal() { return m_internal; } - void AllowSort() { m_internal->SetAllowSort(true); } - void ForbidSort() { m_internal->SetAllowSort(false); } + void AllowSort(bool allow) { m_internal->AllowSort(allow); } void Resort(); @@ -3727,16 +3726,20 @@ void wxDataViewCtrlInternal::BuildBranch( wxGtkTreeModelNode *node ) wxDataViewItemArray children; unsigned int count = m_wx_model->GetChildren( node->GetItem(), children ); - // node will be sorted after last child is inserted - node->ForbidSort(); + // Avoid sorting children multiple times when inserting many nodes, + // this results in O(N^2*log(N)) complexity. + if ( count > 1 ) + node->AllowSort(false); unsigned int pos; for (pos = 0; pos < count; pos++) { wxDataViewItem child = children[pos]; - if (pos==count-1) - node->AllowSort(); + // Rr-enable sorting before inserting the last child if we had + // disabled it above. + if ( pos == count - 1 && pos != 0 ) + node->AllowSort(true); if (m_wx_model->IsContainer( child )) node->AddNode( new wxGtkTreeModelNode( node, child, this ) ); @@ -3745,7 +3748,6 @@ void wxDataViewCtrlInternal::BuildBranch( wxGtkTreeModelNode *node ) // Don't send any events here } - node->AllowSort(); // in case there is no child } } From 0a0a571ee204c3c4c0f0a8739a1868da8778a504 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 7 Apr 2021 02:06:12 +0200 Subject: [PATCH 4/4] Rename AllowSort() to FreezeSort() This better reflects the temporary nature of forbidding sort. Also add a comment describing this better. --- src/gtk/dataview.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/gtk/dataview.cpp b/src/gtk/dataview.cpp index 42e60bbff6..81de210a39 100644 --- a/src/gtk/dataview.cpp +++ b/src/gtk/dataview.cpp @@ -244,8 +244,12 @@ public: bool IsSorted() const { return m_sort_column >= 0; } - void AllowSort(bool allowed) { m_allow_sort = allowed; } - bool IsSortAllowed() const { return m_allow_sort; } + // Freezing sort temporarily disables sorting when inserting the items into + // the tree as an optimization when inserting many items. Sort must be + // thawed, by calling FreezeSort(false), before inserting the last item to + // ensure that the items still end up in the right order. + void FreezeSort(bool freeze) { m_sort_frozen = freeze; } + bool IsSortFrozen() const { return m_sort_frozen; } // Should we be sorted either because we have a configured sort column or // because we have a default sort order? @@ -289,7 +293,7 @@ private: GtkSortType m_sort_order; wxDataViewColumn *m_dataview_sort_column; int m_sort_column; - bool m_allow_sort; + bool m_sort_frozen; GtkTargetEntry m_dragSourceTargetEntry; wxCharBuffer m_dragSourceTargetEntryTarget; @@ -356,7 +360,7 @@ public: m_children.Add( id ); - if ( m_internal->IsSortAllowed() && m_internal->ShouldBeSorted() ) + if ( !m_internal->IsSortFrozen() && m_internal->ShouldBeSorted() ) { gs_internal = m_internal; m_children.Sort( &wxGtkTreeModelChildCmp ); @@ -403,7 +407,7 @@ public: { m_children.Insert( id, pos ); - if (m_internal->IsSortAllowed() && m_internal->ShouldBeSorted() ) + if ( !m_internal->IsSortFrozen() && m_internal->ShouldBeSorted() ) { gs_internal = m_internal; m_children.Sort( &wxGtkTreeModelChildCmp ); @@ -455,7 +459,7 @@ public: wxDataViewItem &GetItem() { return m_item; } wxDataViewCtrlInternal *GetInternal() { return m_internal; } - void AllowSort(bool allow) { m_internal->AllowSort(allow); } + void FreezeSort(bool freeze) { m_internal->FreezeSort(freeze); } void Resort(); @@ -3630,7 +3634,7 @@ wxDataViewCtrlInternal::wxDataViewCtrlInternal( wxDataViewCtrl *owner, wxDataVie m_root = NULL; m_sort_order = GTK_SORT_ASCENDING; m_sort_column = -1; - m_allow_sort = true; + m_sort_frozen = false; m_dataview_sort_column = NULL; m_dragDataObject = NULL; @@ -3729,7 +3733,7 @@ void wxDataViewCtrlInternal::BuildBranch( wxGtkTreeModelNode *node ) // Avoid sorting children multiple times when inserting many nodes, // this results in O(N^2*log(N)) complexity. if ( count > 1 ) - node->AllowSort(false); + node->FreezeSort(true); unsigned int pos; for (pos = 0; pos < count; pos++) @@ -3739,7 +3743,7 @@ void wxDataViewCtrlInternal::BuildBranch( wxGtkTreeModelNode *node ) // Rr-enable sorting before inserting the last child if we had // disabled it above. if ( pos == count - 1 && pos != 0 ) - node->AllowSort(true); + node->FreezeSort(false); if (m_wx_model->IsContainer( child )) node->AddNode( new wxGtkTreeModelNode( node, child, this ) );