Re: [PATCH v2] Fix slow and non-deterministic behavior of isspace() and tolower()

[Pedro Alves <palves@redhat.com>]

On 6/21/19 6:59 PM, Shawn Landden wrote:
>> On 6/10/19 10:30 PM, Shawn Landden wrote:
>>> I was getting 8% and 6% cpu usage in tolower() and isspace(),
>>> respectively, waiting for a breakpoint on ppc64el.
>>>
>>> Also, gdb doesn't want non-deterministic behavior here.
>>>
>>> v2: do not clash with C99 names
>>
>> When I was working on the C++ wildmatching support a
>> couple years ago, I had some testcases that would stress name
>> parsing that I was running under perf, and I also noticed these functions
>> higher up on the profile.  I wrote a few patches back then:
>>
>>  https://github.com/palves/gdb/commits/palves/ada-decode-speedups
>>
>> And this one has the same idea as yours:
>>
>>
>> https://github.com/palves/gdb/commit/f701531b79380356134d53db97adb6f467f9d784
>>
>> So, I agree that this makes sense.
>>
> I don't care how it gets fixed, and the GNU coding standard (which I write
> to for glibc) will take more time than writing this patch. (Or your
> well-documented response) Also, while I have a copyright assignment for
> glibc, mine for GCC and binutils-gdb is only pending.
> 
> Go ahead and fix this, and give me credit.

The interesting thing to do here is 

 >> That patch uses the existing libiberty uppercase TOLOWER, ISXDIGIT,
 >> etc. macros, which are inline and locale independent by design.
 >> See include/safe-ctype.h.  Can we use those instead of adding new
 >> functions?  I don't recall if I benchmarked ISSPACE vs the gdb_isspace
 >> in that optimization patch on my github, but I think I just didn't
 >> remember ISSPACE back then.

How were you benchmarking this?

Thanks,
Pedro Alves