SdlSchemaPluginBase should cache AST instead of printing

So this does slightly reduce memory usage but it requires far more calls per request so it is going to have an impact of performance and it does not offer the savings of #3576164: Improve efficiency of cache usage in \Drupal\graphql\Plugin\GraphQL\Schema\SdlSchemaPluginBase::getSchema()

Test scenario:

1. Install graphql_example
2. Create an article
3. Create a graph server
4. Profile the following query:

query MyQuery {
  article(id: 1) {
    id
    title
  }
}

HEAD

Memory usage: 2.71MB
Function calls: 12540

THIS MR

Memory usage: 2.69MB
Function calls: 14100

#3576164

Memory usage: 2.65MB
Function calls: 12440

Larger schemas (more usual for a complex graphql site are going to push these numbers way way up.

So this code:

    // The base schema is built with the registry to ensure all resolvers are,
    // registered. SchemaExtender::extend must be used because in the PHP
    // implementation (contrary to the JS implementation) the BuildSchema::build
    // call does not properly process `extend` keywords in the SDL; and the
    // extender itself expects `Query`/`Mutation`/`Subscription` to be declared
    // exactly zero or one times.
    $schema = $this->buildSchema($document, $registry);
    $extensionAst = $this->getExtensionDocument($extensions);
    return $extensionAst
      ? SchemaExtender::extend($schema, $extensionAst)
      : $schema;

Means that we have to get both the extension and schema caches but I think the change to only cache the extension document and not the full document will be a good one. I think we we really need to address is the extra 1500 function calls. Thats a lot for the tiny schema exposed by the graphql_example and likely to spiral for larger schemas.

I've tested this change on a client site using graphql 5. The site has a well defined (and small) schema. This change results in nearly doubling the number of function calls per request from 86270 to 138430 calls - this is mostly clearly seen in the number of calls to \GraphQL\Language\Visitor::extractVisitFn - from 20340 to 37080. On the other hand we do see a memory usage reduction from 13.50MB to 12.07MB which reflects not loading the full cached schema twice.

Claude Code has a lot to say about this MR:

Concerns

1. Breaking change in the event API, no deprecation. AlterSchemaDataEvent and AlterSchemaExtensionDataEvent previously carried array (raw SDL, numerically indexed — note $schemaData[0]). Now they carry array (ASTs, keyed by plugin ID). Every downstream subscriber that manipulated SDL via str_replace/regex (exactly the pattern the old test subscriber demonstrated) is silently broken at runtime — wrong types going through visitors that assume strings, or implode on objects. Since 5.x is still pre-release (the tip of 5.x is 5.0.0-beta2), a hard break may be acceptable, but it warrants an entry in the upgrade notes / CHANGELOG and ideally a mention in the commit message.
2. getSchemaDefinition() behavior change (SdlSchemaPluginBase.php:292–303). Previously returned NULL on file_get_contents failure or empty file (via ?: NULL). Now throws InvalidPluginDefinitionException with message "Failed to read schema file". Two nits:
- if (!$contents) conflates false (read error) with "" (empty but readable file). Prefer if ($contents === FALSE) and let an empty file parse (empty is already invalid SDL and will fail downstream with a clearer error) — or at least have the message reflect both possibilities.
- The return type hint is still string but old callers expecting nullable behavior are now getting exceptions. Fine in context (old ?: NULL was ignored by the caller anyway), but worth a line in the docblock noting the throw.
3. Stale docblock on getCacheId() (SdlSchemaPluginBase.php:310): "The cache type, e.g. 'schema' or 'full'." — 'full' no longer exists. Should be 'schema' or 'extension'.
4. Typo in the new comment (SdlSchemaPluginBase.php:137): "all resolvers are,\n // registered" — should be a period, not a comma.
5. Redundant array_filter (SdlSchemaPluginBase.php:208–210, 248–250): the inner array_map already returns NULL for empty/null schemas, so the outer array_filter(..., fn ($d) => !empty($d)) is redundant. Not wrong, just cruft.
6. Null caching for extensions (SdlSchemaPluginBase.php:260–264): when there are no extensions, $ast is NULL and still written to the cache. Works correctly on reads (if ($cache = $this->astCache->get(...)) yields the CacheItem regardless of $cache->data), but it's subtle. Either add a comment or skip the set() when $ast === NULL.
7. Test subscriber comment copy-paste (GraphQlSubscriber.php:103): says "Once we've visited the position field node we can stop" inside the alterSchemaData method that's actually matching id. Minor, test-only.
8. Visitor::stop() after first match: the test subscriber stops on first match of position/id. Fine for this fixture, but serves as an example for module authors — some callers might copy it and be surprised when a second matching node is skipped. Worth a sentence in the example explaining why stopping is safe here.
9. No user-facing documentation update. There's nothing in doc/ mentioning these events, and I'd expect a short migration note given #1. The test subscriber is now the canonical example — fine, but hard to discover.

What looks good

- Removing the print→parse round trip is a clean win; the new flow matches what graphql-php itself expects.
- Indexing the events by plugin ID is a genuine improvement — subscribers can now target a specific plugin's contribution rather than hunting through numerically indexed entries.
- Demonstrating AST visitors in the example subscriber is the right pedagogy.
- assumeValid gated on !$inDevelopment is consistent with the existing pattern for BuildSchema::build.
- Cache IDs and tags (graphql) are consistent, and the per-server server_id suffix is preserved for ConfigurableInterface schemas.

Suggested follow-ups before merge

- Fix the comment typo (SdlSchemaPluginBase.php:137) and the stale getCacheId docblock.
- Tighten if (!$contents) to if ($contents === FALSE) or adjust the error message.
- Drop the redundant outer array_filter calls.
- Add a CHANGELOG/upgrade note flagging the event payload type change.
- Optionally skip astCache->set() when the extension AST is NULL, or add a comment explaining it's intentional.

I did some performance benchmarking and unforuntately this change introduces a performance regression. The getSchema() method takes 60% longer to complete.

5.x branch:
Results
- min: 26.770 ms
- avg: 41.025 ms
- median: 39.083 ms
- p95: 61.782 ms
- max: 67.663 ms
- total: 1230.755 ms

MR 71
Results
- min: 42.585 ms
- avg: 65.877 ms
- median: 65.930 ms
- p95: 73.875 ms
- max: 82.449 ms
- total: 1976.322 ms

Generated test script attached.

I need to instrument getSchema() in more detail to find the bottleneck.

I added some instrumenting lines and tracked it down to SchemaExtender::extend():

getSchema() step breakdown (avg / min / max over 30 measured runs)
Step                               avg ms     min ms     max ms
----------------------------------------------------------------
getExtensions                       0.238      0.197      0.324
getSchemaDocument                  12.884     10.740     14.122
registerResolvers                   2.952      1.351     29.227
buildSchema                         1.107      0.137     28.319
getExtensionDocument                5.012      4.248      5.515
SchemaExtender_extend              27.858      5.451     39.303

27ms just to extend the schema on every request is quite bad.

We really need to cache the full generated Schema object instead if possible. I think that is not possible right now because of anonymous function callbacks in the resolver registry. For that we have #3576071: Deprecate Callback Resolver.

Wild idea: could we automatically dump the anon callbacks into a php file? Probably not because they can interact with their surrounding environment? I'm thinking if we could wrap those resolvers any other way to be able to cache the schema.

Thanks, with this approach I see now a performance improvement of 30% now, testing a grapqhl request through the kernel on jobiqo:

5.x:
Results
- min: 25.066 ms
- avg: 35.403 ms
- median: 35.934 ms
- p95: 39.490 ms
- max: 41.056 ms
- total: 1062.103 ms

MR 71:
Results
- min: 13.302 ms
- avg: 24.005 ms
- median: 23.025 ms
- p95: 38.401 ms
- max: 40.128 ms
- total: 720.142 ms

I will run this through our full test suite now as well.