This PR is a WIP and a proof of concept. I have added a new _resolve method and added a test to compare the speed of this method compared to the current resolve method. The new _resolve method does not use Promise.all to resolve properties. Instead, it uses a basic for loop and done callback. This ensures resolvers that are not actually promises are not scheduled on the event loop.
When running the tests for the package you will see some console.time results. I purposefully left the test configure in a manner that favored syncResolvers. This should accurately illustrate the problem with assuming all resolvers are promises. But you can configure these settings however you would like.
Interestingly, even when using all asyncResolvers the _resolve method is faster. I have noticed this when writing similar updates for yupjs and feathers-fletching. I speculate its because the callback pattern can pop promises off the loop as it goes and Promise.all has to keep them all in the loop until they are done? Pure speculation.
The _resolve method passes all tests but one. You can rename it to resolve (comment/rename real resolve method). It fails one test around virtuals. I played with it and couldn't get it, but I am confident its solvable.
I am also curious if feathersjs/hooks could benefit from a similar treatment. I am not sure how it would work with the next-able hooks, but that may even make it easier. This would allow entire Feathers applications to get a performance boost if hooks like disallow, etc aren't consuming the event loop.
See: https://github.com/feathersjs/feathers/issues/3492
This PR is a WIP and a proof of concept. I have added a new
_resolve
method and added a test to compare the speed of this method compared to the currentresolve
method. The new_resolve
method does not usePromise.all
to resolve properties. Instead, it uses a basic for loop and done callback. This ensures resolvers that are not actually promises are not scheduled on the event loop.When running the tests for the package you will see some
console.time
results. I purposefully left the test configure in a manner that favoredsyncResolvers
. This should accurately illustrate the problem with assuming all resolvers are promises. But you can configure these settings however you would like.Interestingly, even when using all
asyncResolvers
the_resolve
method is faster. I have noticed this when writing similar updates for yupjs and feathers-fletching. I speculate its because the callback pattern can pop promises off the loop as it goes andPromise.all
has to keep them all in the loop until they are done? Pure speculation.The
_resolve
method passes all tests but one. You can rename it toresolve
(comment/rename real resolve method). It fails one test around virtuals. I played with it and couldn't get it, but I am confident its solvable.I am also curious if
feathersjs/hooks
could benefit from a similar treatment. I am not sure how it would work with the next-able hooks, but that may even make it easier. This would allow entire Feathers applications to get a performance boost if hooks likedisallow
, etc aren't consuming the event loop.