Re: [Patch] More responsive QUITs

Sterling Augustine
2011-07-20T18:22:38Z

[-- Attachment #1: Type: text/plain, Size: 2961 bytes --]

On Fri, Jul 15, 2011 at 1:46 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Sterling" == Sterling Augustine <saugustine@google.com> writes:
>
> Sterling> Enclosed is a patch that adds calls to QUIT in various places which
> Sterling> makes GDB more responsive to CTRL-C. In particular, reading a symbol
> Sterling> file at start up can now be interrupted (although perhaps still not as
> Sterling> responsive as one might like), as well as various other long running
> Sterling> operations. I know at least two GDB developers have one or another of
> Sterling> these on their TODO lists.
>
> Yeah, I'm one of those.
>
> Sterling> All three of these calls to QUIT have relatively close cleanup
> Sterling> handlers:
> [...]
>
> I agree these are safe as to their immediate surrounds; but the problem
> is what happens later on.  That is, I think a quit when building
> psymtabs will just leave the remaining psymtabs unread.  This will
> greatly interfere with debugging.

In my tests with the original patch, interrupting the startup
symbol-file read leaves it unavailable, and therefore needing to be
manually read or some such. That seemed reasonable to me. (But this
feature leaks memory, and so I'm withdrawing that part. See below.)

But interrupting any other time in the new locations doesn't leave
things undebuggable in any way that I can see. Every operation that
wasn't completed is retried on demand, and seems to work as far as I
can tell. I'm happy to try tests other than poking around in some of
my large files (where this is a real issue), if you have any
suggestions.

> We have lazy psymtab reading now.  So, it is tempting to try to record
> where the processing was stopped and then restart it there.  However,
> this may be difficult to do without leaking memory, due to the use of
> obstacks.  Perhaps some leakage would be ok, though; or maybe by making
> the granularity a single psymtab it would be possible not to leak at all
> (not sure).

I think that is already done. The new QUIT is guarded by "ps->readin",
so you effectively restart where you left off.

In any event, if I interrupt a long-process at one of the new quit
points inside dwarf2read.c, then restart it and let it run to
completion, GDB consumes slightly more memory than if I just start it
and let it run to completion the first time.

So the original patch leaks memory. However, without the QUITs inside
dwarf2read.c, but leave the new one in map_symbol_filenames_psymtab
then the leaks go away. This still solves my original tab-completion
quitting problem, and I can still debug.

This does remove the ability to break out of the initial symbol-file
read, which would have been a nice feature, but sounds like one to
defer to another day.

Updated patch enclosed.

Sterling

2011-07-20 Sterling Augustine <saugustine@google.com>

* psymtab.c (map_symbol_filenames_psymtab): Call QUIT.

[-- Attachment #2: quit.diff --]
[-- Type: text/x-patch, Size: 433 bytes --]

Index: psymtab.c
===================================================================
RCS file: /cvs/src/src/gdb/psymtab.c,v
retrieving revision 1.30
diff -d -u -r1.30 psymtab.c
--- psymtab.c 10 Jun 2011 21:48:04 -0000 1.30
+++ psymtab.c 20 Jul 2011 18:18:10 -0000
@@ -1093,6 +1093,7 @@
if (ps->readin)
continue;

+ QUIT;
fullname = psymtab_to_fullname (ps);
(*fun) (ps->filename, fullname, data);
}