[Crash-utility] Speed up list/tree '-s' output.
Dave Anderson
anderson at redhat.com
Thu Aug 25 18:34:00 UTC 2016
Alexandr,
The symtab.c patch has been queued for crash-7.1.6:
https://github.com/crash-utility/crash/commit/5e6fbde738b827e2575aa9ec9ba4b3eb02ac65c5
Nice fix!
Thanks,
Dave
----- Original Message -----
>
>
> ----- Original Message -----
> > Hi Dave,
> >
> > here is patch with arguments instead of static variables:
>
> Looks good, thanks!
>
> I'm still working with the symtab.c patch, because as it turns out, it
> can obviate the need for ANON_MEMBER_OFFSET_INIT() in most cases, which
> will speed up session initialization. Pretty nifty...
>
> Thanks,
> Dave
>
>
>
> >
> > --- gdb-7.6.orig/gdb/symtab.c 2016-08-18 09:06:33.000000000 +0300
> > +++ gdb-7.6/gdb/symtab.c 2016-08-24 08:40:45.700772046 +0300
> > @@ -5122,7 +5122,7 @@
> > #define GDB_COMMON
> > #include "../../defs.h"
> >
> > -static void get_member_data(struct gnu_request *, struct type *);
> > +static void get_member_data(struct gnu_request *, struct type *, long,
> > char);
> > static void dump_enum(struct type *, struct gnu_request *);
> > static void eval_enum(struct type *, struct gnu_request *);
> > static void gdb_get_line_number(struct gnu_request *);
> > @@ -5327,7 +5327,7 @@
> > req->typecode = TYPE_CODE(sym->type);
> > req->length = TYPE_LENGTH(sym->type);
> > if (req->member)
> > - get_member_data(req, sym->type);
> > + get_member_data(req, sym->type, 0, 1);
> >
> > if (TYPE_CODE(sym->type) == TYPE_CODE_ENUM) {
> > if (req->flags & GNU_PRINT_ENUMERATORS)
> > @@ -5397,7 +5397,7 @@
> > }
> >
> > if (req->member)
> > - get_member_data(req, type);
> > + get_member_data(req, type, 0, 1);
> >
> > break;
> >
> > @@ -5480,7 +5480,7 @@
> > * member field, and when found, return its relevant data.
> > */
> > static void
> > -get_member_data(struct gnu_request *req, struct type *type)
> > +get_member_data(struct gnu_request *req, struct type *type, long offset,
> > char is_first)
> > {
> > register short i;
> > struct field *nextfield;
> > @@ -5492,7 +5492,7 @@
> > nfields = TYPE_MAIN_TYPE(type)->nfields;
> > nextfield = TYPE_MAIN_TYPE(type)->flds_bnds.fields;
> >
> > - if (nfields == 0) {
> > + if (nfields == 0 && is_first /* The first call */) {
> > struct type *newtype;
> > newtype = lookup_transparent_type(req->name);
> > if (newtype) {
> > @@ -5505,13 +5505,18 @@
> >
> > for (i = 0; i < nfields; i++) {
> > if (STREQ(req->member, nextfield->name)) {
> > - req->member_offset = nextfield->loc.bitpos;
> > + req->member_offset = offset + nextfield->loc.bitpos;
> > req->member_length = TYPE_LENGTH(nextfield->type);
> > req->member_typecode = TYPE_CODE(nextfield->type);
> > if ((req->member_typecode == TYPE_CODE_TYPEDEF) &&
> > (typedef_type = check_typedef(nextfield->type)))
> > req->member_length = TYPE_LENGTH(typedef_type);
> > return;
> > + } else if (*nextfield->name == 0) { /* Anonymous struct/union */
> > + get_member_data(req, nextfield->type,
> > + offset + nextfield->loc.bitpos, 0);
> > + if (req->member_offset != -1)
> > + return;
> > }
> > nextfield++;
> > }
> > @@ -5706,7 +5711,7 @@
> > }
> >
> > if (req->member)
> > - get_member_data(req, type);
> > + get_member_data(req, type, 0, 1);
> >
> > do_cleanups (old_chain);
> > }
> >
> > Thanks,
> > Alexandr
> > ________________________________________
> > From: crash-utility-bounces at redhat.com <crash-utility-bounces at redhat.com>
> > on
> > behalf of Dave Anderson <anderson at redhat.com>
> > Sent: Tuesday, August 23, 2016 23:17
> > To: Discussion list for crash utility usage, maintenance and development
> > Subject: Re: [Crash-utility] Speed up list/tree '-s' output.
> >
> > Hi Alexandr,
> >
> > I'm still testing and sanity-checking the symtab.c patch given that it's
> > modifying such a commonly-called function. Interestingly enough, it's
> > actually uncovered a bug with the current determination of the page.list
> > members, which I found by saving the output of "help -o" into a file,
> > both without and then with your patch, into /tmp/old and /tmp/new, and
> > then diff'ed the two files:
> >
> > # diff /tmp/old /tmp/new
> > 221,223c221,223
> > < page_list: -1
> > < page_list_next: -1
> > < page_list_prev: -1
> > ---
> > > page_list: 32
> > > page_list_next: 32
> > > page_list_prev: 40
> > #
> >
> > Fortunately those fields aren't used by the crash code for more recent
> > kernels, which have gone crazy with the anonymous structures in the page
> > structure. But anyway, that was a good catch by your patch.
> >
> > On the other hand, I don't like the use of the deep and offset static
> > variables, because there's always the remote possibility that a
> > CTRL-C or other function failure may happen while they are set, and
> > then leave them in a bogus state. So I would rather they get passed as
> > arguments.
> >
> > Thanks,
> > Dave
> >
> >
> >
> > ----- Original Message -----
> > > Hi Dave,
> > >
> > > here is updated patch for gdb/symtab.c:
> > >
> > > --- gdb-7.6.orig/gdb/symtab.c 2016-08-18 09:06:33.000000000 +0300
> > > +++ gdb-7.6/gdb/symtab.c 2016-08-23 14:28:45.456510348 +0300
> > > @@ -5486,13 +5486,15 @@
> > > struct field *nextfield;
> > > short nfields;
> > > struct type *typedef_type;
> > > + static int deep = 0;
> > > + static long offset = 0;
> > >
> > > req->member_offset = -1;
> > >
> > > nfields = TYPE_MAIN_TYPE(type)->nfields;
> > > nextfield = TYPE_MAIN_TYPE(type)->flds_bnds.fields;
> > >
> > > - if (nfields == 0) {
> > > + if (nfields == 0 && deep == 0 /* The first call */) {
> > > struct type *newtype;
> > > newtype = lookup_transparent_type(req->name);
> > > if (newtype) {
> > > @@ -5505,13 +5507,21 @@
> > >
> > > for (i = 0; i < nfields; i++) {
> > > if (STREQ(req->member, nextfield->name)) {
> > > - req->member_offset = nextfield->loc.bitpos;
> > > + req->member_offset = offset +
> > > nextfield->loc.bitpos;
> > > req->member_length = TYPE_LENGTH(nextfield->type);
> > > req->member_typecode = TYPE_CODE(nextfield->type);
> > > if ((req->member_typecode == TYPE_CODE_TYPEDEF) &&
> > > (typedef_type =
> > > check_typedef(nextfield->type)))
> > > req->member_length =
> > > TYPE_LENGTH(typedef_type);
> > > return;
> > > + } else if (*nextfield->name == 0) { /* Anonymous
> > > struct/union
> > > */
> > > + deep++;
> > > + offset += nextfield->loc.bitpos;
> > > + get_member_data(req, nextfield->type);
> > > + offset -= nextfield->loc.bitpos;
> > > + deep--;
> > > + if (req->member_offset != -1)
> > > + return;
> > > }
> > > nextfield++;
> > > }
> > > @@ -5705,7 +5715,7 @@
> > > req->target_length = target_type->length;
> > > }
> > >
> > > - if (req->member)
> > > + if (req->member)
> > > get_member_data(req, type);
> > >
> > > do_cleanups (old_chain);
> > >
> > > The patch for crash tool can be found on github:
> > > https://raw.githubusercontent.com/hziSot/crash-patches/41abc6f9c91c2a602973ff9564234395deadb80e/crash_speed_up_tree_list.patch
> > >
> > > Thanks,
> > > Alexandr
> > >
> > > --
> > > Crash-utility mailing list
> > > Crash-utility at redhat.com
> > > https://www.redhat.com/mailman/listinfo/crash-utility
> > >
> >
> > --
> > Crash-utility mailing list
> > Crash-utility at redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> >
> > --
> > Crash-utility mailing list
> > Crash-utility at redhat.com
> > https://www.redhat.com/mailman/listinfo/crash-utility
> >
>
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://www.redhat.com/mailman/listinfo/crash-utility
>
More information about the Crash-utility
mailing list