[COMMITTED] Fix PR symtab/17591

Message ID yjt2a93u61j8.fsf@ruffy.mtv.corp.google.com
State Committed
Headers

Commit Message

Doug Evans Nov. 14, 2014, 12:58 a.m. UTC
  Hi.

This patch fixes pr 17591.
Regression tested on amd64-linux.

The removing of the parens wasn't taking into account
looking up "(anonymous namespace)".

It turns out there can be a significant performance cost.
The lookup would use "" which in the program where this
was found had an entry in .gdb_index with ~1000 CUs,
none of which had (anonymous namespace),
so we were needlessly reading in a *lot* of debug info.

Before:

(gdb) pt obj
type = ...
Command execution time: 50.712058 (cpu), 52.275078 (wall)
Space used: 2379862016 (+2180878336 for this command)
#symtabs: 150433 (+150064), #primary symtabs: 2090 (+2081), #blocks: 774710 (+773450)

After:

(gdb) pt obj
type = ...
Command execution time: 0.850209 (cpu), 0.864219 (wall)
Space used: 232112128 (+33136640 for this command)
#symtabs: 3040 (+2671), #primary symtabs: 107 (+98), #blocks: 22595 (+21335)

2014-11-13  Doug Evans  <dje@google.com>

	PR symtab/17591
	* dwarf2read.c (find_slot_in_mapped_hash): Handle
	"(anonymous namespace)".
  

Comments

Doug Evans Nov. 14, 2014, 1:10 a.m. UTC | #1
On Thu, Nov 13, 2014 at 4:58 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> This patch fixes pr 17591.
> Regression tested on amd64-linux.
>
> The removing of the parens wasn't taking into account
> looking up "(anonymous namespace)".
>
> It turns out there can be a significant performance cost.
> The lookup would use "" which in the program where this
> was found had an entry in .gdb_index with ~1000 CUs,
> none of which had (anonymous namespace),
> so we were needlessly reading in a *lot* of debug info.
>
> Before:
>
> (gdb) pt obj
> type = ...
> Command execution time: 50.712058 (cpu), 52.275078 (wall)
> Space used: 2379862016 (+2180878336 for this command)
> #symtabs: 150433 (+150064), #primary symtabs: 2090 (+2081), #blocks: 774710 (+773450)
>
> After:
>
> (gdb) pt obj
> type = ...
> Command execution time: 0.850209 (cpu), 0.864219 (wall)
> Space used: 232112128 (+33136640 for this command)
> #symtabs: 3040 (+2671), #primary symtabs: 107 (+98), #blocks: 22595 (+21335)
>
> 2014-11-13  Doug Evans  <dje@google.com>
>
>         PR symtab/17591
>         * dwarf2read.c (find_slot_in_mapped_hash): Handle
>         "(anonymous namespace)".
>
> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index ce37adf..0c97447 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -2917,7 +2917,11 @@ find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
>      {
>        /* NAME is already canonical.  Drop any qualifiers as .gdb_index does
>          not contain any.  */
> -      const char *paren = strchr (name, '(');
> +      const char *paren = NULL;
> +
> +      /* Need to handle "(anonymous namespace)".  */
> +      if (*name != '(')
> +       paren = strchr (name, '(');
>
>        if (paren)
>         {

Bleah.  I was focused on the case at hand and wasn't thinking more generally.
I'll submit a better patch tomorrow.
  

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index ce37adf..0c97447 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2917,7 +2917,11 @@  find_slot_in_mapped_hash (struct mapped_index *index, const char *name,
     {
       /* NAME is already canonical.  Drop any qualifiers as .gdb_index does
 	 not contain any.  */
-      const char *paren = strchr (name, '(');
+      const char *paren = NULL;
+
+      /* Need to handle "(anonymous namespace)".  */
+      if (*name != '(')
+	paren = strchr (name, '(');
 
       if (paren)
 	{