Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance, reuse server params for same connection #467

Merged
merged 1 commit into from
Aug 22, 2022

Conversation

clue
Copy link
Member

@clue clue commented Aug 22, 2022

This PR improves the HTTP server performance by reusing the socket addressses stored in the server params for the same connection. This helps avoiding unneeded getpeername() and getsockname() syscalls that show a noticeable performance impact especially during benchmark runs (see also #455). This change is somewhat similar to the Clock introduced to avoid gettimeofday() syscalls in #457.

For a single HTTP request, no syscalls can be eliminated, but for any further request over the same connection (Connection: keep-alive), these syscalls can be eliminated:

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
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 is especially noticeable during a benchmark run. In particular, the HTTP server may handle any number of requests over a single connection, so the socket addresses will only be checked once and reused for all following requests. Running docker run -it --rm --net=host jordi/ab -n1000000 -c50 -k http://localhost:8080/ against the example HTTP server suggests results improved from 22624 req/s to 25301 req/s on my machine (best of 5 each).

This changeset only refactors 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.

Together with #457, this closes #455

@clue clue added this to the v1.7.0 milestone Aug 22, 2022
@clue clue requested a review from WyriHaximus August 22, 2022 16:57
@WyriHaximus WyriHaximus merged commit 00e481e into reactphp:1.x Aug 22, 2022
@clue clue deleted the reuse-address branch August 23, 2022 06:35
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Feb 19, 2024
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.

Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Feb 19, 2024
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.

Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Feb 19, 2024
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.

Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 8, 2024
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.

Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve performance by avoiding unneeded syscalls
2 participants