Skip to content

Commit 300674b

Browse files
committed
Broken PoC of AST caching on Disk
Issue #1312 provided some good pointers of performance considerations. One of which was not parsing the schema on every request. We've previously tried to cache the AST in the database but the representation just isn't very well suited for it. Instead we should use the PhpStorage provided by Drupal to just cache the entire AST as an array on disk which can be loaded quickly. See https://webonyx.github.io/graphql-php/schema-definition-language/#performance-considerations This commit makes an example proof of concept of what this could look like but it has a few problems: - For some reason the extensions (e.g. `extend type Query`) don't get parsed properly which makes our validator think the field has a resolver but no schema. This might be a bug in the validator but that's unclear for now. - We make breaking changes to methods in SdlShcemaPluginBase which could be undesirable and might need some thought and care to make this forward compatible. - There's no good way to get an AST from a Schema so we perform some hacks. However it looks like the underlying library actually loses some of our Source information. The whole point of processing each .graphql file indepdently was so we could get better error messages of where syntax errors exist. This might be fixed in newer versions of the graphql-php library though - We must perform an extension sorting step, at least with the Open Social schema, to make sure that module dependencies are properly handled. For example a .base.graphql file might not `extend` something but might still reference a type defined in another module's .base.graphql file. If the files are processed out of order then this causes an error. Even if this is fixed with the sorting by module dependencies then this might still be a breaking change for people who do not currently build their schema with these constraints in mind (i.e. properly architecting their schema's dependencies).
1 parent bf9dea3 commit 300674b

File tree

4 files changed

+157
-59
lines changed

4 files changed

+157
-59
lines changed

graphql.services.yml

+8
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ services:
5454
- { name: cache.bin }
5555
factory: cache_factory:get
5656
arguments: [graphql_ast]
57+
deprecated: 'The "%service_id%" service is deprecated. There is no replacement. See https://drupal.org/node/'
58+
59+
# File cache for the complete GraphQL document.
60+
# TODO: We don't benefit from auto-cachebin uninstall and need to implement clean-up
61+
cache.graphql.document:
62+
class: Drupal\Component\PhpStorage\PhpStorageInterface
63+
factory: Drupal\Core\PhpStorage\PhpStorageFactory::get
64+
arguments: [graphql_document]
5765

5866
# Cache bin for graphql plugin definitions.
5967
cache.graphql.definitions:

src/Plugin/GraphQL/Schema/AlterableComposableSchema.php

+13-13
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public static function create(ContainerInterface $container, array $configuratio
5959
* The plugin id.
6060
* @param array $pluginDefinition
6161
* The plugin definition array.
62-
* @param \Drupal\Core\Cache\CacheBackendInterface $astCache
62+
* @param \Drupal\Core\Cache\CacheBackendInterface $documentCache
6363
* The cache bin for caching the parsed SDL.
6464
* @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler
6565
* The module handler service.
@@ -73,20 +73,20 @@ public static function create(ContainerInterface $container, array $configuratio
7373
* @codeCoverageIgnore
7474
*/
7575
public function __construct(
76-
array $configuration,
77-
$pluginId,
78-
array $pluginDefinition,
79-
CacheBackendInterface $astCache,
80-
ModuleHandlerInterface $moduleHandler,
81-
SchemaExtensionPluginManager $extensionManager,
82-
array $config,
76+
array $configuration,
77+
$pluginId,
78+
array $pluginDefinition,
79+
CacheBackendInterface $documentCache,
80+
ModuleHandlerInterface $moduleHandler,
81+
SchemaExtensionPluginManager $extensionManager,
82+
array $config,
8383
ContainerAwareEventDispatcher $dispatcher
8484
) {
8585
parent::__construct(
8686
$configuration,
8787
$pluginId,
8888
$pluginDefinition,
89-
$astCache,
89+
$documentCache,
9090
$moduleHandler,
9191
$extensionManager,
9292
$config
@@ -114,7 +114,7 @@ public function __construct(
114114
protected function getSchemaDocument(array $extensions = []) {
115115
// Only use caching of the parsed document if we aren't in development mode.
116116
$cid = "schema:{$this->getPluginId()}";
117-
if (empty($this->inDevelopment) && $cache = $this->astCache->get($cid)) {
117+
if (empty($this->inDevelopment) && $cache = $this->documentCache->get($cid)) {
118118
return $cache->data;
119119
}
120120

@@ -132,7 +132,7 @@ protected function getSchemaDocument(array $extensions = []) {
132132
);
133133
$ast = Parser::parse(implode("\n\n", $event->getSchemaData()));
134134
if (empty($this->inDevelopment)) {
135-
$this->astCache->set($cid, $ast, CacheBackendInterface::CACHE_PERMANENT, ['graphql']);
135+
$this->documentCache->set($cid, $ast, CacheBackendInterface::CACHE_PERMANENT, ['graphql']);
136136
}
137137
return $ast;
138138
}
@@ -156,7 +156,7 @@ protected function getSchemaDocument(array $extensions = []) {
156156
protected function getExtensionDocument(array $extensions = []) {
157157
// Only use caching of the parsed document if we aren't in development mode.
158158
$cid = "extension:{$this->getPluginId()}";
159-
if (empty($this->inDevelopment) && $cache = $this->astCache->get($cid)) {
159+
if (empty($this->inDevelopment) && $cache = $this->documentCache->get($cid)) {
160160
return $cache->data;
161161
}
162162

@@ -174,7 +174,7 @@ protected function getExtensionDocument(array $extensions = []) {
174174
);
175175
$ast = !empty($extensions) ? Parser::parse(implode("\n\n", $event->getSchemaExtensionData())) : NULL;
176176
if (empty($this->inDevelopment)) {
177-
$this->astCache->set($cid, $ast, CacheBackendInterface::CACHE_PERMANENT, ['graphql']);
177+
$this->documentCache->set($cid, $ast, CacheBackendInterface::CACHE_PERMANENT, ['graphql']);
178178
}
179179

180180
return $ast;

src/Plugin/GraphQL/Schema/SdlSchemaPluginBase.php

+126-46
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Drupal\graphql\Plugin\GraphQL\Schema;
44

5+
use Drupal\Component\PhpStorage\PhpStorageInterface;
56
use Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException;
67
use Drupal\Component\Plugin\PluginBase;
78
use Drupal\Core\Cache\CacheableDependencyInterface;
@@ -13,12 +14,21 @@
1314
use Drupal\graphql\Plugin\SchemaExtensionPluginInterface;
1415
use Drupal\graphql\Plugin\SchemaExtensionPluginManager;
1516
use Drupal\graphql\Plugin\SchemaPluginInterface;
17+
use GraphQL\Error\Error;
18+
use GraphQL\Language\AST\DocumentNode;
1619
use GraphQL\Language\AST\InterfaceTypeDefinitionNode;
20+
use GraphQL\Language\AST\Node;
21+
use GraphQL\Language\AST\NodeList;
1722
use GraphQL\Language\AST\TypeDefinitionNode;
1823
use GraphQL\Language\AST\UnionTypeDefinitionNode;
1924
use GraphQL\Language\Parser;
25+
use GraphQL\Language\Source;
26+
use GraphQL\Type\Definition\Type;
27+
use GraphQL\Type\Schema;
28+
use GraphQL\Utils\AST;
2029
use GraphQL\Utils\BuildSchema;
2130
use GraphQL\Utils\SchemaExtender;
31+
use GraphQL\Utils\SchemaPrinter;
2232
use Symfony\Component\DependencyInjection\ContainerInterface;
2333

2434
/**
@@ -28,11 +38,11 @@ abstract class SdlSchemaPluginBase extends PluginBase implements SchemaPluginInt
2838
use RefinableCacheableDependencyTrait;
2939

3040
/**
31-
* The cache bin for caching the parsed SDL.
41+
* The file cache for caching the parsed SDL.
3242
*
33-
* @var \Drupal\Core\Cache\CacheBackendInterface
43+
* @var \Drupal\Component\PhpStorage\PhpStorageInterface
3444
*/
35-
protected $astCache;
45+
protected $documentCache;
3646

3747
/**
3848
* Whether the system is currently in development mode.
@@ -65,7 +75,7 @@ public static function create(ContainerInterface $container, array $configuratio
6575
$configuration,
6676
$plugin_id,
6777
$plugin_definition,
68-
$container->get('cache.graphql.ast'),
78+
$container->get('cache.graphql.document'),
6979
$container->get('module_handler'),
7080
$container->get('plugin.manager.graphql.schema_extension'),
7181
$container->getParameter('graphql.config')
@@ -81,8 +91,8 @@ public static function create(ContainerInterface $container, array $configuratio
8191
* The plugin id.
8292
* @param array $pluginDefinition
8393
* The plugin definition array.
84-
* @param \Drupal\Core\Cache\CacheBackendInterface $astCache
85-
* The cache bin for caching the parsed SDL.
94+
* @param \Drupal\Component\PhpStorage\PhpStorageInterface $documentCache
95+
* The file cache for caching the parsed SDL.
8696
* @param \Drupal\Core\Extension\ModuleHandlerInterface $moduleHandler
8797
* The module handler service.
8898
* @param \Drupal\graphql\Plugin\SchemaExtensionPluginManager $extensionManager
@@ -93,17 +103,17 @@ public static function create(ContainerInterface $container, array $configuratio
93103
* @codeCoverageIgnore
94104
*/
95105
public function __construct(
96-
array $configuration,
97-
$pluginId,
98-
array $pluginDefinition,
99-
CacheBackendInterface $astCache,
100-
ModuleHandlerInterface $moduleHandler,
106+
array $configuration,
107+
$pluginId,
108+
array $pluginDefinition,
109+
PhpStorageInterface $documentCache,
110+
ModuleHandlerInterface $moduleHandler,
101111
SchemaExtensionPluginManager $extensionManager,
102-
array $config
112+
array $config
103113
) {
104114
parent::__construct($configuration, $pluginId, $pluginDefinition);
105115
$this->inDevelopment = !empty($config['development']);
106-
$this->astCache = $astCache;
116+
$this->documentCache = $documentCache;
107117
$this->extensionManager = $extensionManager;
108118
$this->moduleHandler = $moduleHandler;
109119
}
@@ -117,29 +127,68 @@ public function __construct(
117127
*/
118128
public function getSchema(ResolverRegistryInterface $registry) {
119129
$extensions = $this->getExtensions();
130+
131+
// TODO: The fact that the registry gets extended here means we can't cache
132+
// all of it easily
133+
foreach ($extensions as $extension) {
134+
$extension->registerResolvers($registry);
135+
}
136+
137+
// TODO: We might need to think about the naming to work with multiple webheads and configs?
120138
$resolver = [$registry, 'resolveType'];
121-
$document = $this->getSchemaDocument($extensions);
122-
$schema = BuildSchema::build($document, function ($config, TypeDefinitionNode $type) use ($resolver) {
139+
$document = empty($this->inDevelopment) ? $this->loadCachedDocument() : NULL;
140+
if ($document !== NULL) {
141+
return BuildSchema::build($document, function ($config, TypeDefinitionNode $type) use ($resolver) {
142+
if ($type instanceof InterfaceTypeDefinitionNode || $type instanceof UnionTypeDefinitionNode) {
143+
$config['resolveType'] = $resolver;
144+
}
145+
146+
return $config;
147+
});
148+
}
149+
150+
$schema = BuildSchema::build($this->getBaseSchemaAst(), function ($config, TypeDefinitionNode $type) use ($resolver) {
123151
if ($type instanceof InterfaceTypeDefinitionNode || $type instanceof UnionTypeDefinitionNode) {
124152
$config['resolveType'] = $resolver;
125153
}
126154

127155
return $config;
128156
});
129157

130-
if (empty($extensions)) {
131-
return $schema;
158+
/** @var DocumentNode $extensionAst */
159+
foreach ($this->getExtensionAsts($extensions) as $extensionAst) {
160+
$schema = SchemaExtender::extend($schema, $extensionAst);
132161
}
133162

134-
foreach ($extensions as $extension) {
135-
$extension->registerResolvers($registry);
163+
// This does a full schema load, which is slow. But it's the most correct
164+
// way to ensure we can get a cacheable AST that's quick.
165+
// TODO: Move this to a drush command that can precalculate this.
166+
$schemaDefinitions = [$schema->getAstNode(), ...$schema->extensionASTNodes];
167+
foreach ($schema->getTypeMap() as $type) {
168+
if ($type->astNode !== NULL) {
169+
$schemaDefinitions[] = $type->astNode;
170+
}
171+
foreach ($type->extensionASTNodes ?? [] as $extensionNode) {
172+
$schemaDefinitions[] = $extensionNode;
173+
}
136174
}
175+
$ast = new DocumentNode(
176+
['definitions' => new NodeList($schemaDefinitions)]
177+
);
178+
$this->storeCachedDocument($ast);
179+
180+
return $schema;
181+
}
137182

138-
if ($extendSchema = $this->getExtensionDocument($extensions)) {
139-
return SchemaExtender::extend($schema, $extendSchema);
183+
private function loadCachedDocument() : ?Node {
184+
if ($this->documentCache->load($this->getPluginId())) {
185+
return AST::fromArray(__do_get_schema());
140186
}
187+
return NULL;
188+
}
141189

142-
return $schema;
190+
private function storeCachedDocument(Node $document) : bool {
191+
return $this->documentCache->save($this->getPluginId(), "<?php\nfunction __do_get_schema() { return " . var_export(AST::toArray($document), true) . "; }");
143192
}
144193

145194
/**
@@ -149,6 +198,41 @@ protected function getExtensions() {
149198
return $this->extensionManager->getExtensions($this->getPluginId());
150199
}
151200

201+
protected function getBaseSchemaAst() {
202+
$base_source = $this->getSchemaDefinition();
203+
if (is_string($base_source)) {
204+
@trigger_error("Returning a string from " . __CLASS__ . "::getSchemaDefinition is deprecated. Return an instance of Source or NULL instead.");
205+
$base_source = new Source($base_source, __CLASS__);
206+
}
207+
return Parser::parse($base_source ?? "");
208+
}
209+
210+
protected function getExtensionAsts(array $extensions = []) {
211+
$extension_base_asts = [];
212+
$extension_extend_asts = [];
213+
foreach ($extensions as $id => $extension) {
214+
$base_definition = $extension->getBaseDefinition();
215+
if (is_string($base_definition)) {
216+
@trigger_error("Returning a string from " . get_class($extension) . "::getBaseDefinition is deprecated. Return an instance of Source or NULL instead.");
217+
$base_definition = new Source($base_definition, $id . "_base");
218+
}
219+
if ($base_definition !== NULL) {
220+
$extension_base_asts[] = Parser::parse($base_definition);
221+
}
222+
223+
$extend_definition = $extension->getExtensionDefinition();
224+
if (is_string($extend_definition)) {
225+
@trigger_error("Returning a string from " . get_class($extension) . "::getExtensionDefinition is deprecated. Return an instance of Source or NULL instead.");
226+
$extend_definition = new Source($extend_definition, $id . "_extend");
227+
}
228+
if ($extend_definition !== NULL) {
229+
$extension_extend_asts[] = Parser::parse($extend_definition);
230+
}
231+
}
232+
233+
return [...$extension_base_asts, ...$extension_extend_asts];
234+
}
235+
152236
/**
153237
* Retrieves the parsed AST of the schema definition.
154238
*
@@ -161,23 +245,27 @@ protected function getExtensions() {
161245
* @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
162246
*/
163247
protected function getSchemaDocument(array $extensions = []) {
164-
// Only use caching of the parsed document if we aren't in development mode.
165-
$cid = "schema:{$this->getPluginId()}";
166-
if (empty($this->inDevelopment) && $cache = $this->astCache->get($cid)) {
167-
return $cache->data;
168-
}
169248

170-
$extensions = array_filter(array_map(function (SchemaExtensionPluginInterface $extension) {
171-
return $extension->getBaseDefinition();
172-
}, $extensions), function ($definition) {
173-
return !empty($definition);
174-
});
249+
$extensions = array_filter(
250+
array_map(
251+
function (SchemaExtensionPluginInterface $extension) {
252+
$definition = $extension->getBaseDefinition();
253+
if (is_string($definition)) {
254+
@trigger_error("Returning a string from " . get_class($extension) . "::getBaseDefinition is deprecated. Return an instance of Source or NULL instead.");
255+
$definition = new Source($definition);
256+
}
257+
return $definition;
258+
},
259+
$extensions
260+
),
261+
function ($definition) {
262+
return !empty($definition);
263+
}
264+
);
265+
175266

176267
$schema = array_merge([$this->getSchemaDefinition()], $extensions);
177268
$ast = Parser::parse(implode("\n\n", $schema));
178-
if (empty($this->inDevelopment)) {
179-
$this->astCache->set($cid, $ast, CacheBackendInterface::CACHE_PERMANENT, ['graphql']);
180-
}
181269

182270
return $ast;
183271
}
@@ -193,30 +281,21 @@ protected function getSchemaDocument(array $extensions = []) {
193281
* @throws \GraphQL\Error\SyntaxError
194282
*/
195283
protected function getExtensionDocument(array $extensions = []) {
196-
// Only use caching of the parsed document if we aren't in development mode.
197-
$cid = "extension:{$this->getPluginId()}";
198-
if (empty($this->inDevelopment) && $cache = $this->astCache->get($cid)) {
199-
return $cache->data;
200-
}
201-
202284
$extensions = array_filter(array_map(function (SchemaExtensionPluginInterface $extension) {
203285
return $extension->getExtensionDefinition();
204286
}, $extensions), function ($definition) {
205287
return !empty($definition);
206288
});
207289

208290
$ast = !empty($extensions) ? Parser::parse(implode("\n\n", $extensions)) : NULL;
209-
if (empty($this->inDevelopment)) {
210-
$this->astCache->set($cid, $ast, CacheBackendInterface::CACHE_PERMANENT, ['graphql']);
211-
}
212291

213292
return $ast;
214293
}
215294

216295
/**
217296
* Retrieves the raw schema definition string.
218297
*
219-
* @return string
298+
* @return \GraphQL\Language\Source|string
220299
* The schema definition.
221300
*
222301
* @throws \Drupal\Component\Plugin\Exception\InvalidPluginDefinitionException
@@ -236,7 +315,8 @@ protected function getSchemaDefinition() {
236315
$module->getName(), $path, $definition['class']));
237316
}
238317

239-
return file_get_contents($file) ?: NULL;
318+
$contents = file_get_contents($file);
319+
return $contents ? new Source($contents, $file) : NULL;
240320
}
241321

242322
}

src/Plugin/SchemaExtensionPluginManager.php

+10
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ public function getExtensions($id) {
7878
}, array_filter($this->getDefinitions(), function ($definition) use ($id) {
7979
return $definition['schema'] === $id;
8080
}));
81+
82+
// Sort the definitions by module weight which should respect dependencies
83+
// between modules to ensure extensions requiring other extensions are
84+
// processed in the right order.
85+
// We use $2 <=> $1 to make sure we're from dependency -> dependee.
86+
$module_data = \Drupal::service('extension.list.module')->getList();
87+
uasort(
88+
$this->extensions[$id],
89+
fn (SchemaExtensionPluginInterface $definition1, SchemaExtensionPluginInterface $definition2) => $module_data[$this->extractProviderFromDefinition($definition2->getPluginDefinition())]->sort <=> $module_data[$this->extractProviderFromDefinition($definition1->getPluginDefinition())]->sort
90+
);
8191
}
8292

8393
return $this->extensions[$id];

0 commit comments

Comments
 (0)