Re: [Crash-utility] Speed up list/tree '-s' output.

----- 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(a)redhat.com <crash-utility-bounces(a)redhat.com&gt; > on > behalf of Dave Anderson <anderson(a)redhat.com&gt; > 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/41abc6f9c91c2a6029... > > > > Thanks, > > Alexandr > > > > -- > > Crash-utility mailing list > > Crash-utility(a)redhat.com > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility > > -- > Crash-utility mailing list > Crash-utility(a)redhat.com > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility(a)redhat.com https://www.redhat.com/mailman/listinfo/crash-utility