Opened 11 months ago

Last modified 6 months ago

#2879 assigned defect

Cancelling a DataTableVisualPropertiesDialog may take very long and block GUI

Reported by: bburlacu Owned by: bburlacu
Priority: medium Milestone: HeuristicLab 3.3.16
Component: Analysis.Views Version: trunk
Keywords: Cc:

Description

The DataTableVisualPropertiesDialog saves the original visual properties for each row and restores them in the event handler for the dialog cancel button.

This causes each series to be redrawn in the Row_VisualPropertiesChanged event handler of the DataTableView. Inside, the DataPointCollection for each series is cleared, which causes performance problems [1], [2].

On a series with ~7000 points, the clear operation took 32s on my machine. In total, canceling the dialog blocks the HeuristicLab GUI for approximately 2 minutes.

This problem can be alleviated by implementing the fix suggested in [1], which reduces the blocking time from 2min to a few seconds on the tested data.

In the future, it might be useful to consider a different mechanism for updating/restoring row visual properties.

[1 ] https://stackoverflow.com/questions/5744930/datapointcollection-clear-performance

[2 ] https://connect.microsoft.com/VisualStudio/feedback/details/596212/performance-problem-in-mschart-datapointcollection-clear

Attachments (1)

Clear DataPointsCollection.hl (1.5 KB) - added by bburlacu 7 months ago.
data point collection performance bug

Download all attachments as: .zip

Change History (8)

comment:1 Changed 11 months ago by bburlacu

  • Status changed from new to accepted

comment:2 Changed 11 months ago by bburlacu

r15619: Performance improvement in DataTableView

comment:3 Changed 11 months ago by bburlacu

  • Owner changed from bburlacu to pfleck
  • Status changed from accepted to reviewing

comment:4 Changed 11 months ago by abeham

  • Version changed from 3.3.16 to trunk

As there is a trunk commit in this ticket, the version should be set to trunk.

comment:5 Changed 7 months ago by pfleck

  • Owner changed from pfleck to bburlacu
  • Status changed from reviewing to assigned

Tested r15619 and GUI reactiveness is noticeable better with the change.

Code looks fine. But, I do not understand why a loop with a RemoveAt is used instead of a simple Clear. I tried a Clear and seems equally fast. Additionally, I looked at the source and both RemoveAt and Clear seem to be forwarded to a List<T>.RemoveAt and List<T>.Clear. Therefore I think we should use a simple Clear. Please check if this does not have other effects I did not notice when using Clear.

Changed 7 months ago by bburlacu

data point collection performance bug

comment:6 Changed 7 months ago by bburlacu

  • Owner changed from bburlacu to pfleck
  • Status changed from assigned to reviewing

This workaround was used to avoid a performance bug referenced here: https://stackoverflow.com/questions/5744930/datapointcollection-clear-performance

Unfortunately in the mean time the MSDN link has disappeared, but in my tests simply using Clear is much slower. Using 50k points, I get:

  • ClearPoints method: 0.004 s
  • Clear method: 0.837 s

I've attached an HL script which illustrates this behavior.

comment:7 Changed 6 months ago by pfleck

  • Owner changed from pfleck to bburlacu
  • Status changed from reviewing to assigned

It seems that the ScatterPlotView has the same issue with the .Clear() calls. I guess they can be fixed in the same manner.

Can you also look for other places where your improvement could be applied?

Note: See TracTickets for help on using tickets.