Skip to content

Conversation

@theofidry
Copy link
Member

No description provided.

@theofidry
Copy link
Member Author

@kelunik sorry to ping you directly I could not find anything about it.

I'm noticing two issues regarding amphp/parallel-functions in this upgrade:

  • in the current state (prior to this PR or with this PR at the time of writing), compiling PHP-Scoper with Box with parallel enabled (which is a very simple usage), I get something like:
// Memory usage: 67.97MB (peak: 394.46MB), time: 36secs

I didn't remember it being that slow, but fair enough.

Now, without parallel:

// Memory usage: 39.95MB (peak: 96.11MB), time: 6secs

Notice that the peak memory usage is x4 lower and the build x6 faster. This looks pretty suspicious.

  • If to this PR I add the readonly keyword to almost any property, I get:
PHP Fatal error:  Allowed memory size of 2147483648 bytes exhausted (tried to allocate 15728672 bytes) in /Users/path/to/php-scoper/vendor/amphp/parallel/lib/Sync/functions.php on line 49

Needless to say it does not matter the amount of memory I allocate: it just fills it up all right away. I also said it's about any property but it's a bit incorrect: this is about properties for which the class is loaded during a parallel process. So I suspect it has to do with serialization/sleep/wakeup but can't pin it neither find a relevant thread in the PHP bug list.

(I am on PHP 8.1.7)

@kelunik
Copy link

kelunik commented Oct 13, 2022

@theofidry Everything exchanged between the parent and child processes will be serialized. What's $compactors / $compactors->getScoperSymbolsRegistry()?

@theofidry
Copy link
Member Author

theofidry commented Oct 13, 2022

Among others: anything under Configuration, Autoload, Patcher...

Everything exchanged between the parent and child processes will be serialized

This is not really a surprise, but surely, a class with a readonly property can be serialized?

@kelunik
Copy link

kelunik commented Oct 13, 2022

Yes, that should be fine: https://3v4l.org/LXNM4/perf

I'm not sure what could be causing issues with readonly there.

@theofidry
Copy link
Member Author

I think I found a lead

@theofidry
Copy link
Member Author

Ok not 100% sure yet. I think however this is due to a massive memory leak caused by the exceptions. Something along the lines of: a failure happens in the worker due to the readonly property, the exception is caught and parallel continues trying to process everything to then aggregate the errors. It's likely in this case though that the exception payload is so big, it never manages to finish as it runs out of memory before that

@kelunik
Copy link

kelunik commented Oct 13, 2022

What of you set zend.exception_ignore_args? The arguments in the stack might contribute a lot there.

@theofidry
Copy link
Member Author

The value is set to Off

@theofidry
Copy link
Member Author

I'll merge this PR for now, will try to push for more changes in another one with a deeper investigation on how to fix this issue

@theofidry theofidry merged commit ee08e09 into humbug:main Oct 13, 2022
@theofidry theofidry deleted the feature/rector branch October 13, 2022 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants