Lazy loading for arguments #429
Conversation
.gitignore Outdated
| phpcs.xml | ||
| phpstan.neon | ||
| vendor/ | ||
| .idea/ |
There was a problem hiding this comment.
Hi, for this is not project related you should add it to your global gitignore.
There was a problem hiding this comment.
Didn't know that existed, thanks!
| Ah, this looks promising! I have to head to work, but when I get back I'll pull your branch and run my profiler. Very excited to see the results... |
| phpcs.xml | ||
| phpstan.neon | ||
| vendor/ | ||
| vendor/ |
There was a problem hiding this comment.
we're missing a line feed
If you're using Jetbrains IDE, it can be automatically handled by Editor -> General -> Other
| } | ||
| | ||
| if (! is_array($config)) { | ||
| throw new \InvalidArgumentException('$config must be an array or a callable which returns such an array.'); |
There was a problem hiding this comment.
| throw new \InvalidArgumentException('$config must be an array or a callable which returns such an array.'); | |
| throw new \InvalidArgumentException('"$config" must be an array or a callable which returns such an array.'); |
| @mad-ice to move this issue along, you can add a test to https://github.com/webonyx/graphql-php/blob/master/tests/Type/DefinitionTest.php Check out https://github.com/webonyx/graphql-php/blob/master/CONTRIBUTING.md for more information on the dev tooling. Try running |
| public static function createMap(array $config) | ||
| public static function createMap($config) | ||
| { | ||
| if (is_callable($config)) { |
There was a problem hiding this comment.
Sorry for the long delay, but as promised I tried out your changes. So far I don't quite understand how this changes things; createMap is called by the FieldDefinition constructor, and then you just check to see if $config is callable, and if it is you immediately call it? Where do the savings come in?
I tested this by converting a few of the args on my top-level Query fields to functions, and stuck breakpoints inside them; they get called immediately.
What am I missing, here? Where does the "lazy" happen?
There was a problem hiding this comment.
I just POC'ed my own full lazy loading concept; so far seems to work pretty well. I'll try to put together a PR by the end of the week.
| Thanks for the PR. But we will likely go the path suggested in #557 which addresses an issue with lazy loading of individual types |
5 participants
Hi!
This pull request ensures that field arguments can be lazy loaded in the same manner as fields, which should improve performance quite a bit. I've ran a majorly tiny benchmark on the performance impact and found out that this change decreased our schema build time from 28ms to 3ms.
I'd like to get some help on testing this feature as I'm not familiar with maintaining automated tests and got lost trying to find the right place to add them.
And, of course, a big thank you for the amazing library!