[c++/18141] Fix perf issue

Message ID yjt28ueu6qo0.fsf@ruffy.mtv.corp.google.com
State New, archived
Headers

Commit Message

Doug Evans March 18, 2015, 7:39 p.m. UTC
  Hi.

The bug report explains the issue hopefully sufficiently:

https://sourceware.org/bugzilla/show_bug.cgi?id=18141

Basically, gdb is looking up "std" in .gdb_index, the index saying
it's present, gdb expanding the containing CU, but then the final
lookup not finding it.  This leads to expanding one CU in every
objfile that has an entry for "std".

A followup patch will add a complaint for this case.
In this case it's really an internal error, but I'm not sure
it'll always be an internal error, nor whether one can
adequately distinguish them.

Regression tested on amd64-linux.

Before:

(gdb) print foo
....
Command execution time: 297.609072 (cpu), 761.008252 (wall)
Space used: 25874649088 (+20380094464 for this command)
#symtabs: 622313 (+621693), #compunits: 8782 (+8774), #blocks: 4377936 (+4373024)

After:

(gdb) print foo
....
Command execution time: 0.605912 (cpu), 0.971609 (wall)
Space used: 5538639872 (+42561536 for this command)
#symtabs: 924 (+304), #compunits: 10 (+2), #blocks: 8950 (+4038)

2015-03-18  Doug Evans  <dje@google.com>

	PR c++/18141
	* cp-namespace.c (cp_search_static_and_baseclasses): Always look for
	klass in VAR_DOMAIN.
  

Comments

Doug Evans March 30, 2015, 11:42 p.m. UTC | #1
Doug Evans writes:
 > Hi.
 > 
 > The bug report explains the issue hopefully sufficiently:
 > 
 > https://sourceware.org/bugzilla/show_bug.cgi?id=18141
 > 
 > Basically, gdb is looking up "std" in .gdb_index, the index saying
 > it's present, gdb expanding the containing CU, but then the final
 > lookup not finding it.  This leads to expanding one CU in every
 > objfile that has an entry for "std".
 > 
 > A followup patch will add a complaint for this case.
 > In this case it's really an internal error, but I'm not sure
 > it'll always be an internal error, nor whether one can
 > adequately distinguish them.
 > 
 > Regression tested on amd64-linux.
 > 
 > Before:
 > 
 > (gdb) print foo
 > ....
 > Command execution time: 297.609072 (cpu), 761.008252 (wall)
 > Space used: 25874649088 (+20380094464 for this command)
 > #symtabs: 622313 (+621693), #compunits: 8782 (+8774), #blocks: 4377936 (+4373024)
 > 
 > After:
 > 
 > (gdb) print foo
 > ....
 > Command execution time: 0.605912 (cpu), 0.971609 (wall)
 > Space used: 5538639872 (+42561536 for this command)
 > #symtabs: 924 (+304), #compunits: 10 (+2), #blocks: 8950 (+4038)
 > 
 > 2015-03-18  Doug Evans  <dje@google.com>
 > 
 > 	PR c++/18141
 > 	* cp-namespace.c (cp_search_static_and_baseclasses): Always look for
 > 	klass in VAR_DOMAIN.
 > 
 > diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
 > index 4a00cb6..0feeb35 100644
 > --- a/gdb/cp-namespace.c
 > +++ b/gdb/cp-namespace.c
 > @@ -355,8 +355,11 @@ cp_search_static_and_baseclasses (const char *name,
 >    make_cleanup (xfree, nested);
 >  
 >    /* Lookup a class named KLASS.  If none is found, there is nothing
 > -     more that can be done.  */
 > -  klass_sym = lookup_global_symbol (klass, block, domain);
 > +     more that can be done.  KLASS could be a namespace, so always look
 > +     in VAR_DOMAIN.  This works for classes too because of
 > +     symbol_matches_domain (which should be replaced with something else,
 > +     but it's what we have today).  */
 > +  klass_sym = lookup_global_symbol (klass, block, VAR_DOMAIN);
 >    if (klass_sym == NULL)
 >      {
 >        do_cleanups (cleanup);

fyi, committed.
  

Patch

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 4a00cb6..0feeb35 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -355,8 +355,11 @@  cp_search_static_and_baseclasses (const char *name,
   make_cleanup (xfree, nested);
 
   /* Lookup a class named KLASS.  If none is found, there is nothing
-     more that can be done.  */
-  klass_sym = lookup_global_symbol (klass, block, domain);
+     more that can be done.  KLASS could be a namespace, so always look
+     in VAR_DOMAIN.  This works for classes too because of
+     symbol_matches_domain (which should be replaced with something else,
+     but it's what we have today).  */
+  klass_sym = lookup_global_symbol (klass, block, VAR_DOMAIN);
   if (klass_sym == NULL)
     {
       do_cleanups (cleanup);