svn commit: r339349 - in head/sys/amd64: amd64 include

[Mateusz Guzik]

On 10/15/18, Gleb Smirnoff <glebius at freebsd.org> wrote:
>   Mateusz,
>
> On Sat, Oct 13, 2018 at 09:18:32PM +0000, Mateusz Guzik wrote:
> M>   Return is almost always 0. The change replaces 3 branches with 1 in the
> common
> M>   case.
>
> This isn't true. For a webserver working with blocking sockets
> returning EAGAIN for a very large number of syscalls is normal.
>
> I just dtraced on a random Netflix server and in our case we
> get 12% of syscalls with non zero error. But our clients are
> special, they request data in small chunks. I believe a regular
> web server that serves mostly open ended requests will have a
> greater ratio of non-zero returns, up to 50%.
>
> Here is script:
>
> #!/usr/sbin/dtrace -s
>
> fbt::cpu_set_syscall_retval:entry
> {
>         @[args[1]] = count();
> }
>
> I would be interested if anybody reports results on a busy
> web server running nginx.
>
> So, I doubt that using __predict_true() is an optimisation here.

This should be a win even for your somewhat degenerate case.

Code handling all cases is disjoint - regardless of my change it has
to jump over something.

The previous code tests 3 conditions. The case o errno EAGAIN or
similar is handled by the last. So returning zero requires 3 tests
and 0 jumps. Returning EAGAIN requires 3 tests and 1 jump.

With the committed patch returning zero requires 1 test and 0
jumps. Returning EAGAIN requires 3 tests and 2 jumps.

Given how much more frequent returning zero is, I think it's a net
win. However, the code can be further modified to just get rid of the
second jump.

Either way, the entire syscall path is extremely pessimized.

-- 
Mateusz Guzik <mjguzik gmail.com>