Feedback on Experimental Executor

[vladar]

Please post your feedback here on the new experimental executor introduced in #314

Especially interested in following questions:

1. Do you see a performance improvement on your queries? How much on average?
2. Do you experience any issues / unexpected behavior with it?

[jbtbnl]

I did a few queries with the experimental executor and didn't see any significant change in performance. Neither did I experience any unexpected behaviour.

[Graziel]

tried it, run my project tests no problems yet, dont see any noticeable performance gains but the tests says it runs a bit faster (just a bit)

[PowerKiKi]

Here are my observations after I tested the new executor.

I have 3 queries that are ridiculously slow and that I benchmark specifically for execution time. They are all very similar but query less and less fields (A is the biggest, C the smallest). To give you an idea of scale, the Query A generate a ~3MB JSON.

Name	Time before [s]	Time after [s]	Time %
Query A	1.584437	1.71129	108.01%
Query B	1.065754	1.053167	98.82%
Query C	0.711192	0.691833	97.28%

Surprisingly the new executor is becoming slower with huge queries.

Also in my regular test suite, I have 46 queries that are tested. Most of them are relatively simple, usually not going deeper that 3 fields deep. Those test execute real SQL queries, so there is quite a few variable things in the timing, but still:

• PHPUnit reported time before: ~3.6s
• PHPUnit reported time after: ~3.6s (very similar)
• Failing test after: 1

One of my query incorrectly relied on field evaluation order. I guess I could fix it rather easily, but seeing the mixed result of the new executor I might not do it straight away...

[zorji]

My project has to stick with 0.12.6 because the environment has to stay in PHP 7.0 due to legacy dependencies.

I have migrate some queries to GraphQL and recently found that one query in RESTful takes 600ms but GraphQL takes 1400ms.

• 0.12.6 in PHP 7.0: ~1400ms
• 0.12.6 in PHP 7.2: ~900ms
• dev-master in PHP 7.2: ~650ms (It is weird that the first few (less than 5) requests took around 1100ms, which is slower than 0.12.6, but after I rerun it, it dropped to ~650ms, I restart all my cache and even disabled my cache, it stayed in ~650ms)
• dev-master+useExperimentalExecutor in PHP 7.2: ~700ms

I am not quite sure what caused the changes above but I am going to fork a copy make it compatible with PHP 7.0 and try again.

Update

I've done the PHP 7.0 port in https://github.com/zorji/graphql-php/tree/php-70

• dev-master in PHP 7.0: ~1400ms
• dev-master+useExperimentalExecutor in PHP 7.0: ~1400ms

No noticeable difference.

[vladar]

Thanks!

[enumag]

Tried it on my project and the experimental executor caused many of my tests to fail. The reason is that some resolve functions in my generated types receive a Promise instead of the real value now. I might be able to fix this on my side but I'm not certain yet.

'resolve' => function ($value, $args, $context, ResolveInfo $info) {
    return $value->getId();
},

Error: Call to undefined method GraphQL\Executor\Promise\Promise::getId()

[enumag]

I tried to change the resolver to this:

'resolve' => function ($value, $args, $context, ResolveInfo $info) {
	return $value->then(
		function ($value) {
			return $value->getId();
		},
		function () {
			throw new \Exception();
		}
	);
},

but then I receive this error

GraphQL\Error\InvariantViolation : Expected a value of type "ID" but received: instance of GraphQL\Executor\Promise\Promise
.../vendor/webonyx/graphql-php/src/Experimental/Executor/CoroutineExecutor.php:524

I don't think I can fix this...

[jakubkulhan]

Hello, @enumag. Do you use a custom promise adapter? Could you provide a minimal reproducible code?

[enumag]

Do you use a custom promise adapter?

Not to my knwledge. I'm using the overblog bundle but I don't think it changes the promise adapter.

Could you provide a minimal reproducible code?

Considering how complicated my setup is this could easily take days to prepare. I don't have that much time.

[enumag]

Ah ok, yeah there is a custom promise adapter in one of the overblog libraries (and yes, I'm using this adapter). It's for DataLoaders: https://github.com/overblog/dataloader-php/blob/master/lib/promise-adapter/src/Adapter/WebonyxGraphQLSyncPromiseAdapter.php

[enumag]

@jakubkulhan Btw. can I ask what you're using instead of the overblog/dataloader-php package? I've always been under the impression that dataloaders in some form are kind of a necessity when implementing a GraphQL API because otherwise you'll run into the N+1 problem quite often, right?

[simPod]

@enumag I used to have my own "databuffer" but it was very clumsy. Recently rewrote to dataloader-php. I have my own modifications there but in general it's the best solution out there I guess.

[jakubkulhan]

@enumag I use custom buffer implementation based on GraphQL\Deferred.

[enumag]

Would either of you mind sharing your implementation? I don't really like the dataloader-php package. Configuration of dataloaders in overblog is very bad.

[simPod]

The executor needs to be documented tho

[vladar]

@jakubkulhan After having this issue opened for a while and other people trying it, we didn't get enough evidence that the new executor is significantly faster.

I propose to move it to its own package which you'd maintain. Don't see much point to keep two executors in the core. We can add it to the suggested packages and complementary tools section.

[jakubkulhan]

@vladar Ok. I'll create a new package and send a PR to remove the experimental executor.

[PowerKiKi]

I tried running my benchmark again with the old and new executor, just to see if things got magically better since last time. But again I don't see a significant speed increase unfortunately.

I really wish it was that simple to speed up my codebase 🤡

[spawnia]

For what it's worth, i just ran Lighthouse's test suite with the experimental executor without any errors. Seems like it is at least correct.

nuwave/lighthouse#1233

[jrots]

hi, sorry to be late with my comment on this but we're using webonyx/graphql quite intensively in a production environment (5000 requests per second) +/- 200 different types of graphql queries/mutations with the experimental executor.
I did see a significant change back when it was launched vs. the old one +/- 1.5 year ago
& just saw it was deprecated when I wanted to upgrade from 0.13.0 which is sad to see.
I don't see any other big peformance improvements in the changelog coming from 0.13 so I'd rather stick with this version..
If wanted I can give you some stats with and without, but might be too late which I understand -)

[vladar]

@jrots Thank you for the feedback! Can you compare the latest version with and without the experimental executor? Also, do you use Deferred resolvers extensively? We need to somehow understand the use-case when it actually performs better.