-
Notifications
You must be signed in to change notification settings - Fork 371
Check the GC behavior in Process Manager #448
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
Comments
Not sure what the actual problem is that you‘re trying to solve here. Typically there shouldn‘t be a need to interfere with the gc? |
The master process increase so much the memory used, even if the slaves, in my case, are memory stable. That means that if I measure the memory used by the slave after each request / response iteration, the memory doesn't increase (can increase during the request handle, but the server takes care about removing all unused references and variables). But the master is not doing that. Each request / response increases the memory used, and before the GC acts, we can have a 300Mb script running. And this is not good. Basically because this means that, and because as soon as the GC do its job, something is pending there without any type of usage. In that case, seems that some anonymous function is not doing this clean job, and that may cause lots of problems when the server is installed in a memory-protected environment. |
May be possible- but in this case we should rather find the culprit than forcing GC on top of it. If there are unfreed references though I'm wondering why/how GC would release them? |
Exactly. This is what I'm talking about. <?php
gc_disable();
class Test {
private $test;
function doSomething() {
$this->test = function () {
$a = 'Hello';
};
}
function doAnotherthing() {
// Anotherthing
}
}
while (true) {
$t = new Test;
$t->doSomething();
$t->doAnotherthing();
echo memory_get_usage(false) . PHP_EOL;
} The memory goes to infinite. Of course, after N iterations, and if we enable the GC, the memory is reduced and we would have a shark tail graphic. But look at this other example <?php
gc_disable();
class Test {
private $test;
function doSomething() {
$this->test = function () {
$a = 'Hello';
};
}
function doAnotherthing() {
$this->test = null;
}
}
while (true) {
$t = new Test;
$t->doSomething();
$t->doAnotherthing();
echo memory_get_usage(false) . PHP_EOL;
} The memory stays constant, no matter the behaviopr of the GC. |
After talking with @acasademont, this is not a memory leak, but a bad-treated reference in some anonymous function. After finding it, we will be able to have a more memory stable master thread. |
As we discussed privately with Marc, this is not really a leak but some reference that is "partially" released but not really released until GC kicks in. Might be quite difficult to debug. I'll double check memory consumption on our masters. |
We used to manually gc (1e2a705) but I'd really prefer to let php do it's job. |
Absolutely |
Closed via php-pm/php-pm-httpkernel#156 |
I've spent many hours by looking for a memory leak I have in my server that uses PM. After some hard review, and this won't probably be something for you guys, I noticed that the slaves can easily be memory friendly, but not the master, as tons of memory is used and rarely released.
Said that, and after some tests, I realized that adding a manual
gc_collect_cycles()
each time a request has been responsed, makes the work. This will be probably the fastest way to get over the problem, but not the nicest one, and because good code should never be related to external processes like CG, but ensure that the memory is released when is not longer needed, I think that we should take care of that (even if is not part of the project, but part of the project this one is built on top of).After these lines, I'd like to know if this is a persistant problem.
Do you have localized the parts of the ReactPHP code we could have memory leaks?
Do you have any examples of how to fix this? It is for sure related to anonymous functions, but I didn't encountered yet the point.
Thanks so much!
The text was updated successfully, but these errors were encountered: