* [PATCH 0/4] Micro-optimize DWARF partial symbol reading @ 2020-05-20 17:40 Tom Tromey 2020-05-20 17:40 ` [PATCH 1/4] Attribute method inlining Tom Tromey ` (6 more replies) 0 siblings, 7 replies; 16+ messages in thread From: Tom Tromey @ 2020-05-20 17:40 UTC (permalink / raw) To: gdb-patches A personal goal of mine is to improve the startup time of gdb. In the long run, I think the answer lies partly with threading, and perhaps with a more radical rewrite of the DWARF psymbol reader. However, those are difficult goals; and in the short term, I found that just profiling the reader and making small improvements can make a difference. This series improves the performance of the DWARF partial symbol reader about 10% (more in one case) on some real-world executables. See the first patch for details (I chose to put the details there so they would end up in the eventual git log). Regression tested on x86-64 Fedora 30. Let me know what you think. Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/4] Attribute method inlining 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey @ 2020-05-20 17:40 ` Tom Tromey 2020-05-20 19:40 ` Tom Tromey 2020-05-21 1:08 ` Hannes Domani 2020-05-20 17:40 ` [PATCH 2/4] Lazily compute partial DIE name Tom Tromey ` (5 subsequent siblings) 6 siblings, 2 replies; 16+ messages in thread From: Tom Tromey @ 2020-05-20 17:40 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey This inlines a couple of methods on struct attribute, improving the performance of DWARF partial symbol reading. These methods were discovered as hot spots using callgrind. For this patch, and for all the patches in this series, I tested gdb's performance on three programs: 1. gdb itself -- I built gdb and copied it to /tmp, ensuring that the same version was used in all tests. 2. The system libxul.so, the main library of Firefox. I installed the separate debuginfo and ensured that gdb read it. 3. A large-ish Ada program that I happen to have. I ran gdb 10 times like: /bin/time -f %e \ ./gdb/gdb --data-directory ./gdb/data-directory -nx \ -iex 'set debug-file-directory /usr/lib/debug' \ -batch $X ... where $X was the test executable. Then I computed the mean time. This was all done with a standard (-g -O2) build of gdb. The baseline times were gdb 1.90 libxul 2.12 Ada 2.61 This patch brings the numbers down to gdb 1.88 libxul 2.11 Ada 2.60 Not a huge change, but still visible in the results. gdb/ChangeLog 2020-05-20 Tom Tromey <tromey@adacore.com> * dwarf2/attribute.h (struct attribute) <form_is_ref>: Inline. <get_ref_die_offset>: Inline. <get_ref_die_offset_complaint>: New method. * dwarf2/attribute.c (attribute::form_is_ref): Move to header. (attribute::get_ref_die_offset_complaint): Rename from get_ref_die_offset. Just issue complaint. --- gdb/ChangeLog | 9 +++++++++ gdb/dwarf2/attribute.c | 29 ++--------------------------- gdb/dwarf2/attribute.h | 25 +++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 29 deletions(-) diff --git a/gdb/dwarf2/attribute.c b/gdb/dwarf2/attribute.c index 9f808aa7904..b39cfe2c00d 100644 --- a/gdb/dwarf2/attribute.c +++ b/gdb/dwarf2/attribute.c @@ -120,38 +120,13 @@ attribute::form_is_constant () const } } -/* DW_ADDR is always stored already as sect_offset; despite for the forms - besides DW_FORM_ref_addr it is stored as cu_offset in the DWARF file. */ - -bool -attribute::form_is_ref () const -{ - switch (form) - { - case DW_FORM_ref_addr: - case DW_FORM_ref1: - case DW_FORM_ref2: - case DW_FORM_ref4: - case DW_FORM_ref8: - case DW_FORM_ref_udata: - case DW_FORM_GNU_ref_alt: - return true; - default: - return false; - } -} - /* See attribute.h. */ -sect_offset -attribute::get_ref_die_offset () const +void +attribute::get_ref_die_offset_complaint () const { - if (form_is_ref ()) - return (sect_offset) DW_UNSND (this); - complaint (_("unsupported die ref attribute form: '%s'"), dwarf_form_name (form)); - return {}; } /* See attribute.h. */ diff --git a/gdb/dwarf2/attribute.h b/gdb/dwarf2/attribute.h index a9cabd69f9f..ffb91e878f1 100644 --- a/gdb/dwarf2/attribute.h +++ b/gdb/dwarf2/attribute.h @@ -82,7 +82,16 @@ struct attribute /* DW_ADDR is always stored already as sect_offset; despite for the forms besides DW_FORM_ref_addr it is stored as cu_offset in the DWARF file. */ - bool form_is_ref () const; + bool form_is_ref () const + { + return (form == DW_FORM_ref_addr + || form == DW_FORM_ref1 + || form == DW_FORM_ref2 + || form == DW_FORM_ref4 + || form == DW_FORM_ref8 + || form == DW_FORM_ref_udata + || form == DW_FORM_GNU_ref_alt); + } /* Check if the attribute's form is a DW_FORM_block* if so return true else false. */ @@ -92,7 +101,13 @@ struct attribute /* Return DIE offset of this attribute. Return 0 with complaint if the attribute is not of the required kind. */ - sect_offset get_ref_die_offset () const; + sect_offset get_ref_die_offset () const + { + if (form_is_ref ()) + return (sect_offset) u.unsnd; + get_ref_die_offset_complaint (); + return {}; + } /* Return the constant value held by this attribute. Return DEFAULT_VALUE if the value held by the attribute is not @@ -119,6 +134,12 @@ struct attribute ULONGEST signature; } u; + +private: + + /* Used by get_ref_die_offset to issue a complaint. */ + + void get_ref_die_offset_complaint () const; }; /* Get at parts of an attribute structure. */ -- 2.21.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-20 17:40 ` [PATCH 1/4] Attribute method inlining Tom Tromey @ 2020-05-20 19:40 ` Tom Tromey 2020-05-21 1:08 ` Hannes Domani 1 sibling, 0 replies; 16+ messages in thread From: Tom Tromey @ 2020-05-20 19:40 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches >>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes: Tom> 2. The system libxul.so, the main library of Firefox. I installed the Tom> separate debuginfo and ensured that gdb read it. I forgot that this has a .gdb_index, oops :(. Any improvements seen here probably come from making CU expansion a bit faster. Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-20 17:40 ` [PATCH 1/4] Attribute method inlining Tom Tromey 2020-05-20 19:40 ` Tom Tromey @ 2020-05-21 1:08 ` Hannes Domani 2020-05-21 14:26 ` Pedro Alves 1 sibling, 1 reply; 16+ messages in thread From: Hannes Domani @ 2020-05-21 1:08 UTC (permalink / raw) To: Gdb-patches Am Mittwoch, 20. Mai 2020, 19:40:44 MESZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben: > I ran gdb 10 times like: > > /bin/time -f %e \ > ./gdb/gdb --data-directory ./gdb/data-directory -nx \ > -iex 'set debug-file-directory /usr/lib/debug' \ > -batch $X > > ... where $X was the test executable. Then I computed the mean time. > This was all done with a standard (-g -O2) build of gdb. > > The baseline times were > > gdb 1.90 > libxul 2.12 > Ada 2.61 > > This patch brings the numbers down to > > gdb 1.88 > libxul 2.11 > Ada 2.60 When I saw this, I thought I could do a similar profiling test on Windows (but only with gdb itself). So just: gdb.exe -q -batch gdb.exe And I was a bit suprised to see that strcmp_iw_ordered (called from sort_pst_symbols -> std::sort) is in ~24% of the profiling samples. And only because of the functions isspace and tolower. So I made a simple test, and added this before strcmp_iw_ordered: static inline int isspace2 (int c) { return c == 0x20 || (c >= 0x09 && c <= 0x0d); } #define isspace isspace2 static inline int tolower2 (int c) { return (c >= 'A' && c <= 'Z') ? c + 0x20 : c; } #define tolower tolower2 And the mean time went from 3.7s down to 2.7s. I'm not saying this is a correct solution, but does strcmp_iw_ordered have to support anything besides the "C" locale? Also, are isspace and tolower only on Windows a bottleneck? (If anyone wants to see them, I can provide some profiler flame-graphs) Hannes ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-21 1:08 ` Hannes Domani @ 2020-05-21 14:26 ` Pedro Alves 2020-05-21 15:03 ` Hannes Domani 0 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2020-05-21 14:26 UTC (permalink / raw) To: Hannes Domani, Gdb-patches On 5/21/20 2:08 AM, Hannes Domani via Gdb-patches wrote: > Am Mittwoch, 20. Mai 2020, 19:40:44 MESZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben: > >> I ran gdb 10 times like: >> >> /bin/time -f %e \ >> ./gdb/gdb --data-directory ./gdb/data-directory -nx \ >> -iex 'set debug-file-directory /usr/lib/debug' \ >> -batch $X >> >> ... where $X was the test executable. Then I computed the mean time. >> This was all done with a standard (-g -O2) build of gdb. >> >> The baseline times were >> >> gdb 1.90 >> libxul 2.12 >> Ada 2.61 >> >> This patch brings the numbers down to >> >> gdb 1.88 >> libxul 2.11 >> Ada 2.60 > > When I saw this, I thought I could do a similar profiling test on Windows (but > only with gdb itself). > > So just: gdb.exe -q -batch gdb.exe > > And I was a bit suprised to see that strcmp_iw_ordered (called from > sort_pst_symbols -> std::sort) is in ~24% of the profiling samples. > And only because of the functions isspace and tolower. > > So I made a simple test, and added this before strcmp_iw_ordered: > > static inline int isspace2 (int c) > { > return c == 0x20 || (c >= 0x09 && c <= 0x0d); > } > #define isspace isspace2 > > static inline int tolower2 (int c) > { > return (c >= 'A' && c <= 'Z') ? c + 0x20 : c; > } > #define tolower tolower2 > > And the mean time went from 3.7s down to 2.7s. > > > I'm not saying this is a correct solution, but does strcmp_iw_ordered have to > support anything besides the "C" locale? > > Also, are isspace and tolower only on Windows a bottleneck? > > (If anyone wants to see them, I can provide some profiler flame-graphs) There was a patch for this not that long ago. Let me try to dig it up. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-21 14:26 ` Pedro Alves @ 2020-05-21 15:03 ` Hannes Domani 2020-05-21 16:42 ` Pedro Alves 0 siblings, 1 reply; 16+ messages in thread From: Hannes Domani @ 2020-05-21 15:03 UTC (permalink / raw) To: Gdb-patches Am Donnerstag, 21. Mai 2020, 16:26:12 MESZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: > On 5/21/20 2:08 AM, Hannes Domani via Gdb-patches wrote: > > > Am Mittwoch, 20. Mai 2020, 19:40:44 MESZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben: > > > >> I ran gdb 10 times like: > >> > >> /bin/time -f %e \ > >> ./gdb/gdb --data-directory ./gdb/data-directory -nx \ > >> -iex 'set debug-file-directory /usr/lib/debug' \ > >> -batch $X > >> > >> ... where $X was the test executable. Then I computed the mean time. > >> This was all done with a standard (-g -O2) build of gdb. > >> > >> The baseline times were > >> > >> gdb 1.90 > >> libxul 2.12 > >> Ada 2.61 > >> > >> This patch brings the numbers down to > >> > >> gdb 1.88 > >> libxul 2.11 > >> Ada 2.60 > > > > When I saw this, I thought I could do a similar profiling test on Windows (but > > only with gdb itself). > > > > So just: gdb.exe -q -batch gdb.exe > > > > And I was a bit suprised to see that strcmp_iw_ordered (called from > > sort_pst_symbols -> std::sort) is in ~24% of the profiling samples. > > And only because of the functions isspace and tolower. > > > > So I made a simple test, and added this before strcmp_iw_ordered: > > > > static inline int isspace2 (int c) > > { > > return c == 0x20 || (c >= 0x09 && c <= 0x0d); > > } > > #define isspace isspace2 > > > > static inline int tolower2 (int c) > > { > > return (c >= 'A' && c <= 'Z') ? c + 0x20 : c; > > } > > #define tolower tolower2 > > > > And the mean time went from 3.7s down to 2.7s. > > > > > > I'm not saying this is a correct solution, but does strcmp_iw_ordered have to > > support anything besides the "C" locale? > > > > Also, are isspace and tolower only on Windows a bottleneck? > > > > (If anyone wants to see them, I can provide some profiler flame-graphs) > > > There was a patch for this not that long ago. Let me try to dig it up. You're right, I found it here: https://sourceware.org/pipermail/gdb-patches/2019-June/158285.html So I guess it's not just on Windows that slow. And you replied that we maybe should use TOLOWER, ISXDIGIT from libiberty instead: https://sourceware.org/pipermail/gdb-patches/2019-June/158518.html I tried to do that, but the safe-ctype.h defines clash with the ones of chardefs.h: In file included from C:/src/repos/binutils-gdb.git/gdb/utils.c:62: c:\src\repos\binutils-gdb.git\include\safe-ctype.h:89: error: "ISALPHA" redefined [-Werror] 89 | #define ISALPHA(c) _sch_test(c, _sch_isalpha) | In file included from c:\src\repos\binutils-gdb.git\readline\readline\keymaps.h:35, from c:\src\repos\binutils-gdb.git\readline\readline\readline.h:37, from C:/src/repos/binutils-gdb.git/gdb/utils.c:61: c:\src\repos\binutils-gdb.git\readline\readline\chardefs.h:91: note: this is the location of the previous definition 91 | #define ISALPHA(c) (IN_CTYPE_DOMAIN (c) && isalpha ((unsigned char)c)) | Not sure how to solve this. Hannes ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-21 15:03 ` Hannes Domani @ 2020-05-21 16:42 ` Pedro Alves 2020-05-21 17:18 ` Hannes Domani 0 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2020-05-21 16:42 UTC (permalink / raw) To: Hannes Domani, Gdb-patches On 5/21/20 4:03 PM, Hannes Domani via Gdb-patches wrote: > Am Donnerstag, 21. Mai 2020, 16:26:12 MESZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: > >> On 5/21/20 2:08 AM, Hannes Domani via Gdb-patches wrote: >> >>> Am Mittwoch, 20. Mai 2020, 19:40:44 MESZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben: >>> >>>> I ran gdb 10 times like: >>>> >>>> /bin/time -f %e \ >>>> ./gdb/gdb --data-directory ./gdb/data-directory -nx \ >>>> -iex 'set debug-file-directory /usr/lib/debug' \ >>>> -batch $X >>>> >>>> ... where $X was the test executable. Then I computed the mean time. >>>> This was all done with a standard (-g -O2) build of gdb. >>>> >>>> The baseline times were >>>> >>>> gdb 1.90 >>>> libxul 2.12 >>>> Ada 2.61 >>>> >>>> This patch brings the numbers down to >>>> >>>> gdb 1.88 >>>> libxul 2.11 >>>> Ada 2.60 >>> >>> When I saw this, I thought I could do a similar profiling test on Windows (but >>> only with gdb itself). >>> >>> So just: gdb.exe -q -batch gdb.exe >>> >>> And I was a bit suprised to see that strcmp_iw_ordered (called from >>> sort_pst_symbols -> std::sort) is in ~24% of the profiling samples. >>> And only because of the functions isspace and tolower. >>> >>> So I made a simple test, and added this before strcmp_iw_ordered: >>> >>> static inline int isspace2 (int c) >>> { >>> return c == 0x20 || (c >= 0x09 && c <= 0x0d); >>> } >>> #define isspace isspace2 >>> >>> static inline int tolower2 (int c) >>> { >>> return (c >= 'A' && c <= 'Z') ? c + 0x20 : c; >>> } >>> #define tolower tolower2 >>> >>> And the mean time went from 3.7s down to 2.7s. >>> >>> >>> I'm not saying this is a correct solution, but does strcmp_iw_ordered have to >>> support anything besides the "C" locale? >>> >>> Also, are isspace and tolower only on Windows a bottleneck? >>> >>> (If anyone wants to see them, I can provide some profiler flame-graphs) >> >> >> There was a patch for this not that long ago. Let me try to dig it up. > > You're right, I found it here: > https://sourceware.org/pipermail/gdb-patches/2019-June/158285.html Yes, that's the one! > > So I guess it's not just on Windows that slow. > > And you replied that we maybe should use TOLOWER, ISXDIGIT from libiberty > instead: > https://sourceware.org/pipermail/gdb-patches/2019-June/158518.html This message is actually older than the patch above -- I wrote the patch afterwards. The patch is using the libiberty macros, and avoids the readline clash you run into. Could you give it a try? Thanks, Pedro Alves > > I tried to do that, but the safe-ctype.h defines clash with the ones > of chardefs.h: > > In file included from C:/src/repos/binutils-gdb.git/gdb/utils.c:62: > c:\src\repos\binutils-gdb.git\include\safe-ctype.h:89: error: "ISALPHA" redefined [-Werror] > 89 | #define ISALPHA(c) _sch_test(c, _sch_isalpha) > | > In file included from c:\src\repos\binutils-gdb.git\readline\readline\keymaps.h:35, > from c:\src\repos\binutils-gdb.git\readline\readline\readline.h:37, > from C:/src/repos/binutils-gdb.git/gdb/utils.c:61: > c:\src\repos\binutils-gdb.git\readline\readline\chardefs.h:91: note: this is the location of the previous definition > 91 | #define ISALPHA(c) (IN_CTYPE_DOMAIN (c) && isalpha ((unsigned char)c)) > | > > Not sure how to solve this. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-21 16:42 ` Pedro Alves @ 2020-05-21 17:18 ` Hannes Domani 2020-05-22 15:47 ` Pedro Alves 0 siblings, 1 reply; 16+ messages in thread From: Hannes Domani @ 2020-05-21 17:18 UTC (permalink / raw) To: Gdb-patches Am Donnerstag, 21. Mai 2020, 18:42:23 MESZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: > On 5/21/20 4:03 PM, Hannes Domani via Gdb-patches wrote: > > Am Donnerstag, 21. Mai 2020, 16:26:12 MESZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: > > > >> On 5/21/20 2:08 AM, Hannes Domani via Gdb-patches wrote: > >> > >>> Am Mittwoch, 20. Mai 2020, 19:40:44 MESZ hat Tom Tromey <tromey@adacore.com> Folgendes geschrieben: > >>> > >>>> I ran gdb 10 times like: > >>>> > >>>> /bin/time -f %e \ > >>>> ./gdb/gdb --data-directory ./gdb/data-directory -nx \ > >>>> -iex 'set debug-file-directory /usr/lib/debug' \ > >>>> -batch $X > >>>> > >>>> ... where $X was the test executable. Then I computed the mean time. > >>>> This was all done with a standard (-g -O2) build of gdb. > >>>> > >>>> The baseline times were > >>>> > >>>> gdb 1.90 > >>>> libxul 2.12 > >>>> Ada 2.61 > >>>> > >>>> This patch brings the numbers down to > >>>> > >>>> gdb 1.88 > >>>> libxul 2.11 > >>>> Ada 2.60 > >>> > >>> When I saw this, I thought I could do a similar profiling test on Windows (but > >>> only with gdb itself). > >>> > >>> So just: gdb.exe -q -batch gdb.exe > >>> > >>> And I was a bit suprised to see that strcmp_iw_ordered (called from > >>> sort_pst_symbols -> std::sort) is in ~24% of the profiling samples. > >>> And only because of the functions isspace and tolower. > >>> > >>> So I made a simple test, and added this before strcmp_iw_ordered: > >>> > >>> static inline int isspace2 (int c) > >>> { > >>> return c == 0x20 || (c >= 0x09 && c <= 0x0d); > >>> } > >>> #define isspace isspace2 > >>> > >>> static inline int tolower2 (int c) > >>> { > >>> return (c >= 'A' && c <= 'Z') ? c + 0x20 : c; > >>> } > >>> #define tolower tolower2 > >>> > >>> And the mean time went from 3.7s down to 2.7s. > >>> > >>> > >>> I'm not saying this is a correct solution, but does strcmp_iw_ordered have to > >>> support anything besides the "C" locale? > >>> > >>> Also, are isspace and tolower only on Windows a bottleneck? > >>> > >>> (If anyone wants to see them, I can provide some profiler flame-graphs) > >> > >> > >> There was a patch for this not that long ago. Let me try to dig it up. > > > > You're right, I found it here: > > https://sourceware.org/pipermail/gdb-patches/2019-June/158285.html > > Yes, that's the one! > > > > > So I guess it's not just on Windows that slow. > > > > And you replied that we maybe should use TOLOWER, ISXDIGIT from libiberty > > instead: > > https://sourceware.org/pipermail/gdb-patches/2019-June/158518.html > > This message is actually older than the patch above -- I wrote the patch > afterwards. > > The patch is using the libiberty macros, and avoids the readline clash > you run into. Could you give it a try? It wasn't immediately obvious to me, but I think you mean this one: https://sourceware.org/pipermail/gdb-patches/2019-June/158525.html I tried it, and as expected, I get the same speedup as with my previous test, and strcmp_iw_ordered is now in only ~1.5% of the profiling samples. Hannes ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-21 17:18 ` Hannes Domani @ 2020-05-22 15:47 ` Pedro Alves 2020-05-22 20:28 ` Pedro Alves 0 siblings, 1 reply; 16+ messages in thread From: Pedro Alves @ 2020-05-22 15:47 UTC (permalink / raw) To: Hannes Domani, Gdb-patches On 5/21/20 6:18 PM, Hannes Domani via Gdb-patches wrote: > Am Donnerstag, 21. Mai 2020, 18:42:23 MESZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: > >> On 5/21/20 4:03 PM, Hannes Domani via Gdb-patches wrote: >>>> There was a patch for this not that long ago. Let me try to dig it up. >>> >>> You're right, I found it here: >>> https://sourceware.org/pipermail/gdb-patches/2019-June/158285.html >> >> Yes, that's the one! >> >>> >>> So I guess it's not just on Windows that slow. >>> >>> And you replied that we maybe should use TOLOWER, ISXDIGIT from libiberty >>> instead: >>> https://sourceware.org/pipermail/gdb-patches/2019-June/158518.html >> >> This message is actually older than the patch above -- I wrote the patch >> afterwards. >> >> The patch is using the libiberty macros, and avoids the readline clash >> you run into. Could you give it a try? > > It wasn't immediately obvious to me, but I think you mean this one: > https://sourceware.org/pipermail/gdb-patches/2019-June/158525.html Oh, wow, I confused the "158525" in the url with "158285". They looked the same number to me. Sorry. Yes, I meant that patch as you found out. > > I tried it, and as expected, I get the same speedup as with my previous > test, and strcmp_iw_ordered is now in only ~1.5% of the profiling samples. Awesome. Let me write some git log / ChangeLog for it and resend it as a separate thread. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/4] Attribute method inlining 2020-05-22 15:47 ` Pedro Alves @ 2020-05-22 20:28 ` Pedro Alves 0 siblings, 0 replies; 16+ messages in thread From: Pedro Alves @ 2020-05-22 20:28 UTC (permalink / raw) To: Hannes Domani, Gdb-patches On 5/22/20 4:47 PM, Pedro Alves via Gdb-patches wrote: > On 5/21/20 6:18 PM, Hannes Domani via Gdb-patches wrote: >> Am Donnerstag, 21. Mai 2020, 18:42:23 MESZ hat Pedro Alves <palves@redhat.com> Folgendes geschrieben: >> >>> On 5/21/20 4:03 PM, Hannes Domani via Gdb-patches wrote: > >>>>> There was a patch for this not that long ago. Let me try to dig it up. >>>> >>>> You're right, I found it here: >>>> https://sourceware.org/pipermail/gdb-patches/2019-June/158285.html >>> >>> Yes, that's the one! >>> >>>> >>>> So I guess it's not just on Windows that slow. >>>> >>>> And you replied that we maybe should use TOLOWER, ISXDIGIT from libiberty >>>> instead: >>>> https://sourceware.org/pipermail/gdb-patches/2019-June/158518.html >>> >>> This message is actually older than the patch above -- I wrote the patch >>> afterwards. >>> >>> The patch is using the libiberty macros, and avoids the readline clash >>> you run into. Could you give it a try? >> >> It wasn't immediately obvious to me, but I think you mean this one: >> https://sourceware.org/pipermail/gdb-patches/2019-June/158525.html > > Oh, wow, I confused the "158525" in the url with "158285". They looked > the same number to me. Sorry. Yes, I meant that patch as you found out. > >> >> I tried it, and as expected, I get the same speedup as with my previous >> test, and strcmp_iw_ordered is now in only ~1.5% of the profiling samples. > > Awesome. Let me write some git log / ChangeLog for it and resend it as > a separate thread. Sent here: https://sourceware.org/pipermail/gdb-patches/2020-May/168897.html ( I borrowed Tromey's git log text a bit. :-) ) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/4] Lazily compute partial DIE name 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey 2020-05-20 17:40 ` [PATCH 1/4] Attribute method inlining Tom Tromey @ 2020-05-20 17:40 ` Tom Tromey 2020-05-20 17:40 ` [PATCH 3/4] Inline abbrev lookup Tom Tromey ` (4 subsequent siblings) 6 siblings, 0 replies; 16+ messages in thread From: Tom Tromey @ 2020-05-20 17:40 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey Currently the name of a partial DIE is computed eagerly. However, the name is not always needed. This patch changes partial DIEs to compute their name lazily, improving performance by avoiding unnecessary name computations. The run previous to this had times of (see the first patch in the series for an explanation): gdb 1.88 libxul 2.11 Ada 2.60 This patch improves the times to: gdb 1.69 libxul 2.02 Ada 2.52 gdb/ChangeLog 2020-05-20 Tom Tromey <tromey@adacore.com> * dwarf2/read.c (struct partial_die_info) <name>: Declare new method. <canonical_name>: New member. <raw_name>: Rename from "name". (partial_die_info): Initialize canonical_name. (scan_partial_symbols): Check raw_name. (partial_die_parent_scope, partial_die_full_name) (add_partial_symbol, add_partial_subprogram) (add_partial_enumeration, load_partial_dies): Use "name" method. (partial_die_info::name): New method. (partial_die_info::read, guess_partial_die_structure_name) (partial_die_info::fixup): Update. --- gdb/ChangeLog | 15 ++++++++ gdb/dwarf2/read.c | 93 ++++++++++++++++++++++++++++++----------------- 2 files changed, 75 insertions(+), 33 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index ded71f53b59..b40857be5e4 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -962,6 +962,10 @@ struct partial_die_info : public allocate_on_obstack const struct abbrev_info &abbrev, const gdb_byte *info_ptr); + /* Compute the name of this partial DIE. This memoizes the + result, so it is safe to call multiple times. */ + const char *name (dwarf2_cu *cu); + /* Offset of this DIE. */ const sect_offset sect_off; @@ -1003,9 +1007,11 @@ struct partial_die_info : public allocate_on_obstack /* Flag set if spec_offset uses DW_FORM_GNU_ref_alt. */ unsigned int spec_is_dwz : 1; + unsigned int canonical_name : 1; + /* The name of this DIE. Normally the value of DW_AT_name, but sometimes a default name for unnamed DIEs. */ - const char *name = nullptr; + const char *raw_name = nullptr; /* The linkage name, if present. */ const char *linkage_name = nullptr; @@ -1074,6 +1080,7 @@ struct partial_die_info : public allocate_on_obstack fixup_called = 0; is_dwz = 0; spec_is_dwz = 0; + canonical_name = 0; } }; @@ -8044,7 +8051,7 @@ scan_partial_symbols (struct partial_die_info *first_die, CORE_ADDR *lowpc, children, so we need to look at them. Ditto for anonymous enums. */ - if (pdi->name != NULL || pdi->tag == DW_TAG_namespace + if (pdi->raw_name != NULL || pdi->tag == DW_TAG_namespace || pdi->tag == DW_TAG_module || pdi->tag == DW_TAG_enumeration_type || pdi->tag == DW_TAG_imported_unit || pdi->tag == DW_TAG_inlined_subroutine) @@ -8189,7 +8196,7 @@ partial_die_parent_scope (struct partial_die_info *pdi, Work around this problem here. */ if (cu->language == language_cplus && parent->tag == DW_TAG_namespace - && strcmp (parent->name, "::") == 0 + && strcmp (parent->name (cu), "::") == 0 && grandparent_scope == NULL) { parent->scope = NULL; @@ -8213,11 +8220,11 @@ partial_die_parent_scope (struct partial_die_info *pdi, && pdi->tag == DW_TAG_subprogram)) { if (grandparent_scope == NULL) - parent->scope = parent->name; + parent->scope = parent->name (cu); else parent->scope = typename_concat (&cu->comp_unit_obstack, grandparent_scope, - parent->name, 0, cu); + parent->name (cu), 0, cu); } else { @@ -8251,7 +8258,7 @@ partial_die_full_name (struct partial_die_info *pdi, { pdi->fixup (cu); - if (pdi->name != NULL && strchr (pdi->name, '<') == NULL) + if (pdi->name (cu) != NULL && strchr (pdi->name (cu), '<') == NULL) { struct die_info *die; struct attribute attr; @@ -8272,7 +8279,8 @@ partial_die_full_name (struct partial_die_info *pdi, return NULL; else return gdb::unique_xmalloc_ptr<char> (typename_concat (NULL, parent_scope, - pdi->name, 0, cu)); + pdi->name (cu), + 0, cu)); } static void @@ -8294,7 +8302,7 @@ add_partial_symbol (struct partial_die_info *pdi, struct dwarf2_cu *cu) actual_name = built_actual_name.get (); if (actual_name == NULL) - actual_name = pdi->name; + actual_name = pdi->name (cu); partial_symbol psymbol; memset (&psymbol, 0, sizeof (psymbol)); @@ -8556,7 +8564,7 @@ add_partial_subprogram (struct partial_die_info *pdi, /* Ignore subprogram DIEs that do not have a name, they are illegal. Do not emit a complaint at this point, we will do so when we convert this psymtab into a symtab. */ - if (pdi->name) + if (pdi->name (cu)) add_partial_symbol (pdi, cu); } } @@ -8587,13 +8595,13 @@ add_partial_enumeration (struct partial_die_info *enum_pdi, { struct partial_die_info *pdi; - if (enum_pdi->name != NULL) + if (enum_pdi->name (cu) != NULL) add_partial_symbol (enum_pdi, cu); pdi = enum_pdi->die_child; while (pdi) { - if (pdi->tag != DW_TAG_enumerator || pdi->name == NULL) + if (pdi->tag != DW_TAG_enumerator || pdi->raw_name == NULL) complaint (_("malformed enumerator DIE ignored")); else add_partial_symbol (pdi, cu); @@ -18181,8 +18189,8 @@ load_partial_dies (const struct die_reader_specs *reader, || pdi.tag == DW_TAG_base_type || pdi.tag == DW_TAG_subrange_type)) { - if (building_psymtab && pdi.name != NULL) - add_psymbol_to_list (pdi.name, false, + if (building_psymtab && pdi.raw_name != NULL) + add_psymbol_to_list (pdi.name (cu), false, VAR_DOMAIN, LOC_TYPEDEF, -1, psymbol_placement::STATIC, 0, cu->language, objfile); @@ -18213,10 +18221,10 @@ load_partial_dies (const struct die_reader_specs *reader, && parent_die->tag == DW_TAG_enumeration_type && parent_die->has_specification == 0) { - if (pdi.name == NULL) + if (pdi.raw_name == NULL) complaint (_("malformed enumerator DIE ignored")); else if (building_psymtab) - add_psymbol_to_list (pdi.name, false, + add_psymbol_to_list (pdi.name (cu), false, VAR_DOMAIN, LOC_CONST, -1, cu->language == language_cplus ? psymbol_placement::GLOBAL @@ -18300,8 +18308,8 @@ load_partial_dies (const struct die_reader_specs *reader, || last_die->tag == DW_TAG_enumeration_type || (cu->language == language_cplus && last_die->tag == DW_TAG_subprogram - && (last_die->name == NULL - || strchr (last_die->name, '<') == NULL)) + && (last_die->raw_name == NULL + || strchr (last_die->raw_name, '<') == NULL)) || (cu->language != language_c && (last_die->tag == DW_TAG_class_type || last_die->tag == DW_TAG_interface_type @@ -18330,6 +18338,22 @@ partial_die_info::partial_die_info (sect_offset sect_off_, { } +/* See class definition. */ + +const char * +partial_die_info::name (dwarf2_cu *cu) +{ + if (!canonical_name && raw_name != nullptr) + { + struct objfile *objfile + = cu->per_cu->dwarf2_per_objfile->objfile; + raw_name = dwarf2_canonicalize_name (raw_name, cu, objfile); + canonical_name = 1; + } + + return raw_name; +} + /* Read a minimal amount of information into the minimal die structure. INFO_PTR should point just after the initial uleb128 of a DIE. */ @@ -18372,15 +18396,12 @@ partial_die_info::read (const struct die_reader_specs *reader, case DW_TAG_enumerator: /* These tags always have simple identifiers already; no need to canonicalize them. */ - name = DW_STRING (&attr); + canonical_name = 1; + raw_name = DW_STRING (&attr); break; default: - { - struct objfile *objfile = dwarf2_per_objfile->objfile; - - name - = dwarf2_canonicalize_name (DW_STRING (&attr), cu, objfile); - } + canonical_name = 0; + raw_name = DW_STRING (&attr); break; } break; @@ -18532,7 +18553,7 @@ partial_die_info::read (const struct die_reader_specs *reader, Really, though, this is just a workaround for the fact that gdb doesn't store both the name and the linkage name. */ if (cu->language == language_ada && linkage_name != nullptr) - name = linkage_name; + raw_name = linkage_name; if (high_pc_relative) highpc += lowpc; @@ -18709,7 +18730,8 @@ guess_partial_die_structure_name (struct partial_die_info *struct_pdi, if (actual_class_name != NULL) { struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile; - struct_pdi->name = objfile->intern (actual_class_name.get ()); + struct_pdi->raw_name = objfile->intern (actual_class_name.get ()); + struct_pdi->canonical_name = 1; } break; } @@ -18747,7 +18769,7 @@ partial_die_info::fixup (struct dwarf2_cu *cu) /* If we found a reference attribute and the DIE has no name, try to find a name in the referred to DIE. */ - if (name == NULL && has_specification) + if (raw_name == NULL && has_specification) { struct partial_die_info *spec_die; @@ -18757,9 +18779,10 @@ partial_die_info::fixup (struct dwarf2_cu *cu) spec_die->fixup (cu); - if (spec_die->name) + if (spec_die->raw_name) { - name = spec_die->name; + raw_name = spec_die->raw_name; + canonical_name = spec_die->canonical_name; /* Copy DW_AT_external attribute if it is set. */ if (spec_die->is_external) @@ -18787,8 +18810,11 @@ partial_die_info::fixup (struct dwarf2_cu *cu) /* Set default names for some unnamed DIEs. */ - if (name == NULL && tag == DW_TAG_namespace) - name = CP_ANONYMOUS_NAMESPACE_STR; + if (raw_name == NULL && tag == DW_TAG_namespace) + { + raw_name = CP_ANONYMOUS_NAMESPACE_STR; + canonical_name = 1; + } /* If there is no parent die to provide a namespace, and there are children, see if we can determine the namespace from their linkage @@ -18804,7 +18830,7 @@ partial_die_info::fixup (struct dwarf2_cu *cu) /* GCC might emit a nameless struct or union that has a linkage name. See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47510. */ - if (name == NULL + if (raw_name == NULL && (tag == DW_TAG_class_type || tag == DW_TAG_interface_type || tag == DW_TAG_structure_type @@ -18826,7 +18852,8 @@ partial_die_info::fixup (struct dwarf2_cu *cu) base = demangled.get (); struct objfile *objfile = cu->per_cu->dwarf2_per_objfile->objfile; - name = objfile->intern (base); + raw_name = objfile->intern (base); + canonical_name = 1; } } -- 2.21.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/4] Inline abbrev lookup 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey 2020-05-20 17:40 ` [PATCH 1/4] Attribute method inlining Tom Tromey 2020-05-20 17:40 ` [PATCH 2/4] Lazily compute partial DIE name Tom Tromey @ 2020-05-20 17:40 ` Tom Tromey 2020-05-20 17:40 ` [PATCH 4/4] Use add_partial_symbol in load_partial_dies Tom Tromey ` (3 subsequent siblings) 6 siblings, 0 replies; 16+ messages in thread From: Tom Tromey @ 2020-05-20 17:40 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey Profiling showed that calls to abbrev_table::lookup_abbrev were "too visible". As these are just forwarding calls to the hash table, this patch inlines the lookup. Also, htab_find_with_hash is used, avoiding another call. The run previous to this had times of (see the first patch in the series for an explanation): gdb 1.69 libxul 2.02 Ada 2.52 This patch improves the times to: gdb 1.64 libxul 1.99 Ada 2.47 gdb/ChangeLog 2020-05-20 Tom Tromey <tromey@adacore.com> * dwarf2/abbrev.h (struct abbrev_table) <lookup_abbrev>: Inline. Use htab_find_with_hash. <add_abbrev>: Remove "abbrev_number" parameter. * dwarf2/abbrev.c (abbrev_table::add_abbrev): Remove "abbrev_number" parameter. Use htab_find_slot_with_hash. (hash_abbrev): Add comment. (abbrev_table::lookup_abbrev): Move to header file. (abbrev_table::read): Update. --- gdb/ChangeLog | 11 +++++++++++ gdb/dwarf2/abbrev.c | 22 ++++++---------------- gdb/dwarf2/abbrev.h | 13 +++++++++++-- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/gdb/dwarf2/abbrev.c b/gdb/dwarf2/abbrev.c index b85018060fa..1552594efb5 100644 --- a/gdb/dwarf2/abbrev.c +++ b/gdb/dwarf2/abbrev.c @@ -36,6 +36,8 @@ static hashval_t hash_abbrev (const void *item) { const struct abbrev_info *info = (const struct abbrev_info *) item; + /* Warning: if you change this next line, you must also update the + other code in this class using the _with_hash functions. */ return info->number; } @@ -79,25 +81,13 @@ abbrev_table::alloc_abbrev () /* Add an abbreviation to the table. */ void -abbrev_table::add_abbrev (unsigned int abbrev_number, - struct abbrev_info *abbrev) +abbrev_table::add_abbrev (struct abbrev_info *abbrev) { - void **slot = htab_find_slot (m_abbrevs.get (), abbrev, INSERT); + void **slot = htab_find_slot_with_hash (m_abbrevs.get (), abbrev, + abbrev->number, INSERT); *slot = abbrev; } -/* Look up an abbrev in the table. - Returns NULL if the abbrev is not found. */ - -struct abbrev_info * -abbrev_table::lookup_abbrev (unsigned int abbrev_number) -{ - struct abbrev_info search; - search.number = abbrev_number; - - return (struct abbrev_info *) htab_find (m_abbrevs.get (), &search); -} - /* Read in an abbrev table. */ abbrev_table_up @@ -172,7 +162,7 @@ abbrev_table::read (struct objfile *objfile, memcpy (cur_abbrev->attrs, cur_attrs.data (), cur_abbrev->num_attrs * sizeof (struct attr_abbrev)); - abbrev_table->add_abbrev (abbrev_number, cur_abbrev); + abbrev_table->add_abbrev (cur_abbrev); /* Get next abbreviation. Under Irix6 the abbreviations for a compilation unit are not diff --git a/gdb/dwarf2/abbrev.h b/gdb/dwarf2/abbrev.h index b9ace64b448..888f04ebebb 100644 --- a/gdb/dwarf2/abbrev.h +++ b/gdb/dwarf2/abbrev.h @@ -27,6 +27,8 @@ #ifndef GDB_DWARF2_ABBREV_H #define GDB_DWARF2_ABBREV_H +#include "hashtab.h" + /* This data structure holds the information of an abbrev. */ struct abbrev_info { @@ -60,8 +62,15 @@ struct abbrev_table /* Look up an abbrev in the table. Returns NULL if the abbrev is not found. */ - struct abbrev_info *lookup_abbrev (unsigned int abbrev_number); + struct abbrev_info *lookup_abbrev (unsigned int abbrev_number) + { + struct abbrev_info search; + search.number = abbrev_number; + return (struct abbrev_info *) htab_find_with_hash (m_abbrevs.get (), + &search, + abbrev_number); + } /* Where the abbrev table came from. This is used as a sanity check when the table is used. */ @@ -78,7 +87,7 @@ struct abbrev_table struct abbrev_info *alloc_abbrev (); /* Add an abbreviation to the table. */ - void add_abbrev (unsigned int abbrev_number, struct abbrev_info *abbrev); + void add_abbrev (struct abbrev_info *abbrev); /* Hash table of abbrevs. */ htab_up m_abbrevs; -- 2.21.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/4] Use add_partial_symbol in load_partial_dies 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey ` (2 preceding siblings ...) 2020-05-20 17:40 ` [PATCH 3/4] Inline abbrev lookup Tom Tromey @ 2020-05-20 17:40 ` Tom Tromey 2020-05-20 19:30 ` [PATCH 0/4] Micro-optimize DWARF partial symbol reading Christian Biesinger ` (2 subsequent siblings) 6 siblings, 0 replies; 16+ messages in thread From: Tom Tromey @ 2020-05-20 17:40 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey An earlier patch added the add_partial_symbol helper function to dwarf2/read.c. However, a couple of calls to add_psymbol_to_list were left in place. It turns out that these calls slow down partial symbol reading, because they still go via the path that tries to needlessly demangle already-demangled names. This patch improves the performance of partial symbol reading by changing this code to use add_partial_symbol instead. The run previous to this had times of (see the first patch in the series for an explanation): gdb 1.64 libxul 1.99 Ada 2.47 This patch improves the times to: gdb 1.47 libxul 1.89 Ada 2.39 gdb/ChangeLog 2020-05-20 Tom Tromey <tromey@adacore.com> * dwarf2/read.c (load_partial_dies): Use add_partial_symbol. --- gdb/ChangeLog | 4 ++++ gdb/dwarf2/read.c | 13 +++---------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index b40857be5e4..363468f3683 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -18190,10 +18190,8 @@ load_partial_dies (const struct die_reader_specs *reader, || pdi.tag == DW_TAG_subrange_type)) { if (building_psymtab && pdi.raw_name != NULL) - add_psymbol_to_list (pdi.name (cu), false, - VAR_DOMAIN, LOC_TYPEDEF, -1, - psymbol_placement::STATIC, - 0, cu->language, objfile); + add_partial_symbol (&pdi, cu); + info_ptr = locate_pdi_sibling (reader, &pdi, info_ptr); continue; } @@ -18224,12 +18222,7 @@ load_partial_dies (const struct die_reader_specs *reader, if (pdi.raw_name == NULL) complaint (_("malformed enumerator DIE ignored")); else if (building_psymtab) - add_psymbol_to_list (pdi.name (cu), false, - VAR_DOMAIN, LOC_CONST, -1, - cu->language == language_cplus - ? psymbol_placement::GLOBAL - : psymbol_placement::STATIC, - 0, cu->language, objfile); + add_partial_symbol (&pdi, cu); info_ptr = locate_pdi_sibling (reader, &pdi, info_ptr); continue; -- 2.21.3 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/4] Micro-optimize DWARF partial symbol reading 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey ` (3 preceding siblings ...) 2020-05-20 17:40 ` [PATCH 4/4] Use add_partial_symbol in load_partial_dies Tom Tromey @ 2020-05-20 19:30 ` Christian Biesinger 2020-05-20 21:08 ` Simon Marchi 2020-05-27 17:48 ` Tom Tromey 6 siblings, 0 replies; 16+ messages in thread From: Christian Biesinger @ 2020-05-20 19:30 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches On Wed, May 20, 2020 at 12:40 PM Tom Tromey <tromey@adacore.com> wrote: > > A personal goal of mine is to improve the startup time of gdb. In the > long run, I think the answer lies partly with threading, and perhaps > with a more radical rewrite of the DWARF psymbol reader. However, > those are difficult goals; and in the short term, I found that just > profiling the reader and making small improvements can make a > difference. > > This series improves the performance of the DWARF partial symbol > reader about 10% (more in one case) on some real-world executables. > See the first patch for details (I chose to put the details there so > they would end up in the eventual git log). > > Regression tested on x86-64 Fedora 30. > > Let me know what you think. These look good to me, for what it's worth. Christian ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/4] Micro-optimize DWARF partial symbol reading 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey ` (4 preceding siblings ...) 2020-05-20 19:30 ` [PATCH 0/4] Micro-optimize DWARF partial symbol reading Christian Biesinger @ 2020-05-20 21:08 ` Simon Marchi 2020-05-27 17:48 ` Tom Tromey 6 siblings, 0 replies; 16+ messages in thread From: Simon Marchi @ 2020-05-20 21:08 UTC (permalink / raw) To: Tom Tromey, gdb-patches On 2020-05-20 1:40 p.m., Tom Tromey wrote: > A personal goal of mine is to improve the startup time of gdb. In the > long run, I think the answer lies partly with threading, and perhaps > with a more radical rewrite of the DWARF psymbol reader. However, > those are difficult goals; and in the short term, I found that just > profiling the reader and making small improvements can make a > difference. > > This series improves the performance of the DWARF partial symbol > reader about 10% (more in one case) on some real-world executables. > See the first patch for details (I chose to put the details there so > they would end up in the eventual git log). > > Regression tested on x86-64 Fedora 30. > > Let me know what you think. > > Tom I tried the series as a whole, with these two files, libxul.so, which reads this debug info file: $ l /usr/lib/debug/.build-id/06/bc3dd11d2331977ff78ce8e18c59216a8b9a61.debug -rwxrwxr-x 1 root root 1.5G May 8 12:21 /usr/lib/debug/.build-id/06/bc3dd11d2331977ff78ce8e18c59216a8b9a61.debug and libwebkit2gtk-4.0.so.37.28.5, which reads this debug info file: $ l /usr/lib/debug/.build-id/77/5b4022ee4a85d12697b8791001b40570c25f98.debug -rwxrwxr-x 1 root root 1.4G Aug 15 2018 /usr/lib/debug/.build-id/77/5b4022ee4a85d12697b8791001b40570c25f98.debug So both are about the same size. This is without the patchset applied $ for i in 1 2 3 4 5; do time ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch; done ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 97.10s user 1.81s system 102% cpu 1:36.94 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 97.61s user 1.96s system 102% cpu 1:37.55 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 99.33s user 1.90s system 101% cpu 1:39.34 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 96.87s user 1.95s system 101% cpu 1:36.92 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 97.19s user 1.94s system 102% cpu 1:37.10 total $ for i in 1 2 3 4 5; do time ./gdb -nx --data-directory=data-directory /usr/lib/x86_64-linux-gnu/libwebkit2gtk-4.0.so.37.28.5 -batch; done ./gdb -nx --data-directory=data-directory -batch 96.66s user 1.27s system 101% cpu 1:36.76 total ./gdb -nx --data-directory=data-directory -batch 95.63s user 1.45s system 101% cpu 1:35.92 total ./gdb -nx --data-directory=data-directory -batch 92.45s user 1.24s system 101% cpu 1:32.62 total ./gdb -nx --data-directory=data-directory -batch 96.55s user 1.45s system 101% cpu 1:36.85 total ./gdb -nx --data-directory=data-directory -batch 92.75s user 1.34s system 101% cpu 1:32.93 total And this is with the patchset applied: $ for i in 1 2 3 4 5; do time ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch; done ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 58.08s user 1.71s system 103% cpu 57.780 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 57.89s user 1.75s system 103% cpu 57.618 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 57.85s user 1.67s system 103% cpu 57.492 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 58.03s user 1.85s system 103% cpu 57.883 total ./gdb -nx --data-directory=data-directory /usr/lib/firefox/libxul.so -batch 58.16s user 1.73s system 103% cpu 57.833 total $ for i in 1 2 3 4 5; do time ./gdb -nx --data-directory=data-directory /usr/lib/x86_64-linux-gnu/libwebkit2gtk-4.0.so.37.28.5 -batch; do ne ./gdb -nx --data-directory=data-directory -batch 57.81s user 1.17s system 102% cpu 57.788 total ./gdb -nx --data-directory=data-directory -batch 57.60s user 1.27s system 101% cpu 57.728 total ./gdb -nx --data-directory=data-directory -batch 57.75s user 1.18s system 101% cpu 57.847 total ./gdb -nx --data-directory=data-directory -batch 57.33s user 1.19s system 102% cpu 57.318 total ./gdb -nx --data-directory=data-directory -batch 57.95s user 1.17s system 101% cpu 57.967 total It's still a bit too long for an interactive user to wait, but it's quite an improvement! Simon ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 0/4] Micro-optimize DWARF partial symbol reading 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey ` (5 preceding siblings ...) 2020-05-20 21:08 ` Simon Marchi @ 2020-05-27 17:48 ` Tom Tromey 6 siblings, 0 replies; 16+ messages in thread From: Tom Tromey @ 2020-05-27 17:48 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches >>>>> "Tom" == Tom Tromey <tromey@adacore.com> writes: Tom> This series improves the performance of the DWARF partial symbol Tom> reader about 10% (more in one case) on some real-world executables. Tom> See the first patch for details (I chose to put the details there so Tom> they would end up in the eventual git log). I've rebased these and re-run the performance tests to make sure they are still improvements. So, I'm checking them in. Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2020-05-27 17:48 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-20 17:40 [PATCH 0/4] Micro-optimize DWARF partial symbol reading Tom Tromey 2020-05-20 17:40 ` [PATCH 1/4] Attribute method inlining Tom Tromey 2020-05-20 19:40 ` Tom Tromey 2020-05-21 1:08 ` Hannes Domani 2020-05-21 14:26 ` Pedro Alves 2020-05-21 15:03 ` Hannes Domani 2020-05-21 16:42 ` Pedro Alves 2020-05-21 17:18 ` Hannes Domani 2020-05-22 15:47 ` Pedro Alves 2020-05-22 20:28 ` Pedro Alves 2020-05-20 17:40 ` [PATCH 2/4] Lazily compute partial DIE name Tom Tromey 2020-05-20 17:40 ` [PATCH 3/4] Inline abbrev lookup Tom Tromey 2020-05-20 17:40 ` [PATCH 4/4] Use add_partial_symbol in load_partial_dies Tom Tromey 2020-05-20 19:30 ` [PATCH 0/4] Micro-optimize DWARF partial symbol reading Christian Biesinger 2020-05-20 21:08 ` Simon Marchi 2020-05-27 17:48 ` Tom Tromey
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).