Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-18 Thread Tristan Gingold

On Oct 17, 2011, at 3:16 PM, Tom Tromey wrote:

 Tristan == Tristan Gingold ging...@adacore.com writes:
 
 Tom Another way to look at it is that there have been many changes to GCC's
 Tom DWARF output in the last few years.  Surely these have broken these
 Tom DWARF consumers more than this change possibly could.
 
 Tristan Yes, but there is -gstrict-dwarf to stay compatible with old 
 behavior.
 
 Yes, but this change is strictly compliant.

Agreed.

  What makes it different
 from any other change that was made to make GCC more compliant?
 Hypothetical consumers could also break on those changes.

The others changes were often corner cases, while this one is very visible.

What is wrong with my suggestion of adding a command line option to keep the 
siblings ?  This option could be removed in a few years if nobody complained 
about sibling removal.

Tristan.



Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-17 Thread Tom Tromey
 Tristan == Tristan Gingold ging...@adacore.com writes:

Tom Another way to look at it is that there have been many changes to GCC's
Tom DWARF output in the last few years.  Surely these have broken these
Tom DWARF consumers more than this change possibly could.

Tristan Yes, but there is -gstrict-dwarf to stay compatible with old behavior.

Yes, but this change is strictly compliant.  What makes it different
from any other change that was made to make GCC more compliant?
Hypothetical consumers could also break on those changes.

Tom


Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-14 Thread Tristan Gingold

On Oct 13, 2011, at 10:40 PM, Jan Kratochvil wrote:

 On Wed, 12 Oct 2011 16:18:07 +0200, Jan Kratochvil wrote:
 On Wed, 12 Oct 2011 16:07:24 +0200, Tristan Gingold wrote:
 I fear that this may degrade performance of other debuggers.  What about
 adding a command line option ?
 
 I can test idb,
 
 I do not find the difference measurable.  Dropping DW_AT_sibling is 0.25%
 performance _improvement_ but I guess it is just less than the measurement
 error.
 
 libstdc++ built with gcc -gdwarf-2 as with gcc -gdwarf-4 -fdebug-types-section
 it crashes.  i7-920 x86_64 used for testing:
 Intel(R) Debugger for applications running on Intel(R) 64, Version 12.1, 
 Build [76.472.14]
 
 with DW_AT_sibling
 real2m34.206s 2m31.822s 2m31.709s 2m32.316s
 avg = 152.51325 seconds
 
 patched GCC without DW_AT_sibling
 real2m32.528s 2m30.524s 2m33.767s 2m31.719s
 avg = 152.1345 seconds
 
 I do not see a point in keeping DW_AT_sibling there.

I am not against this patch, my only concern is that there are many many dwarf 
consumers and I have no idea how they will react to this change.

Tristan.


Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-14 Thread Tom Tromey
 Tristan == Tristan Gingold ging...@adacore.com writes:

Tristan I am not against this patch, my only concern is that there are many
Tristan many dwarf consumers and I have no idea how they will react to this
Tristan change.

I tend to think that this is the wrong standard to apply.  In this case
we would be avoiding a beneficial change -- as measured in both
performance in a couple of cases, and in size -- for the sake of unknown
and possibly nonexistent consumers.  I think instead the burden of proof
should be on those consumers, both to give their evidence and reasoning
and to engage with GCC.

Another way to look at it is that there have been many changes to GCC's
DWARF output in the last few years.  Surely these have broken these
DWARF consumers more than this change possibly could.

Tom


Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-13 Thread Jan Kratochvil
On Wed, 12 Oct 2011 16:18:07 +0200, Jan Kratochvil wrote:
 On Wed, 12 Oct 2011 16:07:24 +0200, Tristan Gingold wrote:
  I fear that this may degrade performance of other debuggers.  What about
  adding a command line option ?
 
 I can test idb,

I do not find the difference measurable.  Dropping DW_AT_sibling is 0.25%
performance _improvement_ but I guess it is just less than the measurement
error.

libstdc++ built with gcc -gdwarf-2 as with gcc -gdwarf-4 -fdebug-types-section
it crashes.  i7-920 x86_64 used for testing:
Intel(R) Debugger for applications running on Intel(R) 64, Version 12.1, Build 
[76.472.14]

with DW_AT_sibling
real2m34.206s 2m31.822s 2m31.709s 2m32.316s
avg = 152.51325 seconds

patched GCC without DW_AT_sibling
real2m32.528s 2m30.524s 2m33.767s 2m31.719s
avg = 152.1345 seconds

I do not see a point in keeping DW_AT_sibling there.


Regards,
Jan


[patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-12 Thread Jan Kratochvil
Hi,

dropping the optional DWARF attribute DW_AT_sibling has only advantages and no
disadvantages:

For files with .gdb_index GDB initial scan does not use DW_AT_sibling at all.
For files without .gdb_index GDB initial scan has 1.79% time _improvement_.
For .debug files it brings 3.49% size decrease (7.84% for rpm compressed files).

I guess DW_AT_sibling had real performance gains on CPUs with 1x (=no) clock
multipliers.  Nowadays mostly only the data size transferred over FSB matters.

I do not think there would be any DWARF consumers compatibility problems as
DW_AT_sibling has always been optional but I admit I have tested only GDB.


clean is FSF GCC+GDB, ns is FSF GCC with the patch applied.

gdbindex -readnow 100x warm:
clean:
56.975 57.161 57.738 58.243 57.529249 seconds
ns:
57.799 58.008 58.202 58.473 58.1204993 seconds
+1.03% = performance decrease but it should be 0%, it is a measurement error
gdbindex -readnow 20x warm(gdb) cold(data):
clean:
57.989
ns:
58.538
+0.95% = performance decrease but it should be 0%, it is a measurement error
200x warm:
clean:
14.393 14.414 14.587 14.496 14.4724998 seconds
ns:
14.202 14.160 14.174 14.318 14.2134998 seconds
-1.79% = performance improvement of non-gdbindex scan 
(dwarf2_build_psymtabs_hard)

gdbindex .debug:
clean = 5589272 bytes
ns = 5394120 bytes
-3.49% = size improvement

gdbindex .debug.xz9:
clean = 1158696 bytes
ns = 1067900 bytes
-7.84% = size improvement

.debug_info + .debug_types:
clean = 0x1a11a0+0x08f389 bytes
ns = 0x184205+0x0833b0 bytes
-7.31% = size improvement

Intel i7-920 CPU and only libstdc++ from GCC 4.7.0 20111002 and `-O2 -gdwarf-4
-fdebug-types-section' were used for the benchmark.  GCC 4.7.0 20111002
--enable-languages=c++ was used for `make check' regression testing.


Thanks,
Jan


gcc/
2011-10-12  Jan Kratochvil  jan.kratoch...@redhat.com

Stop producing DW_AT_sibling.
* dwarf2out.c (add_sibling_attributes): Remove the declaration.
(add_sibling_attributes): Remove the function.
(dwarf2out_finish): Remove calls of add_sibling_attributes.

--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -3316,7 +3316,6 @@ static int htab_cu_eq (const void *, const void *);
 static void htab_cu_del (void *);
 static int check_duplicate_cu (dw_die_ref, htab_t, unsigned *);
 static void record_comdat_symbol_number (dw_die_ref, htab_t, unsigned);
-static void add_sibling_attributes (dw_die_ref);
 static void build_abbrev_table (dw_die_ref);
 static void output_location_lists (dw_die_ref);
 static int constant_size (unsigned HOST_WIDE_INT);
@@ -7482,24 +7481,6 @@ copy_decls_for_unworthy_types (dw_die_ref unit)
   unmark_dies (unit);
 }
 
-/* Traverse the DIE and add a sibling attribute if it may have the
-   effect of speeding up access to siblings.  To save some space,
-   avoid generating sibling attributes for DIE's without children.  */
-
-static void
-add_sibling_attributes (dw_die_ref die)
-{
-  dw_die_ref c;
-
-  if (! die-die_child)
-return;
-
-  if (die-die_parent  die != die-die_parent-die_child)
-add_AT_die_ref (die, DW_AT_sibling, die-die_sib);
-
-  FOR_EACH_CHILD (die, c, add_sibling_attributes (c));
-}
-
 /* Output all location lists for the DIE and its children.  */
 
 static void
@@ -22496,14 +22477,6 @@ dwarf2out_finish (const char *filename)
   prune_unused_types ();
 }
 
-  /* Traverse the DIE's and add add sibling attributes to those DIE's
- that have children.  */
-  add_sibling_attributes (comp_unit_die ());
-  for (node = limbo_die_list; node; node = node-next)
-add_sibling_attributes (node-die);
-  for (ctnode = comdat_type_list; ctnode != NULL; ctnode = ctnode-next)
-add_sibling_attributes (ctnode-root_die);
-
   /* Output a terminator label for the .text section.  */
   switch_to_section (text_section);
   targetm.asm_out.internal_label (asm_out_file, TEXT_END_LABEL, 0);


Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-12 Thread Tristan Gingold

On Oct 12, 2011, at 3:50 PM, Jan Kratochvil wrote:

 Hi,
 
 dropping the optional DWARF attribute DW_AT_sibling has only advantages and no
 disadvantages:
 
 For files with .gdb_index GDB initial scan does not use DW_AT_sibling at all.
 For files without .gdb_index GDB initial scan has 1.79% time _improvement_.
 For .debug files it brings 3.49% size decrease (7.84% for rpm compressed 
 files).
 
 I guess DW_AT_sibling had real performance gains on CPUs with 1x (=no) clock
 multipliers.  Nowadays mostly only the data size transferred over FSB matters.
 
 I do not think there would be any DWARF consumers compatibility problems as
 DW_AT_sibling has always been optional but I admit I have tested only GDB.

I fear that this may degrade performance of other debuggers.  What about adding 
a command line option ?

Tristan.



Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling

2011-10-12 Thread Jan Kratochvil
On Wed, 12 Oct 2011 16:07:24 +0200, Tristan Gingold wrote:
 I fear that this may degrade performance of other debuggers.  What about
 adding a command line option ?

I can test idb, there aren't so many DWARF debuggers out there I think.

If the default is removed DW_AT_sibling a new options may make sense as some
compatibility safeguard.


Thanks,
Jan