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

Alexandr Terekhov Alexandr_Terekhov at epam.com
Wed Aug 24 05:45:46 UTC 2016


Hi Dave,

here is patch with arguments instead of static variables:

--- 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




More information about the Crash-utility mailing list