amphp / http-client

An advanced async HTTP client library for PHP, enabling efficient, non-blocking, and concurrent requests and responses.
https://amphp.org/http-client
MIT License
706 stars 66 forks source link

10x+ memory usage in https vs http #280

Closed keiviv closed 4 years ago

keiviv commented 4 years ago

PHP 7.4.12 / Amphp 4.5.2

This is not a leak, but a stable excessive (10x+) memory usage in https vs http. There should be some overhead, but 10x+ is likely due to something wrong and very avoidable.

This is the minimal code to replicate the problem (make sure there is no cache):

use Amp\Http\Client\HttpClientBuilder;
use Amp\Http\Client\Request;
use Amp\Loop;

Loop::run(function () {
    /**
     *   http      2-4 MB RAM, depending on OS.
     *   https   40-48 MB RAM, depending on OS
     *
     *   Try both below:
     *   (example.com is an actual working server)
     */

    $url = 'https://example.com';

    $client   = HttpClientBuilder::buildDefault();
    $promise  = $client->request(new Request($url));
    $response = yield $promise;
    $body     = yield $response->getBody()->buffer();

    echo 'Mem: ' . memory_get_usage(true) / 1048576 . ' MB';
});
kelunik commented 4 years ago

This is caused by the lookup table in amphp/hpack. Enable FFI and install nghttp2 or enable opcache, so the lookup table stays in shared memory.

keiviv commented 4 years ago

Cache solves almost everything, that's why I suggested to test without it — hoping the problem under the hood can be fixed. Nether React, or Guz, or any other established async client has this issue.

I like Amp more than any of those, but this isn't right.

kelunik commented 4 years ago

Curl implements hpack natively. React doesn't support HTTP/2 at all. Amp can use native nghttp2, too, if you enable FFI, but falls back to a quite large lookup table. If you're able to optimize that lookup table, a PR is welcome.

You can also disable HTTP/2, then no lookup table will be present in memory.

keiviv commented 4 years ago

I am not able to optimize even my sleep schedule, kelunik — hoped this was an easier problem — some dev leftovers etc. If it cannot be fixed with ease, hell with it then.

trowski commented 4 years ago

The HPack lookup table is quite large. It is generated only once and kept in memory for performance. There's not really a way around this to implement HTTP/2 in pure PHP, it's a necessary evil. A few MB of RAM is a reasonable trade-off for the performance gain. As @kelunik suggested, the memory use can be avoided with FFI or opcache.

keiviv commented 4 years ago

I got it now that it's not an easy problem to solve without cache, just didn't know it before posting. Besides the old Artax project of yours, which is natively HTTP/1.1, what's the way to disable HTTP/2 in here?

keiviv commented 4 years ago

I mean I can just comment $protocols[] = 'h2' out it in DefaultConnectionFactory.php, but I seem to miss an API option if there is one. I'm testing 12 clients at the moment, might have missed it, sorry. Is there one?

Have to run it on a micro-setup, where FFI-to-nghttp2 or memory cache is not an option, sadly.

kelunik commented 4 years ago

You can set the protocol versions on the request to ['1.1'] only.

keiviv commented 4 years ago

Right, inprivate $protocolVersions = ['1.1', '2']. Not an API, but smarter. Thank you, @kelunik

Still testing, but literally nothing out there is faster than Amphp, by a huge margin. On my tests (20-100 concurrent sets) you are 2.5x (avrg) times faster than the next best client. You guys are simply rockstars!

And this quick HTTP protocol fix can solve quirks even in such rare cases like mine. Closing this, with huge respect.

trowski commented 4 years ago

The protocol can be set on each request using Request::setProtocolVersions(['1.0', '1.1']) if you'd like to exclude HTTP/2. You can do this for each request or create an ApplicationInterceptor that does this and attach it when building the client.

keiviv commented 4 years ago

Oh. Thank you, all the APIs of the clients I'm testing atm sort of merged into one messy blob, and I've missed this one. Huge thanks!

trowski commented 4 years ago

Sorry, meant ApplicationInterceptor. Updated my comment.

Oh. Thank you, all the APIs of the clients I'm testing atm sort of merged into one messy blob, and I've missed this one. Huge thanks!

No problem, you're welcome! The number of options you can tweak is actually a bit overwhelming, it's easy to miss things.

keiviv commented 4 years ago

<3 @kelunik @trowski & everyone contributing — you rock hard!

trowski commented 4 years ago

In fact, you don't even need to create an entire interceptor, you can use ModifyRequest:

$httpClientBuilder->intercept(new ModifyRequest(function (Request $request): Request {
    $request->setProtocolVersions(['1.0', '1.1']);
    return $request;
}));
keiviv commented 4 years ago

So simple!