On 10/15/18, Gleb Smirnoff <[email protected]> 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> _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "[email protected]"
