-
-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add schema cache implementation #1008
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, super interesting 👀
I think for the final version the cache should be configurable, similar to what we did we apq caching (driver, prefix [maybe?], ttl).
It will take be some time, so no ETA atm
src/SchemaCache.php
Outdated
public function __construct(Application $app, Cache $cache, Config $config) | ||
{ | ||
$this->app = $app; | ||
$this->cache = $cache; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This property isn't used, I guess the current file_(get|put)_contents and etc. will be changed to use it?
Thanks for the feedback! Fair point about the configurability of this cache. I will look into it soon. Besides, I experienced a problem with custom scalars that needs to be fixed too. |
Ok, thanks for your work on this! For now, I'll put the PR in draft mode! |
The issues with the scalars and types have been resolved now, by the new TypeConfigDecorator class. The pagination types are not defined via the config, so to resolve them consistently I have added an abstract parent. Custom or new pagination types need to extend this I have also made the schema cache configurable (I have updated the example in my original comment). By default the schema cache is disabled on local environment. It only needs to be refreshed if the schema config changes. |
Looks great. do we have any news about this feature? |
I'm waiting for enough free time to focus on it for the review: I think it's a super interesting feature! |
Yes Really good feature especially when schema and gql parsing consume time. my gql queries take by 1 sec. but sql queries fine by 30-50ms |
I wanted to take a look a bit around in the implementation, but due to recent changes in orchestra/testbench, unrelated to the PR, not all tests worked; therefore I merged master into it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't get it working in a private project because the first error I get is that it can't resolve a scalar from the cached schema.
I unfortunately can't share the code and right now I don't have time to build a minimal case, thus I try to share as much details as possible:
- The scalar is registered like any other type in the
config/graphql.php
, in the per-schema'types'
, it's registered like any regular type withApp\GraphQL\Scalars\Date::class
- it's not specific to that scalar; I've multiple scalars defined and when I started to comment out this one, the next one threw up. It's just too many scalars and usages so I couldn't quickly disable all of them to see if that's everything or not
- the schema works in the first request, where it's not yet fetched from the cache; but fails on the second
Stacktrace (with some obfuscation)
[2023-04-08T18:29:25.583680+00:00] local.ERROR: Undefined index: Date {"userId":XXXX,"exception":"[object] (ErrorException(code: 0): Undefined index: Date at /project/vendor/rebing/graphql-laravel/src/Support/SchemaCache/TypeConfigDecorator.php:163)
[stacktrace]
#0 /project/vendor/rebing/graphql-laravel/src/Support/SchemaCache/TypeConfigDecorator.php(163): Illuminate\\Foundation\\Bootstrap\\HandleExceptions->handleError(8, 'Undefined index...', '/project/ve...', 163, Array)
#1 /project/vendor/rebing/graphql-laravel/src/Support/SchemaCache/TypeConfigDecorator.php(40): Rebing\\GraphQL\\Support\\SchemaCache\\TypeConfigDecorator->decorateScalar(Array)
#2 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(265): Rebing\\GraphQL\\Support\\SchemaCache\\TypeConfigDecorator->__invoke(Array, Object(GraphQL\\Language\\AST\\ScalarTypeDefinitionNode), Array)
#3 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(215): GraphQL\\Utils\\ASTDefinitionBuilder->internalBuildType('Date', Object(GraphQL\\Language\\AST\\NamedTypeNode))
#4 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(196): GraphQL\\Utils\\ASTDefinitionBuilder->buildType(Object(GraphQL\\Language\\AST\\NamedTypeNode))
#5 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(193): GraphQL\\Utils\\ASTDefinitionBuilder->buildWrappedType(Object(GraphQL\\Language\\AST\\NamedTypeNode))
#6 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(140): GraphQL\\Utils\\ASTDefinitionBuilder->buildWrappedType(Object(GraphQL\\Language\\AST\\NonNullTypeNode))
#7 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(385): GraphQL\\Utils\\ASTDefinitionBuilder->makeInputValues(Object(GraphQL\\Language\\AST\\NodeList))
#8 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(361): GraphQL\\Utils\\ASTDefinitionBuilder->buildField(Object(GraphQL\\Language\\AST\\FieldDefinitionNode))
#9 /project/vendor/webonyx/graphql-php/src/Utils/ASTDefinitionBuilder.php(342): GraphQL\\Utils\\ASTDefinitionBuilder->makeFieldDefMap(Array)
#10 /project/vendor/rebing/graphql-laravel/src/Support/SchemaCache/TypeConfigDecorator.php(68): GraphQL\\Utils\\ASTDefinitionBuilder->GraphQL\\Utils\\{closure}()
#11 /project/vendor/webonyx/graphql-php/src/Type/Definition/FieldDefinition.php(119): Rebing\\GraphQL\\Support\\SchemaCache\\TypeConfigDecorator->Rebing\\GraphQL\\Support\\SchemaCache\\{closure}()
#12 /project/vendor/webonyx/graphql-php/src/Type/Definition/HasFieldsTypeImplementation.php(28): GraphQL\\Type\\Definition\\FieldDefinition::defineFieldMap(Object(GraphQL\\Type\\Definition\\ObjectType), Object(Closure))
#13 /project/vendor/webonyx/graphql-php/src/Type/Definition/HasFieldsTypeImplementation.php(69): GraphQL\\Type\\Definition\\ObjectType->initializeFields()
#14 /project/vendor/webonyx/graphql-php/src/Validator/Rules/OverlappingFieldsCanBeMerged.php(253): GraphQL\\Type\\Definition\\ObjectType->hasField('me')
#15 /project/vendor/webonyx/graphql-php/src/Validator/Rules/OverlappingFieldsCanBeMerged.php(162): GraphQL\\Validator\\Rules\\OverlappingFieldsCanBeMerged->internalCollectFieldsAndFragmentNames(Object(GraphQL\\Validator\\QueryValidationContext), Object(GraphQL\\Type\\Definition\\ObjectType), Object(GraphQL\\Language\\AST\\SelectionSetNode), Array, Array)
#16 /project/vendor/webonyx/graphql-php/src/Validator/Rules/OverlappingFieldsCanBeMerged.php(92): GraphQL\\Validator\\Rules\\OverlappingFieldsCanBeMerged->getFieldsAndFragmentNames(Object(GraphQL\\Validator\\QueryValidationContext), Object(GraphQL\\Type\\Definition\\ObjectType), Object(GraphQL\\Language\\AST\\SelectionSetNode))
#17 /project/vendor/webonyx/graphql-php/src/Validator/Rules/OverlappingFieldsCanBeMerged.php(60): GraphQL\\Validator\\Rules\\OverlappingFieldsCanBeMerged->findConflictsWithinSelectionSet(Object(GraphQL\\Validator\\QueryValidationContext), Object(GraphQL\\Type\\Definition\\ObjectType), Object(GraphQL\\Language\\AST\\SelectionSetNode))
#18 /project/vendor/webonyx/graphql-php/src/Language/Visitor.php(388): GraphQL\\Validator\\Rules\\OverlappingFieldsCanBeMerged->GraphQL\\Validator\\Rules\\{closure}(Object(GraphQL\\Language\\AST\\SelectionSetNode), 'selectionSet', Object(GraphQL\\Language\\AST\\OperationDefinitionNode), Array, Array)
#19 /project/vendor/webonyx/graphql-php/src/Language/Visitor.php(451): GraphQL\\Language\\Visitor::GraphQL\\Language\\{closure}(Object(GraphQL\\Language\\AST\\SelectionSetNode), 'selectionSet', Object(GraphQL\\Language\\AST\\OperationDefinitionNode), Array, Array)
#20 /project/vendor/webonyx/graphql-php/src/Language/Visitor.php(261): GraphQL\\Language\\Visitor::GraphQL\\Language\\{closure}(Object(GraphQL\\Language\\AST\\SelectionSetNode), 'selectionSet', Object(GraphQL\\Language\\AST\\OperationDefinitionNode), Array, Array)
#21 /project/vendor/webonyx/graphql-php/src/Validator/DocumentValidator.php(122): GraphQL\\Language\\Visitor::visit(Object(GraphQL\\Language\\AST\\DocumentNode), Array)
#22 /project/vendor/webonyx/graphql-php/src/GraphQL.php(149): GraphQL\\Validator\\DocumentValidator::validate(Object(GraphQL\\Type\\Schema), Object(GraphQL\\Language\\AST\\DocumentNode), Array)
#23 /project/vendor/webonyx/graphql-php/src/GraphQL.php(92): GraphQL\\GraphQL::promiseToExecute(Object(GraphQL\\Executor\\Promise\\Adapter\\SyncPromiseAdapter), Object(GraphQL\\Type\\Schema), Object(GraphQL\\Language\\AST\\DocumentNode), NULL, Object(App\\GraphQL\\Context), NULL, NULL, Array, NULL)
#24 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/GraphqlExecutionMiddleware.php(32): GraphQL\\GraphQL::executeQuery(Object(GraphQL\\Type\\Schema), Object(GraphQL\\Language\\AST\\DocumentNode), NULL, Object(App\\GraphQL\\Context), NULL, NULL, Array)
#25 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AbstractExecutionMiddleware.php(34): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\GraphqlExecutionMiddleware->handle('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, Object(App\\GraphQL\\Context), Object(Closure))
#26 /project/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AbstractExecutionMiddleware->resolve(Array, Object(Closure))
#27 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AbstractExecutionMiddleware.php(33): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Array)
#28 /project/app/GraphQL/Middleware/Execution/ContextValueMiddleppware.php(31): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AbstractExecutionMiddleware->Rebing\\GraphQL\\Support\\ExecutionMiddleware\\{closure}('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, Object(App\\GraphQL\\Context))
#29 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AbstractExecutionMiddleware.php(34): App\\GraphQL\\Middleware\\Execution\\ContextValueMiddleware->handle('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, Object(App\\GraphQL\\Context), Object(Closure))
#30 /project/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AbstractExecutionMiddleware->resolve(Array, Object(Closure))
#31 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AbstractExecutionMiddleware.php(33): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Array)
#32 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AutomaticPersistedQueriesMiddleware.php(42): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AbstractExecutionMiddleware->Rebing\\GraphQL\\Support\\ExecutionMiddleware\\{closure}('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, NULL)
#33 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AbstractExecutionMiddleware.php(34): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AutomaticPersistedQueriesMiddleware->handle('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, NULL, Object(Closure))
#34 /project/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AbstractExecutionMiddleware->resolve(Array, Object(Closure))
#35 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AbstractExecutionMiddleware.php(33): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Array)
#36 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/ValidateOperationParamsMiddleware.php(39): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AbstractExecutionMiddleware->Rebing\\GraphQL\\Support\\ExecutionMiddleware\\{closure}('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, NULL)
#37 /project/vendor/rebing/graphql-laravel/src/Support/ExecutionMiddleware/AbstractExecutionMiddleware.php(34): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\ValidateOperationParamsMiddleware->handle('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, NULL, Object(Closure))
#38 /project/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Rebing\\GraphQL\\Support\\ExecutionMiddleware\\AbstractExecutionMiddleware->resolve(Array, Object(Closure))
#39 /project/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}(Array)
#40 /project/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(115): Illuminate\\Pipeline\\Pipeline->then(Object(Closure))
#41 /project/vendor/rebing/graphql-laravel/src/GraphQL.php(184): Illuminate\\Pipeline\\Pipeline->thenReturn()
#42 /project/vendor/rebing/graphql-laravel/src/GraphQL.php(167): Rebing\\GraphQL\\GraphQL->executeViaMiddleware(Array, 'default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, NULL)
#43 /project/vendor/rebing/graphql-laravel/src/GraphQL.php(153): Rebing\\GraphQL\\GraphQL->executeAndReturnResult('default', Object(GraphQL\\Type\\Schema), Object(Rebing\\GraphQL\\Support\\OperationParams), NULL, NULL)
#44 /project/vendor/rebing/graphql-laravel/src/GraphQLController.php(41): Rebing\\GraphQL\\GraphQL->execute('default', Object(Rebing\\GraphQL\\Support\\OperationParams))
#45 /project/vendor/rebing/graphql-laravel/src/Helpers.php(25): Rebing\\GraphQL\\GraphQLController->Rebing\\GraphQL\\{closure}(Object(GraphQL\\Server\\OperationParams))
#46 /project/vendor/rebing/graphql-laravel/src/GraphQLController.php(38): Rebing\\GraphQL\\Helpers::applyEach(Object(Closure), Object(GraphQL\\Server\\OperationParams))
…
[cut off, outside of graphql here]
Spefifically, it's this code part throwing:
protected function decorateScalar(array $config): array
{
$className = $this->classMapping['types'][$config['astNode']->name->value];
It looks up Date
but the mappings are expressed as App\\GraphQL\Scalars\Date
.
HTH
6f8feb0
to
8ea785d
Compare
I've improved the mapping from name to type class, when not provided via the config. This mapping is also cached now. I needed to access the name of the queries/types, so I introduced a |
Thx, I hope this I'll find sooner time to test it! Was something wrong with my configuration or what would been a config which worked? 🤔 |
'types' => $this->config->get('graphql.types', []), | ||
]; | ||
|
||
foreach ($schemaConfig as $group => $types) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method neglects to consider we don't just have GraphQL types in the config. There are keys (in this case: $group
) which are middleware
, method
or execution_middleware
.
I think this code needs to only act on the specifics (query
, mutation
and types
).
Btw, this this also work when global types are defined?
FTR, in my case I now can't even load anything once schema caching is enabled:
[2023-04-14T19:45:30.293877+00:00] local.ERROR: Target class [name-of-middleware] does not exist. {"exception":"[object] (Illuminate\\Contracts\\Container\\BindingResolutionException(code: 0): Target class [name-of-middleware] does not exist. at /project/vendor/laravel/framework/src/Illuminate/Container/Container.php:879)
[stacktrace]
#0 /project/vendor/laravel/framework/src/Illuminate/Container/Container.php(758): Illuminate\\Container\\Container->build('name-of-middleware')
#1 /project/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(851): Illuminate\\Container\\Container->resolve('name-of-middleware', Array, true)
#2 /project/vendor/laravel/framework/src/Illuminate/Container/Container.php(694): Illuminate\\Foundation\\Application->resolve('name-of-middleware', Array)
#3 /project/vendor/laravel/framework/src/Illuminate/Foundation/Application.php(836): Illuminate\\Container\\Container->make('name-of-middleware', Array)
#4 /project/app/Services/Vendor/Illuminate/Foundation/Application.php(49): Illuminate\\Foundation\\Application->make('name-of-middleware', Array)
#5 /project/vendor/laravel/framework/src/Illuminate/Foundation/helpers.php(119): App\\Services\\Vendor\\Illuminate\\Foundation\\Application->make('name-of-middleware', Array)
#6 /project/vendor/rebing/graphql-laravel/src/Support/SchemaCache/SchemaCache.php(121): app('name-of-middleware')
…
(Note: there's a custom Application
at play, which however does not play a role for this issue)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have pushed a fix and test for this config issue! Global types were already supported, see the Upload type in the SchemaCacheTest class.
@sforward are you still interested in wrapping up? I hope to push the new major version out soon-ish. |
Absolutely, I missed your last feedback but will look into soon! |
The first impression I now got is that everything is working. I then did some very primitive benchmarking and did not see any improvements. Before proceeding I want to find out how to best properly measure this. My test is really simply:
Cache off, 100 iterations
Cache off, 1000 iterations
Cache on, 100 iterations
Cache on, 1000 iterations
It actually seems slower a bit. I made sure the cache is working, checked the cache key and it's a very big json blob so this part seems to work. I then tried using a different approach, copied the GraphQL request from the browser network tab as (async() => {
console.time();
for (i = 0; i < 1000; i++) {
var res = await fetch('https://host/api/graphql', {
// …
'body': '{"query":"query itsame {\\n me {\\n id\\n }\\n}","operationName":"itsame"}',
// …
});
console.log(await res.text());
}
console.timeEnd();
})();
🤷🏼I also have some failing tests in my private project to look into, but I first want to measure the improvement. A couple of years ago we had #390 where we did some benchmarks but there was no code-optimization-change outcome AFAIR. How did you benchmark it? |
I have not enough time to keep this going currently. I will close this PR for now, hopefully I find some time later this year. |
😭 sure thing! Let's all enjoy summer 🏖️ |
Summary
Currently, on every single request the schema is built by instantiating all types, queries, mutations, etc, and calling all
args
,fields
andtype
methods. This takes some time, especially when you have lots of queries and mutations. When investigating this issue, I came across a possible solution in de docs of webonyx/graphql-php. This works by caching the intermediate parsed representation of the schema to the filesystem. After loading from cache only the necessary classes will be resolved.The schema cache could be enabled via the config. By default it is disabled on local environment:
If the schema cache is enabled, it could be disabled per schema via the config:
The implementation comes with some breaking changes, so hopefully it's in time for v9. When this PR is approved, I will update the readme and the changelog.
Type of change:
Checklist:
composer fix-style