This is the mail archive of the
mailing list for the GDB project.
Re: [PATCH v2] Fix slow and non-deterministic behavior of isspace() and tolower()
- From: Pedro Alves <palves at redhat dot com>
- To: Shawn Landden <shawn at git dot icu>, gdb-patches at sourceware dot org
- Cc: jhb at freebsd dot org, eliz at gnu dot org
- Date: Fri, 21 Jun 2019 18:35:24 +0100
- Subject: Re: [PATCH v2] Fix slow and non-deterministic behavior of isspace() and tolower()
- References: <email@example.com> <firstname.lastname@example.org>
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:
And this one has the same idea as yours:
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:
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: