Reduce memory consumption for failed connections

[valga]

At first, some background on this. I have a long-running script that maintains persistence on some connections, so if connection was reset, it retries with an exponential backoff. Sometimes it runs without issues and sometimes it eats a lot of memory and even becomes unresponsive and is getting killed by watchdog. Apart from maintaining connections, my script just reads incoming data and writes it into a file, so there are no memory leaks in my code.

After a research, I found that timed out connections produce a lot of garbage, that can be cleaned only with GC. Here is a test scenario:

$loop = React\EventLoop\Factory::create();
$loop->addPeriodicTimer(1, function () use ($loop) {
$connector = new \React\Socket\Connector($loop, array(
'timeout' => 0.5,
'dns' => false,
'tls' => false,
'unix' => false,
));
// Use host and port combination that results in connection being timed out.$connector->connect('89.123.45.67:3128')
->then(function (\React\Socket\ConnectionInterface$stream) {
$stream->close();
printf("[+] %.3fMB\n", memory_get_usage() / 1024 / 1024);
})->otherwise(function (\Exception$e) {
printf("[-] %.3fMB (%s)\n", memory_get_usage() / 1024 / 1024, $e->getMessage());
});
});
$loop->addTimer(60, function () use ($loop) {
printf(
"%.3fMB => %d => %.3fMB\n",
memory_get_usage() / 1024 / 1024,
gc_collect_cycles(),
memory_get_usage() / 1024 / 1024);
$loop->stop();
});
$loop->run();

Result:

[-] 0.755MB (Timed out after 0.5 seconds)
[-] 0.786MB (Timed out after 0.5 seconds)
[-] 0.810MB (Timed out after 0.5 seconds)
...
[-] 2.114MB (Timed out after 0.5 seconds)
[-] 2.138MB (Timed out after 0.5 seconds)
[-] 2.162MB (Timed out after 0.5 seconds)
2.187MB => 9145 => 0.775MB

After doing more research, I found that is caused by promise cancellation: the more ->then() you have on cancellable promise, the more garbage it will leave after cancellation. So I tweaked code a bit and got this result:

[-] 0.749MB (Timed out after 0.5 seconds)
[-] 0.771MB (Timed out after 0.5 seconds)
[-] 0.787MB (Timed out after 0.5 seconds)
...
[-] 1.636MB (Timed out after 0.5 seconds)
[-] 1.651MB (Timed out after 0.5 seconds)
[-] 1.667MB (Timed out after 0.5 seconds)
1.684MB => 6195 => 0.761MB

Now we have 30% less garbage to collect. There is still a room for improvement, but it needs to be done with the Promise component.

[kelunik]

This should rather be fixed in react/promise, but the PR might be fine either way.

[valga]

@kelunik Even if it will be fixed in react/promise, I see no point in creating these 2 promises, because it is just a waste of resources.

[valga]

Ok, I found a problem with react/promise and it is bigger than I thought: basically you can't use exceptions as rejection reasons. See reactphp/promise#46 (comment) for more details on this.

[clue]

Thanks for filing this ticket and the elaborate discussion in the related tickets! 👍

I've updated to title to reflect the current state of the discussion and IMO it makes sense to get this in as-is. To sum this up, this component does not "leak" any memory, but reducing the memory consumption by reducing the number of circular dependencies is certainly a good thing 👍

[WyriHaximus]

LGTM

[clue]

@valga Thank you for raising this discussion! This obscure memory behavior should be resolved as part of #161 once these issues have been resolved in the upstream components