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

[Pedro Alves <palves at redhat dot com>]

Hi,

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 also agree that we don't want to depend on the current
locale when parsing symbol names.

In my version I was naming the functions gdb_xxx while in
your version you're using xxx_inline.  gdb_xxx naming is
more common as a gdb version of some standard function,
so I would prefer that.

But, meanwhile, last year I merged this patch:
<https://sourceware.org/ml/gdb-patches/2018-05/msg00561.html>
which touches a somewhat subject.

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.

Least but not least, the patch as is is not following the 
GNU/GDB coding format conventions.  Take a look here:

 https://sourceware.org/gdb/wiki/ContributionChecklist

Thanks,
Pedro Alves