Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 8 Feb 2021 23:19:01 +0000
From:      Jessica Clarke <jrtc27@freebsd.org>
To:        Kevin Bowling <kevin.bowling@kev009.com>
Cc:        Mateusz Guzik <mjg@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@freebsd.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@freebsd.org>
Subject:   Re: git: af366d353b84 - main - amd64: implement strlen in assembly
Message-ID:  <8E61EA5C-39D1-49CC-8319-06E9192FF735@freebsd.org>
In-Reply-To: <CAK7dMtA5rbT6QoCK2HQfzLY2f%2B3kbhdJp0ERmRpc_CVAog%2B6bg@mail.gmail.com>
References:  <202102081915.118JFXkJ067892@gitrepo.freebsd.org> <3E64387A-42DD-4470-8893-5B774F19754E@freebsd.org> <CAK7dMtA5rbT6QoCK2HQfzLY2f%2B3kbhdJp0ERmRpc_CVAog%2B6bg@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
> On 8 Feb 2021, at 23:13, Kevin Bowling <kevin.bowling@kev009.com> =
wrote:
>=20
> 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 <jrtc27@freebsd.org> =
wrote:
>>=20
>> On 8 Feb 2021, at 19:15, Mateusz Guzik <mjg@FreeBSD.org> wrote:
>>>=20
>>> The branch main has been updated by mjg:
>>>=20
>>> URL: =
https://cgit.FreeBSD.org/src/commit/?id=3Daf366d353b84bdc4e730f0fc563853ab=
c338271c
>>>=20
>>> commit af366d353b84bdc4e730f0fc563853abc338271c
>>> Author:     Mateusz Guzik <mjg@FreeBSD.org>
>>> AuthorDate: 2021-02-08 17:01:48 +0000
>>> Commit:     Mateusz Guzik <mjg@FreeBSD.org>
>>> CommitDate: 2021-02-08 19:15:21 +0000
>>>=20
>>>   amd64: implement strlen in assembly
>>>=20
>>>   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).
>>>=20
>>>   On top of that the routine can is a starting point for copyinstr
>>>   which operates on words instead of bytes.
>>>=20
>>>   Tested with glibc test suite.
>>>=20
>>>   Sample results (calls/s):
>>>=20
>>>   Haswell:
>>>   $(perl -e "print 'A' x 3"):
>>>   stock:  211198039
>>>   patched:338626619
>>>   asm:    465609618
>>>=20
>>>   $(perl -e "print 'A' x 100"):
>>>   stock:   83151997
>>>   patched: 98285919
>>>   asm:    120719888
>>>=20
>>>   AMD EPYC 7R32:
>>>   $(perl -e "print 'A' x 3"):
>>>   stock:  282523617
>>>   asm:    491498172
>>>=20
>>>   $(perl -e "print 'A' x 100"):
>>>   stock:  114857172
>>>   asm:    112082057
>>=20
>> No Reviewed by? More than one pair of eyes on non-trivial assembly is
>> almost always a good idea.
>>=20
>> Jess
>>=20
>> _______________________________________________
>> 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"




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?8E61EA5C-39D1-49CC-8319-06E9192FF735>