Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 24 Nov 2011 16:17:45 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Eitan Adler <eadler@FreeBSD.org>
Cc:        src-committers@FreeBSD.org, theraven@FreeBSD.org, svn-src-all@FreeBSD.org, dim@FreeBSD.org, Brooks Davis <brooks@FreeBSD.org>, bde@FreeBSD.org, svn-src-head@FreeBSD.org
Subject:   Re: svn commit: r227812 - head/lib/libc/string
Message-ID:  <20111124152617.B1102@besplex.bde.org>
In-Reply-To: <CAF6rxgmPeZCZ3c0xbd-4riqvLHob8U9eWG25R8P6FG2BjTfyyA@mail.gmail.com>
References:  <201111220250.pAM2oPWC070856@svn.freebsd.org> <20111122153332.GA20145@zim.MIT.EDU> <CAF6rxgmPeZCZ3c0xbd-4riqvLHob8U9eWG25R8P6FG2BjTfyyA@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--0-1898521387-1322111865=:1102
Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE

On Tue, 22 Nov 2011, Eitan Adler wrote:

> Meta comment:
> I should have sent this patch to -hackers prior to seeking approval to
> commit. I learned my lesson and will seek wider review before making
> such changes in the future.
> On Tue, Nov 22, 2011 at 6:21 AM, Bruce Evans <brde@optusnet.com.au> wrote=
:
> I saw your email about the style changes. I'd like to flesh out what
> changes should be made first before making another commit>> I guess
> I'm a little confused. =C2=A0Do we really have profiling>> information at
> this level that suggests the overhead of the branch is>> significant?
> I thought most hardware had pretty good>> branch-prediction,
> particularly with speculative execution.
> I made this change at the direction of theraven@. I assume he knows a
> little bit more about compilers than I do. ;)It seems from the rest of
> this thread that even if this were an optimization it still shouldn't
> be done at source code code level.
>> Every time I have profiled the string functions, changes like this alway=
s> have an insignificant effect, even in benchmarks. =C2=A0Normal code spen=
ds> perhaps 0.01% of its time calling string functions, so the improvements=
> from optimizing string functions are 0.01% of an insignificant amount.

Maybe my mail client (unmentionable) is messing up something.  This came
back with the quotes even more mangled than usual, especially for my
reply.

> The problem with profiling this type of change is that it is hard to
> find a good representative benchmark. I could easily write code that
> will show you that adding the equality check is a good idea or that it
> is a horrible idea. IMHO it saves enough time when they are equal, but
> loses almost no time when the strings are not equal.
>>> Wouldn't something like __predict_false() have more value for>> perform=
ance,
> It seems that at most this couldn't hurt.
> On Tue, Nov 22, 2011 at 10:33 AM, David Schultz <das@freebsd.org> wrote:
>> Third, it's not clear that checking whether s1 =3D=3D s2 is even an
>> optimization. =C2=A0Most programs simply aren't going to pass identical
>> pointers as arguments to strcmp(), so for the overwhelming
>> majority of cases, this is just a wasted test and a wasted slot in
>> a branch predict table. =C2=A0(FWIW, I doubt that a realistic benchmark
>> would demonstrate any measurable difference.)
>
> It is fairly common for programs to compare a value passed to a
> function to a array of strings. Also note that for the strn*cmp
> functions we already have a branch so this isn't always a wasted slot.

But das's mail never has these problems, even when you quote it.

> Here is what I'd like to do next:
>
> - fix bde@'s style nits
> - change the | to a || and remove the comment
> - but leave the equality check as is.
> - find a src committer to approve the patch
> - go back to working on ports for a while ;)
>
> Is this the right course of action? Or should I just revert both
> commits entirely?

I would revert.

Another point that I forgot to mention in a previous reply is that many
of the MI implementations of string functions aren't actually used on
primary arches, so optimizing them has no effect instead of 0.01% of
an insignificant effect.  This applies to at least strcmp() in the
recent changes:
- amd64 has a sophisticated MD strcmp.  It's sophistication didn't
   include checking for equal pointers, and the recent changes didn't
   touch it.  "sophisticated" may mean over-engineered.
- i386 has an "optimized" MD strcmp.  It uses 8-way unrolling.  This
   may have been good when it was written for the original i386 or i486,
   but it is now dubious or worse.  Its optimizations didn't include
   checking for equal pointers, and the recent changes didn't touch it.
This doesn't apply to most of the other changed functions.  The efficiency
of the others was considered so unimportant that no one ever tried to
optimize them for amd64; i386 has MD "optimized" versions for many more
string functions than does i386, perhaps including a couple of the changed
ones.

One of the changed functions is strcasecmp().  This can't possibly be
signifcantly optimized at the MD level, since it does locale stuff.
The bloat in the locale stuff in it was expanded recently, so the
constant factor for the O(N) in it expanded and the pointer-equality
optimization does more.  But if the optimality of strcasecmp() mattered,
then it would have been thinner and its expansion would have generated
more mail.

Bruce
--0-1898521387-1322111865=:1102--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111124152617.B1102>