Opened 2 years ago

Closed 14 months ago

#14073 closed defect (fixed)

Performance issue of adding many items to a sorted wxDVC

Reported by: koichi Owned by:
Priority: normal Milestone:
Component: GUI-all Version: stable-latest
Keywords: wxDataViewCtrl, sort Cc: e.raijmakers@…
Blocked By: Blocking:
Patch: yes

Description

On wxMSW, adding a huge items to wxDVC takes too long when the tree has sorting column. This is because the tree is sorted every time an item is added. While the behavior is slightly different depending on OSes, the same problem exists on wxGTK, and probably on wxOSX.

In principle, the tree should be sorted only once after the entire items are added. Even if only a part of nodes needs to be sorted after items are added, it is difficult to check which nodes need to be sorted and which nodes need not, and when adding many items, sorting the entire tree is much much faster than sorting a parent node every time an item is added.

Since ItemsAdded internally calls wxDataViewModelNotifier::ItemAdded for each item, I want to temporarily disable sorting function before calling ItemsAdded, and after that call wxDataViewModelNotifier::Resort once.

Is there a good way to temporarily disable sorting?

Attachments (1)

datavgen.cpp.patch download (649 bytes) - added by Eric Raijmakers 14 months ago.
Patch: If control is frozen, then sorting is disabled. When Thawed a Resort is triggered.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 2 years ago by juliansmart

It sounds as though wxDVC needs a DoThaw function to sort and refresh the control, and a test for IsFrozen to avoid calling sort (and maybe other code). Then application code can call Freeze and Thaw around multiple-item adding.

Julian

comment:2 Changed 15 months ago by Eric Raijmakers

I encountered the same issue and came up with sort of a workaround.
Thought I'd just mention it:

I inspected the sources and found that sorting is done by:

if (g_column >= -1) m_branchData->children.Sort( &wxGenericTreeModelNodeCmp );

where g_column was initialized by:

void SortPrepare()
{

g_model = GetModel();
wxDataViewColumn* col = GetOwner()->GetSortingColumn();
if( !col )
{

if (g_model->HasDefaultCompare())

g_column = -1;

else

g_column = -2;

...

What I did to temporarily suspend sorting:

Overrule wxDataViewModel::HasDefaultCompare() and wxDataViewCtrl::GetSortingColumn():

bool DataViewModel::HasDefaultCompare() const
{

if (mHoldSorting)
{

return false;

}
return wxDataViewModel::HasDefaultCompare();

}

wxDataViewColumn* ProjectViewCtrl::GetSortingColumn() const
{

const DataViewModel* model = dynamic_cast<const DataViewModel*>(GetModel());
if (model->holdSorting())
{

return 0;

}
return wxDataViewCtrl::GetSortingColumn();

}

Hold/Resume of sorting is done via the following code:

void DataViewModel::holdSorting(bool hold)
{

mHoldSorting = hold;
if (!hold)
{

Resort();

}

}

So, to insert a lot of items (in the model class):
holdSorting(true);
ItemsAdded(...,...)
holdSorting(false)

comment:3 Changed 15 months ago by Eric Raijmakers

  • Cc e.raijmakers@… added

comment:4 Changed 14 months ago by vadz

  • Status changed from new to confirmed
  • Summary changed from Performance issue of sorting/adding items in wxDVC to Performance issue of adding many items to a sorted wxDVC

This is exactly what Julian meant, we need to "hold sorting" in DoFreeze() and re-enable it (and resort everything) in DoThaw().

If you -- or anybody else interested in fixing this issue -- could please submit a patch with your tested changes, it would be great.

Changed 14 months ago by Eric Raijmakers

Patch: If control is frozen, then sorting is disabled. When Thawed a Resort is triggered.

comment:5 Changed 14 months ago by Eric Raijmakers

  • Patch set

Turned out to be easier than I had expected.
Nothing needed to be done on the OnFreeze: SortPrepare is called regularly already.

comment:6 Changed 14 months ago by vadz

Is it really harmless to Resort() when we don't need to be sorted? I'm not quite sure if we really want to do this unconditionally in DoThaw()...

comment:7 Changed 14 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [73635]) Disable sorting of generic wxDataViewCtrl while it is frozen.

Don't sort the contents of wxDataViewCtrl while it is frozen and resort it
only when it is thawed. This dramatically speeds up adding items to the
control when sorting is used as we only sort it once now instead of doing it
after adding every item.

Closes #14073.

Note: See TracTickets for help on using tickets.