Huge performance regression caused by new class finder package

[faizanakram99]

PR #664 causes huge performance regressions, earlier the class list was cached (using apcu or php files adapter), it isn't anymore. I initially thought it was due to the changes in symfony-bundle (https://github.com/thecodingmachine/graphqlite-bundle/pull/203#issuecomment-2139354656) but apparently the problem lies within the main repo.

See the difference here

Before (with graphqlite using thecodingmachine/class-explorer)

After (with latest graphqlite)

As you can see, earlier it was a 1 or 2 seconds for each request (see the main request and the ajax requests), now it is at least 45 seconds for each request.

I profiled it with xdebug and the regression comes from ComposerFinder (the second column shows time in seconds)

[fogrye]

Thx for your feedback, looks like it's related to #671

[faizanakram99]

Thx for your feedback, looks like it's related to #671

Not really, the namespace containing graphql stuff has very limited number of classes, that isn't taking time. ComposerFinder is being iterated over one too many times and each time it rewinds the iterator and loops over the whole thing (even with php files adapter cache enabled). It does this for every request (not just during cache warmup).

For now I've forked both this package and graphqlite symfony-bundle and reverted changes related to class loader, it works like charm.

[alekitto]

Hi all, I've investigated the issue a little bit. As already explained in https://github.com/alekitto/class-finder/issues/21 the impact of this issue is much lower in production environment thanks to the authoritative classmap generated by composer.

In all the other environments, I've found that the finder is iterated multiple times from TheCodingMachine\GraphQLite\SchemaFactory::createSchema. This behavior should be avoided as much as possible. Ideally one single iteration of the finder should be done at the beginning of the createSchema method (and cached in production environment) and use the produced class list in the subsequent operations.

For this purpose, I've created a ClassMapFinder class into the class-finder package (now in the master branch) and used in the SchemaFactory class in my fork (https://github.com/alekitto/graphqlite/commit/dfc71125754d6ea89c99cb95d69a6022416bb17a).

@faizanakram99 could you please test these modifications in your project and give me a feedback on the performance?

[oojacoboo]

@alekitto thanks for looking into this

[faizanakram99]

Thank you, I will try to test it this week.

[tasselchof]

Did anybody had a chance to test it? I've bumped into those performance issues in my project.

[faizanakram99]

@alekitto thanks, I can't get it working with https://github.com/fogrye/graphqlite-bundle , apparently the bundle references some constants which don't exist, I think some commits were made to thecodingmaching/graphqlite::master since v7.9 release which removed some stuff that the bundle (fogrye/graphqlite-bundle) still references and since your fork was created from master it also contains those changes making it incompatible with https://github.com/fogrye/graphqlite-bundle .

Not sure whats the best way forward here, maybe merge your changes and create a release (7.1) and pin graphqlite-bundle on graphqlite 7.1 with adjustments to code so that it doesn't reference those constants anymore, then only we can test it, @fogrye wdyt? or maybe there is a better alternative.

For now we solved it for our project by using a different implementation of Finder which just looks up src directory of our project https://github.com/fogrye/graphqlite-bundle/compare/master...QbilSoftware:graphqlite-bundle:master

[fogrye]

I finally have time to come back and look at this issue. Initially motivation for ComposerFinder was to let graphiqlite discover classes which are outside the src/ directory, but, unfortunately, I underestimated the performance impact (and I have no test project with large codebase to test it agains) so I propose a solution:

• in dev environment look for files under src as it was + use ComposerFinder but cache results outside of src/ with composer-lock.json content-hash as a key
• in prod environment it's ok to use ComposerFinder so I would live it unchanged with possibility to use ClassMapFinder when it will be in release

I'll try to implement those and push my PR to graphqlite-bundle