http icon indicating copy to clipboard operation
http copied to clipboard

Improve performance by avoiding unneeded syscalls

Open clue opened this issue 3 years ago • 3 comments

This project has some potential for additional performance improvements by avoiding a number of unneeded syscalls. These low-level system calls are usually issued as a result of some higher-level PHP function call and have a noticeable performance impact in many cases. Some of these calls are the result of invocations inside this project, some are actually issued lower-level in the Socket or Stream component. As a first step, I'm filing this here to try to start a discussion about this idea.

Here's the strace output with some candidates for removal highlighted:

$ strace php examples/51-server-hello-world.php [::]:8080
pselect6(5, [4], [], [], NULL, NULL)    = 1 (in [4])
poll([{fd=4, events=POLLIN|POLLERR|POLLHUP}], 1, 0) = 1 ([{fd=4, revents=POLLIN}])
accept(4, {sa_family=AF_INET6, sin6_port=htons(54462), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28]) = 3
fcntl(3, F_GETFL)                       = 0x2 (flags O_RDWR)
fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK)    = 0
- fcntl(3, F_GETFL)                       = 0x802 (flags O_RDWR|O_NONBLOCK)
- fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK)    = 0
pselect6(5, [3 4], [], [], NULL, NULL)  = 1 (in [3])
poll([{fd=3, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 1 ([{fd=3, revents=POLLIN}])
recvfrom(3, "G", 1, MSG_PEEK, NULL, NULL) = 1
recvfrom(3, "GET / HTTP/1.1\r\nHost: localhost:"..., 65536, 0, NULL, NULL) = 78
poll([{fd=3, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 0 (Timeout)
recvfrom(3, 0x7fceeb63d066, 65458, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
- getpeername(3, {sa_family=AF_INET6, sin6_port=htons(54462), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28]) = 0
- getsockname(3, {sa_family=AF_INET6, sin6_port=htons(8080), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1", &sin6_addr), sin6_scope_id=0}, [128 => 28]) = 0
- gettimeofday({tv_sec=1652528350, tv_usec=915571}, NULL) = 0
- gettimeofday({tv_sec=1652528350, tv_usec=915621}, NULL) = 0
gettimeofday({tv_sec=1652528350, tv_usec=915765}, NULL) = 0
- pselect6(5, [3 4], [3], [], NULL, NULL) = 1 (out [3])
sendto(3, "HTTP/1.1 200 OK\r\nContent-Type: t"..., 150, 0, NULL, 0) = 150
pselect6(5, [3 4], [], [], NULL, NULL)  = 1 (in [3])
poll([{fd=3, events=POLLIN|POLLPRI|POLLERR|POLLHUP}], 1, 0) = 1 ([{fd=3, revents=POLLIN}])
recvfrom(3, "", 1, MSG_PEEK, NULL, NULL) = 0
recvfrom(3, "", 65536, 0, NULL, NULL)   = 0
shutdown(3, SHUT_RDWR)                  = 0
- fcntl(3, F_GETFL)                       = 0x802 (flags O_RDWR|O_NONBLOCK)
- fcntl(3, F_SETFL, O_RDWR|O_NONBLOCK)    = 0
close(3)                                = 0

pselect6(5, [4], [], [], NULL, NULL)    = …

In particular, we may want to avoid the following calls:

  • stream_set_blocking() (see O_NONBLOCK) is called multiple times through the lifetime of the socket connection. Doing this once should be sufficient.
  • time() and microtime() (see gettimeofday()) is called twice only in order to populate the $request->getServerParams() array with REQUEST_TIME and REQUEST_TIME_FLOAT. Are these really needed? (See also #174, #192 and others)
  • stream_socket_get_name() (see getpeername() and getsockname()) is called twice only in order to populate the $request->getServerParams() array with REMOTE_ADDR, REMOTE_PORT, SERVER_ADDR and SERVER_PORT. Are these really needed? (See also #174, #192 and others) The remote address could also be reused from the accept() syscall.
  • fwrite() (see sendto() and pselect6()) will only be called after the stream is reported as writable. We may use an immediate write if the stream is nonblocking. (See also https://github.com/reactphp/stream/pull/54)

clue avatar May 14 '22 12:05 clue

Do you have an estimation of how many requests per second or percentage increase this will yield?

WyriHaximus avatar May 14 '22 16:05 WyriHaximus

Still working out the details, but preliminary benchmark results suggest some noticeable improvements from 19810 req/s to 25032 req/s for the above example on my machine 🔥

clue avatar May 14 '22 16:05 clue

Nice, that is indeed worth eliminating them!

Something we could consider is making populating $request->getServerParams() optional through configuration.

WyriHaximus avatar May 15 '22 09:05 WyriHaximus

Closed via #457 and #467.

Remaining syscall optimizations will be done in stream and socket components and will be linked against this one.

clue avatar Aug 23 '22 12:08 clue