Re: [PATCH] powerpc: strcasestr optimization

[Steven Munroe <munroesj at linux dot vnet dot ibm dot com>]

On Tue, 2015-06-02 at 23:00 +0200, OndÅej BÃlka wrote:
> On Tue, Jun 02, 2015 at 01:47:03PM -0500, Steven Munroe wrote:
> > On Mon, 2015-06-01 at 18:22 +0200, OndÅej BÃlka wrote:
> > > On Mon, Jun 01, 2015 at 09:36:47AM -0500, Steven Munroe wrote:
> > > > On Mon, 2015-06-01 at 14:28 +0200, OndÅej BÃlka wrote:
> > > > > On Mon, Jun 01, 2015 at 04:11:28PM +0530, Rajalakshmi Srinivasaraghavan wrote:
> > > > > > 
> > > > > > This patch optimizes strcasestr function for power >= 7 systems.
> > > > > > This patch uses optimized strlen and strnlen for calculating
> > > > > > string length and the average improvement of this optimization is ~40%.
> > > > > > This patch is tested on powerpc64 and powerpc64le.
> > > > > > Attached the benchresults with this new patch.
> > > > > > 
> > > > > Thats not enough. As strcasestr that I submited is around three times
> > > > > slower your implementation would likely be regression over generic one.
> > > > > 
> > > > > A problem here is that you use moronic algorithm. Fix algorithm first
> > > > > before trying to optimize it.
> > > > > 
> > > > 
> > > > This is not very helpful. You are demanding changes without clear
> > > > explanation and justification.
> > > > 
> > > > What is wrong with Raja's algorithm? What is insufficient in the
> > > > benchmark data she has provided? And why do you think your specific
> > > > design applies to PowerISA and POWER7/POWER8 micro-architecture.
> > > > 
> > > > What data do you have that justified this objection?
> > > 
> > > I replied on strstr patch thread on why what she submitted is
> > > performance regression. So I will repeat arguments from other thread
> > > which still apply.
> > > 
> > > First was problem with quadratic behaviour. She tried to fix it but it
> > > isn't a fix at all. Just benchmark 
> > 
> > Which is fixed in the latest patch
> > > 
> > > strcasestr ("aaa...(4000 times)...aaa", "aaa...(2000 times)...aab")
> > > 
> Did you read also following line? It isn't solved at all.
> > > That call would take around hundred times than before which is
> > > unacceptable.
> > > 

Ok we need to make progress, either you need to provide data from a
POWER machine that proves your point. Or you need to proved a benchmark
that can be include in GLIBC and we can run, that proves your point.

So far this only your opinion from a abstract perspective vs my opinion
based on my experience as a platform maintainer. It is past time to
break this logjam!

Anyone disagree with this approach?

> > So there is no excuse to not have data to back you assertion.
> 
> Except that all power machines except osuosl are offline. There may be
> osuosl ones but gcc11[012].osuosl.org which don't resolve
> 

My team assures me that both gcc110 gcc112 are up and available.

-bash-4.3$ uptime
01:30PM   up 127 days,   3:38,  0 users,  load average: 2.36, 2.11, 1.94

you might try via the fsffrance.org domain.