Re: [lmi] UI updates during long operations

[Greg Chicares]

On 2018-06-26 16:27, Vadim Zeitlin wrote:
> On Tue, 26 Jun 2018 15:15:27 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> Might it therefore be the case that, when UponPasteCensus() calls
> GC> wxSafeYield(), it pulls queued events out of the message loop, and
> GC> then they are prevented from having any effect? because "it disables
> GC> the user input", so in effect it's as though we had written
> GC>   {
> GC>   wxWindowDisabler xyzzy;
> GC>   wxYield();
> GC>   }
> GC> ?
> 
>  Yes, this is exactly what happens. Knowing, now, that the keyboard/mouse
> events are queued for later execution -- and not taken into account
> immediately as I originally thought -- I don't have any trouble with
> explaining this behaviour any longer: without any calls to wxYield(), the
> keyboard messages synthesized by Windows itself for the application remain
> in its message queue and are converted to wx events, which are then
> dispatched as usual, when the program eventually gets back to the event
> loop. With wxSafeYield(), these events are dispatched from inside it, while
> the window is disabled, and so are just ignored. Of course, this just
> spells out in more details the same conclusion that you've already arrived
> to empirically, so I realize this is not really helpful

No, it's very helpful, thanks. I just have two questions.

>  The only thing I can add is that while we can rely on the latter
> behaviour, I don't think we should really count on the former because the
> message queue could overflow (it is relatively small) and if there were
> sufficiently many unprocessed messages in it, the keyboard events could
> just be lost.

Which behaviors do "former" and "latter" apply to? Quoting from above:

> With wxSafeYield(), these events are dispatched from inside it, while
> the window is disabled, and so are just ignored.

Is it that
  "former" = "events...dispatched...while...disabled", and
  "latter" = "just ignored"?

If so, then
 - we can rely on an event being ignored, if it's dispatched from
   inside the wx event loop while the window is disabled; but
 - we can't rely on all events being dispatched from inside the
   wx event loop, even though we call wxSafeYield(), because wx
   events are held in a small queue that could overflow.

But I doubt I've interpreted that correctly, because it would lead
to the conclusion that
 - if the queue doesn't overflow, events are read from the queue and
   dispatched, but without effect because all windows are disabled;
 - if the queue does overflow, events are discarded, without effect;
but either way the result is the same. The first way, we look at an
event, do nothing with it, and then send it into a black hole; the
second way, we send it into a black hole without even looking at it.

That was my easier question. The hard one is: should lmi ever use
wxSafeYield(), and why, and how? I looked through
  git log -G'wxSafeYield' --stat
and it would seem that I was the original author of at least the
majority of these calls, but it's pretty likely that I didn't really
understand what I was doing. IIRC, I needed a "yield" function, and
automatically chose the one that's "safe" because safety sounds like
a good thing. Grepping the list archives for "SafeYield" reveals no
evidence that I ever understood this function, to put it mildly.

What's its intended purpose in wx? AFAICT, it's twofold:

 (A) wxSafeYield(SomeWindow) disables every other window in the
     application, but processes events for SomeWindow. Maybe that
     would be useful in a context like
       GetStatusBar()->SetStatusText("Making progress...");
       wxSafeYield(GetStatusBar());
     (but I've never done quite that in lmi).

 (B) wxSafeYield() with default argument NULL disables every window
     in the application, and relinquishes the app's timeslice. Maybe
     that's why I originally started using it, on a pentium 4 with
     one physical core, for which I may not even have enabled
     hyperthreading: it may have prevented a long lmi operation from
     preempting every other process on that msw-95 machine. This just
     seems quaint nowadays, when I usually have 31 idle cores. How is
       wxSafeYield();
     really different from
       {wxWindowDisabler foo;} // Disable events for zero nanoseconds.
     ? Is it just that it surrenders the app's timeslice? Does it even
     make sense to speak of timeslices in a millennium where preemptive
     multitasking is presumably universal even for refrigerators?

Surveying lmi's current use of wxSafeYield():
  /opt/lmi/src/lmi[0]$grep wxSafeYield *.?pp |less -S
I see that it is never given an argument (it always defaults to NULL),
and it's used almost exclusively in unit tests, the only exception
being in CensusView::UponPasteCensus():
    status() << "Added cell number " << cells.size() << '.' << std::flush;
    wxSafeYield();
The apparent purpose there is to let the statusbar update smoothly,
but as recently reported it updates choppily, presumably because the
invocation should be
    wxSafeYield(pointer_to_statusbar_window);
Now I wonder how that statusbar manages to get even choppy updates:
it would seem that the CPU is constantly busy, preventing the wx
event loop from doing anything...except during wxSafeYield(), which
should disable the statusbar so that it can't be updated.

I wouldn't be surprised if every call lmi makes to wxSafeYield() is
actually a mistake. [I'm removing one that certainly was.]

> GC> Let me send this now, and defer replying to the rest of your comments.
> 
>  This was a good idea as I think we can close this particular subthread as
> we've established that the observed behaviour does conform to the
> expectations, finally, so there is no mystery here. Let's now concentrate
> on how this behaviour could be improved and I'd like to hear what do you
> think about it.

My main concern was to prevent events from being processed asynchronously
and stomping on each other, e.g.:

  Census | Paste census
    ...pasting cell #1...2...3...4
  Census | Add cell
    Added one cell.
    ...pasting cell #5...6...7...

But my diligent attempts to trigger such misbehavior failed. Apparently wx
event processing waits until one command has finished before beginning
another. I don't see any problematic behavior here that needs improvement.
AFAICT, therefore, nothing's broken after all, and this discussion is not
as urgent as I had initially feared.

Then there's the whole matter of wxSafeYield(), where I suspect that a
thorough audit of the small handful of cases where lmi uses it will find
that all are dubious.

The only other concern that comes to mind right now is the one you raised,
that wxBusyCursor ought to be avoided or somehow conditionalized when it
might just flicker annoyingly for a single eyeblink. Paraphrasing our
earlier discussion, delaying its onset means using a progress-meter API,
which is so complicated that we'd rather not use it casually. An intriguing
alternative is to show something on the statusbar, because that's less
obtrusive that changing the cursor for a fleeting instant. If it says only
"Pasted cell 1", that's fine; if it counts up to "Pasted cell 1691", that's
also fine. Writing such a message to the statusbar is probably a simple,
self-contained, one-line change that we can drop in anywhere.

In the changeset I'm just about to push, I've
 - reverted the busy-cursor commit that started this thread;
 - called wxWindow::Update() when writing to the statusbar as you suggested;
 - added a status-bar counter in CensusView::DoCopyCensus(), in lieu of
   the former busy cursor.
The last of those three changes is experimental, as it raises an interesting
question: would you rather see chatter on the statusbar during an operation
that takes an appreciable fraction of a second for a 1691-cell census (and
presumably several seconds for one with more than 10000 cells), or would
you rather have that operation take only half as long? Or--perhaps even
better--in a case like this, since we're already using something like the
progress-meter API, would we be better off with the 100-msec-delayed busy
cursor previously discussed, assuming that its overhead is less than this
rather hungry statusbar's? OTOH, because counting to 1691 on the statusbar
adds the same amount of time to any 1691-fold operation, if it's negligible
for one, should we deem it negligible for any operation?