Re: [PATCH 1/4] Attribute method inlining

Hannes Domani
2020-05-21T17:18:28Z

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