Re: [patch] dwarf2out: Drop the size + performance overhead of DW_AT_sibling
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
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
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
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
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
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
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
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