Date: Tue, 22 Nov 2011 13:01:51 -0500 From: Eitan Adler <eadler@freebsd.org> To: Eitan Adler <eadler@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Cc: dim@freebsd.org, Brooks Davis <brooks@freebsd.org>, theraven@freebsd.org, bde@freebsd.org Subject: Re: svn commit: r227812 - head/lib/libc/string Message-ID: <CAF6rxgmPeZCZ3c0xbd-4riqvLHob8U9eWG25R8P6FG2BjTfyyA@mail.gmail.com> In-Reply-To: <20111122153332.GA20145@zim.MIT.EDU> References: <201111220250.pAM2oPWC070856@svn.freebsd.org> <20111122153332.GA20145@zim.MIT.EDU>
next in thread | previous in thread | raw e-mail | index | archive | help
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 always= > have an insignificant effect, even in benchmarks. =C2=A0Normal code spend= s> perhaps 0.01% of its time calling string functions, so the improvements>= from optimizing string functions are 0.01% of an insignificant amount. 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>> performa= nce, 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. 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? --=20 Eitan Adler Ports committer X11, Bugbusting teams
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAF6rxgmPeZCZ3c0xbd-4riqvLHob8U9eWG25R8P6FG2BjTfyyA>