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

[Matt Rice <ratmice@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1383 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.

FWIW i don't have anything with enough objfiles for the above to matter,
but here's something that can apply on top of your patch which
helps somewhat for objfiles with lots of symtabs, it still contains
the same worst case,
but helps in the 'info var' case with no arguments in e.g. ./gdb ./gdb
-batch -ex 'info var' case
where there is a lot of symtabs per objfile, and many symbols returned
by info var.

it could be faster if we had a way to know if a msymbol has no symbol
associated with it,
but this was as good as I could get without any major refactoring,
though maybe it is a little too ad-hoc.

anyhow if you don't mind having a look/testing it out.

it does come with a change of behaviour (IMO bugfix)
in that it compares the symbol/msymbol addresses,
this is for the case of ambiguous variable names, where previously it
would not output a msymbol if it found a symbol of the same name.

anyhow it speeds up the aforementioned ./gdb ./gdb -batch -ex 'info
var' case a bit.
though i doubt it will affect 'info var foo' much if at all.

thanks

[-- Attachment #2: foo.diff --]
[-- Type: application/octet-stream, Size: 5453 bytes --]

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 868bcf2..dd5e73f 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1559,26 +1559,132 @@ lookup_symbol_aux_symtabs (int block_index, const char *name,
   return NULL;
 }
 
+/* Helper for lookup_symbol_msymbol_blockvector */
+struct symbol *
+lookup_block_symbol_for_msymbol (const struct block *block,
+                                 struct minimal_symbol *msym,
+				 const char *name);
+struct symbol *
+lookup_block_symbol_for_msymbol (const struct block *block,
+                                 struct minimal_symbol *msym,
+				 const char *name)
+{
+  struct dict_iterator iter;
+  struct symbol *sym;
+
+  for (sym = dict_iter_name_first (BLOCK_DICT (block),
+                                   name, &iter);
+       sym != NULL;
+       sym = dict_iter_name_next (name, &iter))
+    {
+      if (SYMBOL_VALUE_ADDRESS (msym) == SYMBOL_VALUE_ADDRESS (sym))
+	{
+	  return sym;
+	}
+    }
+  return NULL;
+}
+
+struct symbol *
+lookup_symbol_msymbol_block_vector (const char *name,
+				    struct minimal_symbol *msym,
+                                    struct blockvector *bv);
+
+struct symbol *
+lookup_symbol_msymbol_block_vector (const char *name,
+				    struct minimal_symbol *msym,
+                                    struct blockvector *bv)
+{
+  struct block *block;
+  struct symbol *sym = NULL;
+
+  block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK);
+  sym = lookup_block_symbol_for_msymbol (block, msym, name);
+  if (sym)
+    return sym;
+
+  block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK);
+  sym = lookup_block_symbol_for_msymbol (block, msym, name);
+
+  return sym;
+}
+
+struct symtab_cache {
+  struct objfile *last_objfile;
+  struct symtab *last_symtab;
+};
+
+void symtab_cache_init (struct symtab_cache *cache);
+void symtab_cache_init (struct symtab_cache *cache)
+{
+  cache->last_objfile = NULL;
+  cache->last_symtab = NULL;
+}
+
 /* Wrapper around lookup_symbol_aux_objfile for search_symbols.
    Look up MSYMBOL in DOMAIN in the global and static blocks of OBJFILE.  */
 
 static struct symbol *
 lookup_msymbol_in_objfile (struct objfile *objfile,
 			   struct minimal_symbol *msymbol,
-			   domain_enum domain)
+			   domain_enum domain,
+			   struct symtab_cache *cache)
 {
   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 symbol *returnval;
+  struct cleanup *cleanup;
+  struct symbol *returnval = NULL;
+  struct symtab *current_symtab;
+  struct symtab *orig_symtab;
+  struct symtab *next_symtab;
+  struct symtab *stop_at_this_symtab;
+
+  /* FIXME: can this kill the symtab expansion side-effect? */
+  if (!objfile_has_symbols (objfile))
+    return NULL;
+
+  cleanup = demangle_for_lookup (name, lang, &modified_name);
+
+  if (cache->last_objfile != objfile)
+    current_symtab = objfile->symtabs;
+  else
+    current_symtab = cache->last_symtab;
+
+  orig_symtab = current_symtab;
+  stop_at_this_symtab = NULL;
+  next_symtab = objfile->symtabs;
+
+  while (current_symtab != stop_at_this_symtab)
+    {
+      if (current_symtab->primary)
+        {
+          struct blockvector *bv = BLOCKVECTOR (current_symtab);
 
-  returnval = lookup_symbol_aux_objfile (objfile, GLOBAL_BLOCK,
-					 modified_name, domain);
-  if (returnval == NULL)
-    returnval = lookup_symbol_aux_objfile (objfile, STATIC_BLOCK,
-					   modified_name, domain);
+          returnval = lookup_symbol_msymbol_block_vector (name, msymbol, bv);
 
+          if (returnval)
+            {
+              if (cache->last_objfile != objfile)
+		cache->last_objfile = objfile;
+
+	      if (cache->last_symtab != current_symtab)
+		cache->last_symtab = current_symtab;
+
+	      goto ret_sym;
+	    }
+	}
+      if (current_symtab->next == NULL)
+	{
+	  current_symtab = next_symtab;
+	  stop_at_this_symtab = orig_symtab;
+	  next_symtab = orig_symtab;
+	}
+      else
+        current_symtab = current_symtab->next;
+    }
+
+ ret_sym:
   do_cleanups (cleanup);
   return returnval;
 }
@@ -3474,6 +3580,10 @@ search_symbols (char *regexp, enum search_domain kind,
 
   if (nfiles == 0 && (kind == VARIABLES_DOMAIN || kind == FUNCTIONS_DOMAIN))
     {
+      struct symtab_cache st_cache;
+
+      symtab_cache_init(&st_cache);
+
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
@@ -3492,7 +3602,8 @@ search_symbols (char *regexp, enum search_domain kind,
 		if (kind == FUNCTIONS_DOMAIN
 		    ? find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol)) == NULL
 		    : lookup_msymbol_in_objfile (objfile, msymbol,
-						 VAR_DOMAIN) == NULL)
+						 VAR_DOMAIN,
+						 &st_cache) == NULL)
 		  found_misc = 1;
 	      }
 	  }
@@ -3572,6 +3683,10 @@ search_symbols (char *regexp, enum search_domain kind,
 
   if (found_misc || kind != FUNCTIONS_DOMAIN)
     {
+      struct symtab_cache st_cache;
+
+      symtab_cache_init(&st_cache);
+
       ALL_MSYMBOLS (objfile, msymbol)
       {
         QUIT;
@@ -3592,7 +3707,8 @@ search_symbols (char *regexp, enum search_domain kind,
 		    /* Variables/Absolutes:  Look up by name.  */
 		    /* FIXME: Why do we also look up fns by name?  */
 		    if (lookup_msymbol_in_objfile (objfile, msymbol,
-						   VAR_DOMAIN) == NULL)
+						   VAR_DOMAIN,
+						   &st_cache) == NULL)
 		      {
 			/* match */
 			psr = (struct symbol_search *)