git: af366d353b84 - main - amd64: implement strlen in assembly

[Jessica Clarke]

> On 8 Feb 2021, at 23:13, Kevin Bowling <kevin.bowling at kev009.com> 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:

	• 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 at freebsd.org> wrote:
>> 
>> On 8 Feb 2021, at 19:15, Mateusz Guzik <mjg at FreeBSD.org> wrote:
>>> 
>>> The branch main has been updated by mjg:
>>> 
>>> URL: https://cgit.FreeBSD.org/src/commit/?id=af366d353b84bdc4e730f0fc563853abc338271c
>>> 
>>> commit af366d353b84bdc4e730f0fc563853abc338271c
>>> Author:     Mateusz Guzik <mjg at FreeBSD.org>
>>> AuthorDate: 2021-02-08 17:01:48 +0000
>>> Commit:     Mateusz Guzik <mjg at FreeBSD.org>
>>> 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 at 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 at freebsd.org"