QGIS » QGIS Application

For me it still not fixed. I am getting always a QGIS freezing on rollback action.

Salvatore Larosa wrote:

For me it still not fixed. I am getting always a QGIS freezing on rollback action.

It's faster. But the changes need to be rolled back one by one and therefore the time it takes to rollback is somewhat the same as to do the changes in the first place.

Matthias Kuhn wrote:

It's not quite the same, as when doing changes the first time via the field calculator, one iterator is opened and looped while in a rollback, there is one iterator per feature.

Not anymore. That was the case when the previous values were not stored in the first undo command - therefore the original value had to be retrieved from the provider using a iterator. Now that only happens if an attribute change happens when the previous value isn't already available (which is not the case in the field calculator).

Still all subscribers to the attributeValueChanged signal must be notified that the value was changed back (BTW the dualview form view doesn't update in that case).

Jürgen Fischer wrote:

Not anymore. That was the case when the previous values were not stored in the first undo command - therefore the original value had to be retrieved from the provider using a iterator. Now that only happens if an attribute change happens when the previous value isn't already available (which is not the case in the field calculator).

So the problem is the field calculator not saving the previous values?

Still all subscribers to the attributeValueChanged signal must be notified that the value was changed back (BTW the dualview form view doesn't update in that case).

I can take care of that. I think introducing an afterRollback signal should do the trick.

Ah, the form view and not the table view.

/me should read more carefully.

I will take care of this for 2.4 with the new editor components.

Matthias Kuhn wrote:

Jürgen Fischer wrote:

Not anymore. That was the case when the previous values were not stored in the first undo command - therefore the original value had to be retrieved from the provider using a iterator. Now that only happens if an attribute change happens when the previous value isn't already available (which is not the case in the field calculator).

So the problem is the field calculator not saving the previous values?

IMHO there is no problem anymore. The rollback is faster now, as it doesn't need to reload the values from the provider, but it still needs time because it needs to signal each undo - and that's by design.

We only have the attributeValueChange signal and subscribers can currently assume that they will be notified for each change. We could introduce a bulkAttributeValueChange signal (and undo command), that tells subscribers that all the values of an attribute changed (maybe include fid ranges or fid sets) and define that attributeValueChange doesn't cover bulk changes. But that would be a feature.

I am not at home to test now but when I tested yesterday it seemed to me that the field calculator is still faster than the rollback - and they should emit the same amount of signals. Maybe the decrease in performance is related to debug output the rollback produces.

bulkAttributeValueChange signal would be fine, but on the other hand I experienced that caching values retrieved from attributeChanged signals and postponing heavy operations to editCommandEnded() works very well. So the signals themselves seem to be a minor problem as long as their implementations are not performance-killers. The only remaining problem with this approach I can think of is that if several consumers cache the values it's a waste of memory.

Matthias Kuhn wrote:

I am not at home to test now but when I tested yesterday it seemed to me that the field calculator is still faster than the rollback - and they should emit the same amount of signals. Maybe the decrease in performance is related to debug output the rollback produces.

Before or after 52616b6? I didn't do benchmarking either, but the test expression to update and undo I used was trivial.

Tested again with current master.

I noticed, that when updating a column, the rollback is fast. But when I add a newly calculated column that's what takes so long to rollback.

In the latter case the debug output I mentioned is:

src/providers/postgres/qgspostgresconn.cpp: 825: (openCursor) Starting read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 839: (closeCursor) Committing read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 825: (openCursor) Starting read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 839: (closeCursor) Committing read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 825: (openCursor) Starting read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 839: (closeCursor) Committing read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 825: (openCursor) Starting read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 839: (closeCursor) Committing read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 825: (openCursor) Starting read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 839: (closeCursor) Committing read-only transaction
src/providers/postgres/qgspostgresconn.cpp: 825: (openCursor) Starting read-only transaction