Re: [PATCH 1/4] Attribute method inlining

[Pedro Alves <palves@redhat.com>]

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.
>