This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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:

Pedro Alves

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]