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@xxxxxxxxxx <crash-utility-bounces@xxxxxxxxxx> > > on > > behalf of Dave Anderson <anderson@xxxxxxxxxx> > > Sent: Tuesday, August 23, 2016 23:17 > > To: Discussion list for crash utility usage, maintenance and development > > Subject: Re: 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@xxxxxxxxxx > > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > > > > -- > > Crash-utility mailing list > > Crash-utility@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > -- > > Crash-utility mailing list > > Crash-utility@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > -- > Crash-utility mailing list > Crash-utility@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/crash-utility > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
