search

nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.25k stars 321 forks source link

fs: Use branchless code #1230

Closed alejandro-colomar closed 2 months ago

alejandro-colomar commented 2 months ago

That branch was to avoid an infinite loop on the slash. However, we can achieve the same by using a +1 to make sure we advance at least 1 byte in each iteration.

Cc: @ac000

alejandro-colomar commented 2 months ago

Not tested.

alejandro-colomar commented 2 months ago

On the face of it change looks good.

As an aside I created a new git/github workflow document (Dan's OK'd it, just hasn't been publicly released yet) anyway the point is in it I recommend to use lower case for the topic(s), some look weird capitalised and some simply shouldn't be. It also matches other projects...

Okay; both styles are similarly wide spread. In the Linux man-pages, Michael's rule was that commit messages, when removing the prefix, should be correct English sentences (except that we omit the trailing period), but of course it's arbitrary to omit the period but not omit the capital, so I guess also omitting the capital is fine.

I'll adapt the patch.

alejandro-colomar commented 2 months ago

v1b changes:

$ git range-diff master gh/mkdirp mkdirp 
1:  77340b39 ! 1:  88ddba5e Fs: Use branchless code
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>

      ## Commit message ##
    -    Fs: Use branchless code
    +    Fs: use branchless code

         That branch was to avoid an infinite loop on the slash.  However, we can
         achieve the same by using a +1 to make sure we advance at least 1 byte
ac000 commented 2 months ago

Heh, you've mis-understood.

It's the topic(s) that should be lower case, I.e

fs: Use branchless code
ac000 commented 2 months ago

Tested with cgroups and isolation/rootfs (hopefully the pytests also tested it!)

Tested-by: Andrew Clayton <a.clayton@nginx.com>
alejandro-colomar commented 2 months ago

Heh, you've mis-understood.

It's the topic(s) that should be lower case, I.e

fs: Use branchless code

Ahh, yup! That sounds much better!! I never understood why one would sentence-case a prefix :)

alejandro-colomar commented 2 months ago

Tested with cgroups and isolation/rootfs (hopefully the pytests also tested it!)

Tested-by: Andrew Clayton <a.clayton@nginx.com>

Lovely! Thanks.

alejandro-colomar commented 2 months ago

v1c changes:

$ git range-diff master gh/mkdirp mkdirp 
1:  88ddba5e ! 1:  163309e0 Fs: use branchless code
    @@ Metadata
     Author: Alejandro Colomar <alx@kernel.org>

      ## Commit message ##
    -    Fs: use branchless code
    +    fs: Use branchless code

         That branch was to avoid an infinite loop on the slash.  However, we can
         achieve the same by using a +1 to make sure we advance at least 1 byte
         in each iteration.

    +    Tested-by: Andrew Clayton <a.clayton@nginx.com>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>

      ## src/nxt_fs.c ##
alejandro-colomar commented 2 months ago

Closing in favour of https://github.com/nginx/unit/pull/1235