Re: [PATCH 1/4] Attribute method inlining

Hannes Domani
2020-05-21T15:04:01Z

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