reactphp / stream

Event-driven readable and writable streams for non-blocking I/O in ReactPHP.
https://reactphp.org/stream/
MIT License
618 stars 62 forks source link

Avoid unneeded syscall when creating non-blocking `DuplexResourceStream` #164

Closed clue closed 2 years ago

clue commented 2 years ago

This simple PR removes an unneeded syscall when creating a non-blocking DuplexResourceStream. The current logic separates the readable and writable side and accordingly sets non-blocking mode twice even if it references the exact same file descriptor. This changeset suggests avoiding the first call for the DuplexResourceStream, as the non-blocking mode will always be set for its writable side through the WritableResourceStream anyway.

This change does not affect performance in any noticeable way, but helps avoid unneeded syscalls in an attempt to ease debugging and improve overall performance (see https://github.com/reactphp/http/issues/455). For a single HTTP request, this means the following syscalls can be eliminated:

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)    = …

This changeset only removes some internal logic and does not affect the public API, so it should be safe to apply. The test suite confirms this has 100% code coverage.

Refs https://github.com/reactphp/http/issues/455 and https://github.com/reactphp/socket/pull/292