search

youshido-php / GraphQLBundle

Pure PHP implementation of GraphQL Server – Symfony Bundle
MIT License
284 stars 44 forks source link

Memory consumption in Symfony 4.3 #77

Closed florentdestremau closed 5 years ago

florentdestremau commented 5 years ago

I was trying to move to 4.3 from 4.2 and the memory consumption of this bundle is through the roof. Does anyone has any idea what could cause this ?

Pirokiko commented 5 years ago

I have also experienced this. According to the symfony profiler the pre-resolve and post-resolve events are where the memory consumption is high.

I have a request being made for a report with multiple colums and rows for about 8 different sections of the report, so we sent a decent amount of objects through the graphql layer. Not sure how many objects it is, but the json response, formatted so we can read it, had over 12000 lines. This used up about 600 MB of memory.

Next part is purely speculation : If i take the memory report of symfony at face value, i suspect the memory consumption comes from the type checking of the response. Possibly keeping all response objects as a copy in memory before releasing everything at the end.

Pirokiko commented 5 years ago

I have done some further research and have found the issue. In Symfony 4.3 the EventDispatchers dispatch method has a different signature and now throws a deprecation warning. On large data sets this means that this deprecation warning is triggered a lot of times and this is what is eating up memory.

My before mentioned data set has this deprecation warning thrown a total 28460 times.

The graphql.pre_resolve and graphql.post_resolve events are thrown in the youshido/graphql-bundle/Execution/Processer.php class. Updating these dispatch events to the new style, reduced the memory consumption tremendously, back to only 2 Mb for the same request.

florentdestremau commented 5 years ago

This is actually hilarious, very glad you took the time to find a fix ! How can we make this PR merged and update packagist ?

florentdestremau commented 5 years ago

ping @viniychuk how do you feel about #78 ?

florentdestremau commented 5 years ago

@portey looks like you have write access, can you do something about this PR ? This bundle looks pretty much abandoned so I'm wondering if we should simply fork it or if there is any utility in trying to make things move.

viniychuk commented 5 years ago

Hey guys, apparently I wasn't receiving any notifications to my emails... merging.

florentdestremau commented 5 years ago

Thanks @viniychuk ! Is this going to be available through packagist ?