From owner-svn-src-all@FreeBSD.ORG Thu Nov 24 05:17:49 2011 Return-Path: Delivered-To: svn-src-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8593F1065670; Thu, 24 Nov 2011 05:17:49 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by mx1.freebsd.org (Postfix) with ESMTP id 020118FC16; Thu, 24 Nov 2011 05:17:48 +0000 (UTC) Received: from c211-28-227-231.carlnfd1.nsw.optusnet.com.au (c211-28-227-231.carlnfd1.nsw.optusnet.com.au [211.28.227.231]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id pAO5HjXM028760 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Thu, 24 Nov 2011 16:17:46 +1100 Date: Thu, 24 Nov 2011 16:17:45 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eitan Adler In-Reply-To: Message-ID: <20111124152617.B1102@besplex.bde.org> References: <201111220250.pAM2oPWC070856@svn.freebsd.org> <20111122153332.GA20145@zim.MIT.EDU> MIME-Version: 1.0 Content-Type: MULTIPART/MIXED; BOUNDARY="0-1898521387-1322111865=:1102" Cc: src-committers@FreeBSD.org, theraven@FreeBSD.org, svn-src-all@FreeBSD.org, dim@FreeBSD.org, Brooks Davis , bde@FreeBSD.org, svn-src-head@FreeBSD.org Subject: Re: svn commit: r227812 - head/lib/libc/string X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Nov 2011 05:17:49 -0000 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 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 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--