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 <b...@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. Do 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. Normal code spends> > 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>> performance, It seems that at most this couldn't hurt. On Tue, Nov 22, 2011 at 10:33 AM, David Schultz <d...@freebsd.org> wrote: > Third, it's not clear that checking whether s1 == s2 is even an > optimization. Most 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. (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? -- Eitan Adler Ports committer X11, Bugbusting teams _______________________________________________ svn-src-head@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
