Re: [RFA] massively speed up "info var foo" on large programs

[Doug Evans <dje@google.com>]

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

On Thu, May 24, 2012 at 2:28 PM, Doug Evans <dje@google.com> wrote:
> On Thu, May 24, 2012 at 10:58 AM, Doug Evans <dje@google.com> wrote:
>> Hi.
>>
>> I'm not entirely sure this patch is correct, but it feels correct (*1),
>> and is a massive win.
>> "info var Task" in one large program goes from 350 seconds to 28 seconds.
>>
>> [...]
>>
>> 2012-05-23  Doug Evans  <dje@google.com>
>>
>>        * symtab.c (lookup_msymbol_in_objfile): New function.
>>        (search_symbols): Call it.
>
> Hmmm.
> One thing that occurs to me is separate debug objfiles.
> lookup_msymbol_in_objfile should probably search them.

This is a revised patch.
It scans separate debug files.
I think I understand the code better so I've removed the FIXME: The
comments in the code were misleading, find_pc_symtab is, I think, an
optimization to avoid unnecessarily calling lookup_symbol.
This patch also adds a (nfiles == 0) check to the second minsym loop:
there's no point in scanning minsyms for specific files.

The output is different from the previous code, I didn't take into
account the symbols that gdb creates for @plt entries.  I think if we
want to continue to provide the current output, we should add an
option to "info var|fun|type" to produce it: the normal case shouldn't
be that slow.

This patch removes the gdb-created minsyms from the output.
Another way to go is to print them.  I don't have a strong opinion on
either choice.
[The different with the current behaviour is that if the minsym is
found in any objfile then the current code won't print it in the
"Non-debugging symbols" section of the output.]

Ok to check in?

2012-05-25  Doug Evans  <dje@google.com>

        * symtab.c (minimal_symbol): New member created_by_gdb.
        * elfread.c (elf_symtab_read): Set created_by_gdb for @plt minsym
        created by gdb.
        * symtab.c (lookup_msymbol_in_objfile): New function.
        (search_symbols): Call it.  Only scan minsyms if nfiles == 0.

[-- Attachment #2: gdb-120525-search-symbols-speedup-2.patch.txt --]
[-- Type: text/plain, Size: 5697 bytes --]

2012-05-25  Doug Evans  <dje@google.com>

	* symtab.c (minimal_symbol): New member created_by_gdb.
	* elfread.c (elf_symtab_read): Set created_by_gdb for @plt minsym
	created by gdb.
	* symtab.c (lookup_msymbol_in_objfile): New function.
	(search_symbols): Call it.  Only scan minsyms if nfiles == 0.

Index: elfread.c
===================================================================
RCS file: /cvs/src/src/gdb/elfread.c,v
retrieving revision 1.131
diff -u -p -r1.131 elfread.c
--- elfread.c	18 May 2012 21:02:47 -0000	1.131
+++ elfread.c	25 May 2012 07:13:41 -0000
@@ -594,6 +594,7 @@ elf_symtab_read (struct objfile *objfile
 		  if (mtramp)
 		    {
 		      MSYMBOL_SIZE (mtramp) = MSYMBOL_SIZE (msym);
+		      mtramp->created_by_gdb = 1;
 		      mtramp->filename = filesymname;
 		      gdbarch_elf_make_msymbol_special (gdbarch, sym, mtramp);
 		    }
Index: symtab.c
===================================================================
RCS file: /cvs/src/src/gdb/symtab.c,v
retrieving revision 1.306
diff -u -p -r1.306 symtab.c
--- symtab.c	24 May 2012 02:51:48 -0000	1.306
+++ symtab.c	25 May 2012 07:13:41 -0000
@@ -1559,6 +1559,48 @@ lookup_symbol_aux_symtabs (int block_ind
   return NULL;
 }
 
+/* Wrapper around lookup_symbol_aux_objfile for search_symbols.
+   Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE
+   and all related objfiles.  */
+
+static struct symbol *
+lookup_msymbol_in_objfile (struct objfile *objfile,
+			   struct minimal_symbol *msymbol,
+			   domain_enum domain)
+{
+  const char *name = SYMBOL_LINKAGE_NAME (msymbol);
+  enum language lang = current_language->la_language;
+  const char *modified_name;
+  struct cleanup *cleanup = demangle_for_lookup (name, lang, &modified_name);
+  struct objfile *main_objfile, *cur_objfile;
+
+  if (objfile->separate_debug_objfile_backlink)
+    main_objfile = objfile->separate_debug_objfile_backlink;
+  else
+    main_objfile = objfile;
+
+  for (cur_objfile = main_objfile;
+       cur_objfile;
+       cur_objfile = objfile_separate_debug_iterate (main_objfile, cur_objfile))
+    {
+      struct symbol *sym;
+
+      sym = lookup_symbol_aux_objfile (cur_objfile, GLOBAL_BLOCK,
+				       modified_name, domain);
+      if (sym == NULL)
+	sym = lookup_symbol_aux_objfile (cur_objfile, STATIC_BLOCK,
+					 modified_name, domain);
+      if (sym != NULL)
+	{
+	  do_cleanups (cleanup);
+	  return sym;
+	}
+    }
+
+  do_cleanups (cleanup);
+  return NULL;
+}
+
 /* A helper function for lookup_symbol_aux that interfaces with the
    "quick" symbol table functions.  */
 
@@ -3463,21 +3505,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)))
-		  {
-		    /* FIXME: carlton/2003-02-04: Given that the
-		       semantics of lookup_symbol keeps on changing
-		       slightly, it would be a nice idea if we had a
-		       function lookup_symbol_minsym that found the
-		       symbol associated to a given minimal symbol (if
-		       any).  */
-		    if (kind == FUNCTIONS_DOMAIN
-			|| lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-					  (struct block *) NULL,
-					  VAR_DOMAIN, 0)
-			== NULL)
-		      found_misc = 1;
-		  }
+		/* Note: An important side-effect of these lookup functions
+		   is to expand the symbol table if msymbol is found.  */
+		if (kind == FUNCTIONS_DOMAIN
+		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
+		    : lookup_msymbol_in_objfile (objfile, msymbol,
+						 VAR_DOMAIN) == NULL)
+		  found_misc = 1;
 	      }
 	  }
       }
@@ -3554,12 +3588,15 @@ search_symbols (char *regexp, enum searc
   /* If there are no eyes, avoid all contact.  I mean, if there are
      no debug symbols, then print directly from the msymbol_vector.  */
 
-  if (found_misc || kind != FUNCTIONS_DOMAIN)
+  if (found_misc || (nfiles == 0 && kind != FUNCTIONS_DOMAIN))
     {
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
 
+	if (msymbol->created_by_gdb)
+	  continue;
+
 	if (MSYMBOL_TYPE (msymbol) == ourtype
 	    || MSYMBOL_TYPE (msymbol) == ourtype2
 	    || MSYMBOL_TYPE (msymbol) == ourtype3
@@ -3569,14 +3606,13 @@ search_symbols (char *regexp, enum searc
 		|| regexec (&datum.preg, SYMBOL_NATURAL_NAME (msymbol), 0,
 			    NULL, 0) == 0)
 	      {
-		/* Functions:  Look up by address.  */
-		if (kind != FUNCTIONS_DOMAIN ||
-		    (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))))
+		/* For functions we can do a quick check of whether the
+		   symbol might be found via find_pc_symtab.  */
+		if (kind != FUNCTIONS_DOMAIN
+		    || find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL)
 		  {
-		    /* Variables/Absolutes:  Look up by name.  */
-		    if (lookup_symbol (SYMBOL_LINKAGE_NAME (msymbol),
-				       (struct block *) NULL, VAR_DOMAIN, 0)
-			 == NULL)
+		    if (lookup_msymbol_in_objfile (objfile, msymbol,
+						   VAR_DOMAIN) == NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)
Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.206
diff -u -p -r1.206 symtab.h
--- symtab.h	10 May 2012 20:04:00 -0000	1.206
+++ symtab.h	25 May 2012 07:13:42 -0000
@@ -339,6 +339,10 @@ struct minimal_symbol
 
   ENUM_BITFIELD(minimal_symbol_type) type : 8;
 
+  /* Non-zero if this symbol was created by gdb.
+     Such symbols do not appear in the output of "info var|fun".  */
+  unsigned int created_by_gdb : 1;
+
   /* Two flag bits provided for the use of the target.  */
   unsigned int target_flag_1 : 1;
   unsigned int target_flag_2 : 1;