Memory issue in v0.13

[Kocal]

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	yes/no
RFC?	yes/no
Version/Branch	0.13

Hi,

I'm still working on GraphQLBundle update from 0.11 to 0.13 on our big app at work, but I detected a memory issue while running our Behat suite tests.

It does not affect 0.11 and 0.12.

I've recorded htop output while running behat:

• 0.11, the VM used memory stays under 600 MB: https://streamable.com/sjrgq0
• 0.13, the memory increase quickly to ~2 GB, and makes PHP crash: https://streamable.com/qmcnfx

Do you have an idea of what can happens?
Thanks!

[mcg-web]

Hi,

InputValidator is the only big change between 0.12 and 0.13 but if you only updating you are not concern by this. Have you updated others dependencies (Symfony, Webonyx, ...) ?

[Kocal]

While updating from 0.11.11 to 0.13.3, the following dependencies has been updated:

• symfony/config, symfony/dependency-injection, symfony/event-dispatcher, symfony/filesystem, symfony/inflector, symfony/options-resolver, and symfony/property-access from 4.4.10 to 4.4.11
• symfony/event-dispatcher-contracts from 1.1.7 to 1.1.9
• symfony/polyfill-ctype from 1.17.1 to 1.18.1
• symfony/service-contracts from 2.1.2 to 2.1.3
• webonyx/graphql-php from 0.12.6 to 0.13.9

Then I downgraded to overblog/graphql-bundle from 0.13.3 to 0.12.8 but no other dependencies has been downgraded...

So it means that the issue comes from the bundle 😕

[Kocal]

I've done some tests today, and this is what I found.

The memory leak also happens when running Behat tests not on the GraphQL API. For example some classical tests on our controllers (ArticleController):

• 0.12.8:
  • features/api: 1.10GB max
  • features/articles: 1GB max
• 0.13.3:
  • features/api: increase to 2GB, then I stop the tests
  • features/articles: increase to 1.48 GB, then tests stop themselves (all successful)

I can confirm the issue is also present in 0.13.0, so something wrong should have happens in v0.12.8...v0.13.0

[akomm]

Hi @Kocal

Did the php version remain the same?

[Kocal]

Hi @akomm,

Oh my bad, I didn't say what version of PHP I'm using.
I'm using PHP 7.3.20 and it was the same version during my tests.

[akomm]

If neither your tests change, nor behat+its deps, the best bet would be to make a profile with xdebug.

A small chance also that some memory leak bug in the php version that was not triggered before is triggered after the changes, so if you swap the php version you might quickly exclude this case. Provided your dependencies allow for an increase to let's say 7.4 ...

[Kocal]

I did some blackfire traces:

• php 7.3.20:
  • graphqlbundle 0.12.8: https://blackfire.io/profiles/b1ff3280-1de8-4e44-af9e-8a685aba5086/graph
  • graphqlbundle 0.13.3: https://blackfire.io/profiles/4906123b-7be4-4a18-8244-96b0943ab4d5/graph
  • diff: https://blackfire.io/profiles/compare/1935c4ae-269e-4a9d-bcfd-a0a001b5f14c/graph +400% memory 😨
• php 7.4.9:
  • graphqlbundle 0.12.8: https://blackfire.io/profiles/53fc3c6e-c645-4670-856a-32c1a1976cde/graph
  • graphqlbundle 0.13.3: https://blackfire.io/profiles/e3ea21a3-d21f-4e4a-8edf-3ebd1b8e91d0/graph
  • diff: https://blackfire.io/profiles/compare/8d4986f2-9d0e-4574-a465-69bd2fd4cf4b/graph +~2% memory but it's not significative 🎉

I really don't know what can happens, but I guess we will have to update PHP from 7.3 to 7.4... 😬 ping @tristanbes / @RomulusED69

[murtukov]

@Kocal there are some reports in internet about memory leak in PHP 7.3

[akomm]

@murtukov yes, I have checked for this, but found only some in 7.3.2m which supposedly were fixed. However, I agree, as I also said myself, there might still be some I/you missed or some, which nobody found yet ;)

[akomm]

Actually, when I check the logs, there are dozens of memory leaks closed after 7.3.2: https://www.php.net/ChangeLog-7.php , including 7.4.0+

[akomm]

The diff php 7.3 is rather interesting. If you analyze / sort by memory usage, you find the main troublemaker is PhpFilesAdapter (symfony). It gives +211 MB by itself. Its callers down the stack each only give much smaller increase. But together they still sum up to the other +310 MB, which is significant.

This increase everywhere, rather than at the top of the (filtered) stack is a sign of there being a memory leak that affects everything. But PhpFilesAdapter is hit by it most.

A last note on this: leak for me is not only bug, but might also be accidentally caused by code or having some profiling enabled (symfony, debugger, etc.). PHP7.4 might just circumvent it with the new memory optimizations they have added. I also recall there was a feature adding some profiling for graphql in recent updates, just to name an example. Not sure though which version it was.

[theofidry]

Little update: we're migrating a project step by step and we hit a wall with 0.13. The test suite no longer runs due to huge memory leaks.

A standard test can see deviations as high as +280% of memory usages. It's a bit late so I'm gonna stop here but from what I could collect, the issue is not so much of a higher memory consumption, but rather a leakage, and I can trace source of the problem here.

If I replace the content with:

public function __construct($name, callable $compiler, ?callable $evaluator = null)
    {
        if (null === $evaluator) {
            $evaluator = static function () {};
        }

        parent::__construct($name, $compiler, $evaluator);
    }

Then the memory leak issue is gone.

My suspicion is that you're allocating an exception which can never be garbage collected resulting in the leak.

[mcg-web]

@theofidry is this the file you are talking about ? I think we can replace this exception by a warning. Maybe you have a better solution ?

[theofidry]

@mcg-web indeed; I just vendor hotfixed it to the old code (a static function throwing an exception) and it worked just fine. There is still a significant memory increase but not as much leak (which was a problem since quickly resulting in a bailout when running the test suite).

I am not exactly sure why, I assume because you made it an actual exception it probably captures the stack trace & everything. So maybe it's not much that the evaluator is not release (it's probably not release in any case), but more that the memory footprint is much bigger.