Re: Blocking the UI from CPU-intensive gdbmi parsing

[Jonathon Jongsma <jonathon quotidian org>]

Dodji Seketeli wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Jonathon Jongsma a écrit :
> > 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.
>
> Yes, of course. But still, many seconds, on current beefed up machines, to parse
> the list of files seems too much to me. Unless the gdb output represents some
> gigabytes of ram. And I doubt that. When I have time I'll try to profile that
> and see what I can do.

Yes, this is true.  we should really do some profiling. (just for the record, parsing the 
list of source files for nemiver itself takes approximately 6-7 seconds on my machine)

> > 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).
>
> Sure, I am aware of that. Idealy, I think gdb should notify us about its
> progress. That would let us adress this issue in a direct and consistent way
> with what I was proposing.

Yeah, that could be nice, but I guess even if gdb added that feature, we wouldn't be able 
to assume users had that version for quite a long time.

> >  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.
>
> Well, We have IDebugger::command_done_signal() that is emitted right after
> IDebugger sends the command GDB, if GDB acknowledges the command. So I *think*
> we could just start pulsing when we receive the IDebugger::command_done_signal.
> I think that would work, but would be hackish until GDB can really notify us
> about its progress.

So if I understand correctly, you're suggesting that we could handle this signal and start 
a timeout-based progress pulse.  then when we actually get the reply from gdb, we could 
disable the timeout and pulse progress as we parse the output (?)

> > 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?
>
>
> The way i see it is to have some possible long operations be invoked through an
> API that ressembles:
>
> void IDebugger::do_long_operation (sigc::slot<void, void> &a_notification_slot);
>
> a_notification_slot would be the slot invoked to notify the progress. For the
> sake of simplicity, I choosed sigc::slot<void, void> in this exemple, but it
> could be a slot that has got a more useful signature, I guess.

hm, so that looks like basically the same sort of thing as my suggestion, except you're 
passing a slot as an argument to the debugger call, whereas in my prototype I'm passing a 
signal back from the debugger call.

So in my case, the user of the debugger API would look something like this:

IProgressNotifierSafePtr progress = debugger->list_files();
progress->signal_progress_pulse ().connect (sigc::mem_fun(...));

and I've actually toyed around with the idea of adding some API to the progress notifier 
that would enable API users to cancel long-running commands, e.g.:  progress->cancel (); 
This could set a flag that the parser could check whenever it checks in to pulse the 
notifier and if it is set, it could abort the operation...  Obviously the idea is not 
fully-thought-out yet, but it could be useful.

> I am currently playing with the support of variable object, and I have
> introduced that kind of API to easy the code of the client side (not for
> notification purposes though), and it looks interesting.
>
> You can have a feeling of how to use it by looking at the code of the function
> on_variable_created_signal2 at
> http://git.gnome.org/cgit/nemiver/tree/tests/test-vars.cc?h=varobjs-support.
>
> More on that later. I need to thinker with the code a little more before being
> fully sure :)

I will take a look at your branch.  Btw, I wasn't sure that I wanted to push my 
experimental notifier branch to git.gnome.org since I'm not sure it will be the approach 
we end up using.  So I pushed it to my homedir on gnome.org instead:
 http://www.gnome.org/~jjongsma/git/nemiver.git/
Do you know if git.gnome.org will provide a simpler way to host user repositories?

If you have time, I'd love it if you could checkout my 'progress-notifier' branch and let 
me know what you think of the approach.   (If you want to see a demonstration of it in 
action, build that branch and select 'File > Open Source File ...' while debugging a 
non-trivial application).

--
jonner