Skip to content

Commit

Permalink
Greatly speed up attribute table loading
Browse files Browse the repository at this point in the history 
Don't advise for rows added when a model reset is in progress.
Otherwise the rows are tested for sort order, etc triggering
a bunch of useless calculations, given that the model is in
the process of being reset anyway.

Tested using a 150k point shapefile, decreased attribute table
load times from 50+ seconds to 4 seconds.

Refs #16577, #16239
  • Loading branch information
@nyalldawson
nyalldawson committed May 22, 2017
1 parent af8fb04 commit b97a980
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
10 changes: 6 additions & 4 deletions src/gui/attributetable/qgsattributetablemodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ bool QgsAttributeTableModel::removeRows( int row, int count, const QModelIndex &
return true;
}

void QgsAttributeTableModel::featureAdded( QgsFeatureId fid )
void QgsAttributeTableModel::featureAdded( QgsFeatureId fid , bool resettingModel )
{
QgsDebugMsgLevel( QString( "(%2) fid: %1" ).arg( fid ).arg( mFeatureRequest.filterType() ), 4 );
bool featOk = true;
Expand Down Expand Up @@ -225,10 +225,12 @@ void QgsAttributeTableModel::featureAdded( QgsFeatureId fid )
if ( ! mIdRowMap.contains( fid ) )
{
int n = mRowIdMap.size();
beginInsertRows( QModelIndex(), n, n );
if ( !resettingModel )
beginInsertRows( QModelIndex(), n, n );
mIdRowMap.insert( fid, n );
mRowIdMap.insert( n, fid );
endInsertRows();
if ( !resettingModel )
endInsertRows();
reload( index( rowCount() - 1, 0 ), index( rowCount() - 1, columnCount() ) );
}
}
Expand Down Expand Up @@ -412,7 +414,7 @@ void QgsAttributeTableModel::loadLayer()

t.restart();
}
featureAdded( mFeat.id() );
featureAdded( mFeat.id(), true );
}

emit finished();
Expand Down
2 changes: 1 addition & 1 deletion src/gui/attributetable/qgsattributetablemodel.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ class GUI_EXPORT QgsAttributeTableModel: public QAbstractTableModel
* Launched when a feature has been added
* @param fid feature id
*/
virtual void featureAdded( QgsFeatureId fid );
virtual void featureAdded( QgsFeatureId fid, bool resettingModel = false );

/**
* Launched when layer has been deleted
Expand Down

7 comments on commit b97a980

@NathanW2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Win!

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on b97a980 May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@elpaso
Copy link
Contributor

@elpaso elpaso commented on b97a980 May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@luipir
Copy link
Contributor

@luipir luipir commented on b97a980 May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

elegant solution, but some questions:
when featureAdded would be called to trigger model reset? the end of editing? the end of coping, the end of something. Is it a way to move control outside the the MV qt model to a business function level?

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feature added is used in two places - one is when the table is already visible and a new feature is actually added to the layer. The other is for every feature added to the table when the table is being loaded. This is when it's being used whole the model is being reset.

I'd say it's more of a workaround qt's model handling. I think any rowsAboutToBeInserted, ... should always be ignored if a model is in the process of being reset. But it seems this isn't the case, so adding the thousands of features when loading the table triggers a whole bunch of expensive sort handling after every feature is added.

@luipir
Copy link
Contributor

@luipir luipir commented on b97a980 May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for me :)

@gioman
Copy link
Contributor

@gioman gioman commented on b97a980 May 22, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nyalldawson

b) 2.18.7

rendering time:
real 0m25.341s
user 0m19.684s
sys 0m1.120s

table
in 9:30 minutes it loadaed about half of ~758k records (the I aborted)

Please test b97a980

  • I think this should fix it.

with your patch

2.18.8

rendering time:
real 0m38.663s
user 0m33.572s
sys 0m1.244s

so... quite slower in rendering my test 758k ploygon layer.

table:
it took 27 seconds to open the whole table, which is 10 seconds slower
than on 2.14.4

cheers!

Please sign in to comment.