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

[Shawn Landden <slandden at gmail dot com>]

El vie., 21 de jun. de 2019 12:35, Pedro Alves <palves@redhat.com> escribió:

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

Cheers.

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