Re: [RFA/dwarf] Don't process types multiple times

[Jim Blandy <jimb@redhat.com>]

Daniel Jacobowitz <drow@false.org> writes:
> This independent patch is a performance and correctness fix for full symbol
> processing.  My description from the intercu branch posting:
> 
>  There are two ways that we can process a general DIE: process_die or
>  read_type_die.  Children of particular DIE types may be processed directly,
>  but these are the only major dispatch points.  It's interesting to notice
>  that almost everything called from read_type_die starts with "if (die->type)
>  return": everything but enumeration types and aggregate types, in fact.
>  This means that if the first reference to an enumeration or aggregate type
>  is a DW_AT_type or DW_AT_containing_type in a DIE numerically before the
>  type's DIE, we'll end up calling new_symbol for it twice.
> 
>  Fixing this saves about 8% memory and 4% time from gdb -readnow libc.so.6,
>  a lot of duplicate entries on maint print symbols which I vaguely recall
>  being confused about but never investigated, and some serious problems for
>  inter-CU support.  Without this, types could be added to the wrong symbol
>  table.
> 
> Tested i686-pc-linux-gnu, no regressions.  OK to commit?

So the essential idea here is to make sure that we only create symbols
when we reach them via the main traversal, not when we happen to reach
them by following references with die_type.  The current code relies
on the existing read_structure_scope and read_enumeration functions to
both read the type and create the symbol, so it can't distinguish
between the two cases.  Your patch creates a separate function for
each role, and ensures that the symbol creation functions are only
called from the main traversal.

Is that right?  If so, please commit, to trunk and branch.

One suggestion: for the aggregate types, it seems like we're using
"read_structure_scope" to mean "create the type object, but don't call
new_symbol", and "process_structure_scope" to mean "go ahead and
create the new symbols".  But for the enumerations, the two
corresponding functions are called "read_enumeration_type" and
"read_enumeration_scope".  Their functions are parallel, but the names
don't reflect it.  So how about:


                 don't make sym                       make sym

struct:        read_structure_scope             process_structure_scope
enum:          read_enumeration_scope           process_enumeration_scope

or:

struct:        read_structure_type              process_structure_scope
enum:          read_enumeration_type            process_enumeration_scope