Allow synchronous execution of hooks without async to increase the whole performance

[MurzNN]

At now all hooks are executed via await calls:

keystone/packages-next/keystone/src/lib/core/mutations/create-update.ts

Line 238 in a3b07ea

await runSideEffectOnlyHook(list, 'beforeChange', args, shouldCallFieldLevelSideEffectHook);

and

keystone/packages-next/keystone/src/lib/core/mutations/hooks.ts

Line 58 in a3b07ea

await field.hooks[hookName]?.({ fieldPath: fieldKey, ...args });

that always breaks current execution via process.nextTick(), as I understand, slowing down the whole process as result.

So, for example, even if we create many beforeChange() hooks as sync functions (without async), all of them will be called using await, which waste many CPU ticks.

Maybe implement some checker, that will check the type of each hook function and executes it synchronously, if it not return the promise?

As I understand, this trick will greatly improve performance of synchronous hooks calls, right?

[molomby]

I'd be surprised if this made much difference to be honest and certainly wouldn't want to invest time in it without something demonstrating an actual problem. Have you run any benchmarks to validate your assumptions? My guess would be that other factors (network latency, DB IO, etc.) would far outweigh the loss of a few cycles due to async invocation (if it even occurs).

Also, keep in mind that the generated GraphQL API (and associated hook system, etc.) is intended to facilitate the Admin UI and as a starting point for general app dev. If you're worried about performance at this level of detail, you should probably be looking to move more critical operations to custom mutations where you have significantly more control.

[MurzNN]

I didn't run any real benchmarks, but my point is that on each selection query the hook function executed multiple times, for example if we select 100 Post items with querying relationship fields from Author and Tags fields, even the one hook function will be executed at lease 300 times (100 per each loaded item, if each item have only one hook function, but it can have per-field hooks too).

And I'm very surprised that process.nextTick() can produce delays for about 50-100 ms even on an almost empty event loop, so if we have a very filled event loop, those delays can be much more.

So we need to produce some tests on high-load app to be sure how much will be profit of this improvement. I'll try to do this in some near future, and report here the results.

[molotow11]

It would be also great to execute all the hooks in Queue, because when it is all processed at once we have just a mess in logging