From owner-dev-commits-src-main@freebsd.org Tue Feb 9 00:18:20 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 82C5D546FA9 for ; Tue, 9 Feb 2021 00:18:20 +0000 (UTC) (envelope-from kevin.bowling@kev009.com) Received: from mail-yb1-xb31.google.com (mail-yb1-xb31.google.com [IPv6:2607:f8b0:4864:20::b31]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DZNlc1zLzz3N0l for ; Tue, 9 Feb 2021 00:18:20 +0000 (UTC) (envelope-from kevin.bowling@kev009.com) Received: by mail-yb1-xb31.google.com with SMTP id e132so16424077ybh.8 for ; Mon, 08 Feb 2021 16:18:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kev009.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=9mpgF1vFaoosp1Q7FztwyeaDDcq0pgBKtjY1w964Pm0=; b=KS62rzBkZ/U/RKzCHtvHSBMDGyT0HExNzBf9VWkNeeTyy3GbQxCc2+PAT1ewlR2UzA kG9vfBgSYeVPs2Le7tN4TdUzf8n5LmaK8EtLdYT8G5GuMjYuuYUOC7Wnsxv/BtevmKXr ZNcmVWqilbR8alFNFFQOc9KtdCcLBDYOteHR8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9mpgF1vFaoosp1Q7FztwyeaDDcq0pgBKtjY1w964Pm0=; b=d92VkT6zMcQjX6UEGHaZFVxsRXYeNJ4aCGebOO5in/9Xp+fEnEGI2yrcsGkFtbTk7f oFkIfxYHj/V6skLfKJSABRDX3pSfFXvM08lPIs4DrtQnZPYoXTn8CtSPKtn1rp0fnLAP +fcK2A5G4fcnyMAD1+bIxu6HhZnh08cO391JIw+1iezLZYHbiGL8z0bJpebgQmHENw6A p61dQQ0Q1rvk6uziyXhm6DIteL5AwR4sGzRCKJTFGFXpAbGz1H9+0Qw8iqV2FKn5C/zZ LB3OK26A0emthk/53Gj3L44OPYo2D8OyMyQCRhn9oI7KqNAxMapuJWVHtIGj7Zw2g+zA JtNQ== X-Gm-Message-State: AOAM53284m68zvsjLI+wpbM1l1lxnbMnaJTrxGI7dHCNmvH63PRfeEl7 8Eyw+1LxmBGlYFNbXnIhFDVw/A7J7rS1Q/GTOPT6/Q== X-Google-Smtp-Source: ABdhPJyLp2qp6/oTDAkeO72jiLeAWPT73klx5nxbxWJV8VRw6yuI7dM0v0wEGK6adp0uCTHOPN+z/hR9XNVxI+pPBEc= X-Received: by 2002:a25:7453:: with SMTP id p80mr28474014ybc.297.1612829899236; Mon, 08 Feb 2021 16:18:19 -0800 (PST) MIME-Version: 1.0 References: <202102081915.118JFXkJ067892@gitrepo.freebsd.org> <3E64387A-42DD-4470-8893-5B774F19754E@freebsd.org> <8E61EA5C-39D1-49CC-8319-06E9192FF735@freebsd.org> In-Reply-To: <8E61EA5C-39D1-49CC-8319-06E9192FF735@freebsd.org> From: Kevin Bowling Date: Mon, 8 Feb 2021 17:18:08 -0700 Message-ID: Subject: Re: git: af366d353b84 - main - amd64: implement strlen in assembly To: Jessica Clarke Cc: Mateusz Guzik , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" , "src-committers@freebsd.org" X-Rspamd-Queue-Id: 4DZNlc1zLzz3N0l X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.34 X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Feb 2021 00:18:20 -0000 I understand your position and Warner=E2=80=99s from the documentation. Th= e problem which is not described is that frustration is asymptotically higher in the other direction without volunteering to do work. As another example I could reply and ask for unit tests for any change (tests are obviously helpful too) but unless I am willing to help it is just a suggestion and should not be sent as a command. If you are willing to do such reviews timely, or have command of someone who will, I will coordinate with mjg and person. Otherwise it=E2=80=99s volunteering other people=E2=80=99s time an= d reduces the willpower to fix these performance areas. The head model supports occasional break and revert. I think this is important given the resources FreeBSD has available, since a lot of the less glamorous work is unpaid and underpaid. Picking on mjg for this is suspicious given how frequently breakage happens by anyone right now juxtaposed to his track record of improvements and quickness to address issues or revert where issue arose. Regards, Kevin On Mon, Feb 8, 2021 at 4:19 PM Jessica Clarke wrote: > > On 8 Feb 2021, at 23:13, Kevin Bowling wrote= : > > > > FreeBSD does not require pre-commit approval unless called out > > specifically. Are you volunteering to review the changes, and if so > > where is your guidance? These messages are otherwise unhelpful. > > It is not a hard requirement, but it is strongly encouraged. Section 7 > of the committer's guide says: > > =E2=80=A2 All non-trivial changes should be reviewed before they = are > committed to the repository. > > This was a non-trivial change. I was particularly frustrated to see > this commit go in without review having previously called out mjg@ for > not getting any reviews for his (now reverted) previous strlen change. > > Jess > > > On Mon, Feb 8, 2021 at 12:37 PM Jessica Clarke > wrote: > >> > >> On 8 Feb 2021, at 19:15, Mateusz Guzik wrote: > >>> > >>> The branch main has been updated by mjg: > >>> > >>> URL: > https://cgit.FreeBSD.org/src/commit/?id=3Daf366d353b84bdc4e730f0fc563853a= bc338271c > >>> > >>> commit af366d353b84bdc4e730f0fc563853abc338271c > >>> Author: Mateusz Guzik > >>> AuthorDate: 2021-02-08 17:01:48 +0000 > >>> Commit: Mateusz Guzik > >>> CommitDate: 2021-02-08 19:15:21 +0000 > >>> > >>> amd64: implement strlen in assembly > >>> > >>> The C variant in libkern performs excessive branching to find the > >>> non-zero byte instead of using the bsfq instruction. The same code > >>> patched to use it is still slower than the routine implemented here > >>> as the compiler keeps neglecting to perform certain optimizations > >>> (like using leaq). > >>> > >>> On top of that the routine can is a starting point for copyinstr > >>> which operates on words instead of bytes. > >>> > >>> Tested with glibc test suite. > >>> > >>> Sample results (calls/s): > >>> > >>> Haswell: > >>> $(perl -e "print 'A' x 3"): > >>> stock: 211198039 > >>> patched:338626619 > >>> asm: 465609618 > >>> > >>> $(perl -e "print 'A' x 100"): > >>> stock: 83151997 > >>> patched: 98285919 > >>> asm: 120719888 > >>> > >>> AMD EPYC 7R32: > >>> $(perl -e "print 'A' x 3"): > >>> stock: 282523617 > >>> asm: 491498172 > >>> > >>> $(perl -e "print 'A' x 100"): > >>> stock: 114857172 > >>> asm: 112082057 > >> > >> No Reviewed by? More than one pair of eyes on non-trivial assembly is > >> almost always a good idea. > >> > >> Jess > >> > >> _______________________________________________ > >> dev-commits-src-main@freebsd.org mailing list > >> https://lists.freebsd.org/mailman/listinfo/dev-commits-src-main > >> To unsubscribe, send any mail to " > dev-commits-src-main-unsubscribe@freebsd.org" > >