Re: Bug #47090

[Ben Pfaff]

On Thu, Feb 25, 2016 at 10:02:28AM +0100, Friedrich Beckmann wrote:
> Am 24.02.2016 um 17:21 schrieb John Darrington <address@hidden>:
> > Regarding Bug #47090:
> > 
> > In our current git HEAD operations on the sheet are of complexity
> > O(n^2) in the number of columns.  This means that operations on data,
> > when there are a large number of variables are unacceptably slow.
> > I have to wait longer than 5 minutes for an operation on 2500 variables
> > to complete (it is almost instantaneous without the GUI).
> > 
> > Ideally, these operations can be done with O(1) complexity, and that
> > is what happens with the "efficient-sheet" I have been working on.
> > However that is not going to be ready for public consumption in the
> > next few weeks.
> > 
> > In release 0.8.5 this operation was O(n) so I think we should look
> > for a short term solution which restores this behaviour.
> > 
> > I can see two possible solutions:
> > 
> > Solution 1:
> > 
> > Revert commit 8298a4c6e326a39e204ca58d686497758333a102.
> > 
> > This commit by Friedrich appears to have undone an earlier optimisation.
> > Friedrich can be elaborate on the bug you noticed which prompted you
> > to make this commit?  Maybe there is another solution?
> 
> From the commit message:
> 
> <msg>
> fixed initial show of names in header window. Variable names only showed up 
> after switching between variable and data window. The reason is that the 
> buttons of the columns are not created. This is due to interdependency 
> between the size allocation and the button creation. I removed the 
> need_button mechanism.
> </msg>

This commit message makes it sound like when PSPPIRE opens a file, it
does not display variable names.  This was never my experience, and our
users did not mention it either.  So there must have been some other
factor behind the issue that you saw.  Can you tell us what that was?

> > Solution 2:
> > 
> > The patch below fixes the time complexity, and so far as I can see
> > introduces no adverse effects.  However I haven't done any very
> > thorough testing.
> > 
> > It removes a loop which calls _pspp_sheet_view_column_cell_set_dirty
> > on all the columns.  Note that the function 
> > _pspp_sheet_view_column_cell_set_dirty
> > sets a flag:
> >  tree_column->dirty = TRUE;
> > but there is nothing in our code which reads this flag or unsets it.
> > Perhaps it is here only for historical reasons?

I think that removing this call is likely to cause problems.  I think
it's likely to prevent columns from resizing themselves as columns are
added and deleted.

However, there's a lot of redundancy in calling
_pspp_sheet_view_column_cell_set_dirty() on all the columns.  The
following code in it only needs to be done once, even if there are N
columns.  If one were to factor that out or otherwise avoid running it N
times, it would probably improve performance a great deal.

  if (tree_column->tree_view &&
      gtk_widget_get_realized (tree_column->tree_view))
    {
      _pspp_sheet_view_install_mark_rows_col_dirty (PSPP_SHEET_VIEW 
(tree_column->tree_view));
      gtk_widget_queue_resize (tree_column->tree_view);
    }