Re: Blocking the UI from CPU-intensive gdbmi parsing

[Jonathon Jongsma <jonathon quotidian org>]

Dodji Seketeli wrote:
> I am really sorry to reply to this email so late. I was stuck in stupid
> other stuff.

No problem.  I'm stuck in a lot of stupid other stuff myself ;)

> Jonathon Jongsma a écrit :
> > In general, nemiver is pretty good about not blocking UI while it is
> > doing processing of various things (I/O, parsing, etc).  But sometimes
> > where there is a lot of data to parse, the UI blocks quite badly.  For
> > example, if you debug nemiver with itself, and then open the 'open file'
> > dialog (Ctrl+O), the list of source files is very long, and parsing this
> > file list takes quite a few seconds.
>
> Man, this sucks. I remember we had this problem at some point, and after
> profiling we could find some hot spots to hammer on, and we ended up
> reducing the parsing time to something quit reasonable. I should try and
>  profile this again. Maybe some new parsing code introduced some
> performance regression in this area. Who knows. Also I wonder how many
> files we have to parse that'd take that long.

Yeah, it could be worth doing some profiling again, but I think it's inevitable that some 
parsing operations may take a long time, especially for very large C++ programs.  So I 
think it would be nice to have a reasonable solution for this issue, even if we manage to 
get specific cases optimized.

> I am thinking out loud here.
>
> I think we could for instance let IDebugger provide an interface that
> would notify the user about the progress of some operation.
> It could look like:
>
> IDebuggerSafePtr debugger = get_debugger_from_somewhere ();
>
> ISomeNotifierSafePtr notifier = debugger->get_some_progress_notifier ();
>
> notifier->pulse_signal ().connect (&on_notification_pulse);
>
> /*void on_notification_pulse (); is declared somewhere*/
>
> For the particular case of file list parsing, the implementation of the
> notifier type would emit pulse events on each parsed file or something
> like that. Unfortunately, GDB/MI doesn't let us know how many files it
> is going to send us. So we can't give percentage of completion
> information. Still I think we can say "hey, we are working, we aren't
> just stuck".
>
> The implementation of on_notification_pulse () can even trigger gtk UI
> event loop iterations to avoid the UI from freezing, if need be. That's
> hackish, but it's simple to try and to debug. To see if

So, my first reaction to this idea is that it will basically get the job done and it's not 
too ugly.  However, there are a couple issues / questions I have with it.

First, it really only works for parsing operations.  For instance, consider the file list 
operation (the case that caused me to think about this issue in the first place).  The 
file list operation has two distinct parts:  the first part where gdb actually gathers the 
 list of files and spits it out to the io channel, and the second part where we parse the 
output of gdb and build it up into a list that we pass back to the application.  Imagine 
(i'm using arbitrary numbers just for illustration here) that the first part where gdb 
gathers the list of files takes 5 seconds, and the second part where we parse and build 
the list also takes 5 seconds.  So for the first 5 seconds of the operation, we will not 
be blocking, but we won't be emitting any progress pulses (since we're not doing anything 
-- we're just waiting for output).  For the second 5 seconds we'd be parsing output and 
emitting progress pulses periodically.  So we've solved the problem of blocking the UI, 
but the application user experience is a little bit inconsistent.  Because if we want a 
progress bar to pulse for the entire 10 seconds (instead of just the last 5 seconds), we 
have to find a slightly different approach.

The other question I have about the above API is how to associate a certain progress event 
 with a particular operation.  Perhaps all debugger operations that can notify about 
progress can return a progress notifier object or something, e.g.:

virtual ISomeNotifierSafePtr list_files (const UString &);

And then if you want to be notified of progress you can connect to its signals.  I'm not 
sure how the internal implementation of this would work, but it seems like a reasonably 
clean API to me.  what do you think?

> > Can we farm out some of these long intensive parsing operations (e.g.
> > GDBMIParser::parse_file_list()) out to worker threads?
>
> I am not keen on doing multi threaded stuff at this level. In theory,
> threading could certainly help here. But in practice, doing
> multi-threaded parsing in interaction with a main loop can quickly
> become really complicated and lead to instability if not crafted very
> very carefully. I'd try simpler things first, and if really they don't
> work I'd get into the multi-thread hammer solution.

yeah, I agree, this is really a last resort sort of solution, I think.

> > Can we break up the long parsing functions into smaller chunks
> > and do them in idle callbacks?
>
> This is not easy. The client sends multiple commands to GDB. Let's say
> comands C(0), C(1), and C(2). For each command C(i), the client expects
> a reply R(i). R(0) must be sent back to the client before R(1), and R(1)
> must be sent back bedore R(2). Before sending back each R(i), IDebugger
> does some processing P(i) on the result of the command C(i).
>
> So what happens is a command C(0) is sent to gdb. It triggers as reply
> from GDB. That reply is processed in P(0). As a result, a reply R(0) is
> sent to the client.
>
> So {C(0), P(0), R(0)}, {C(1), P(1), R(1)} must happen in that /order/.
> The order is important here.

Yeah, this would get really ugly really quickly.  I mentioned it since it's the other 
option that popped into my head, but I agree that it's not really an approach worth pursuing.

> I think the problem you are raising is an important one that needs
> thinking and testing. Thanks for bringing this up.

Yeah, hopefully I'll get a bit of time to think and hack on this a bit in the near future.

--
jonner