Skip to content

Conversation

@rstata
Copy link

@rstata rstata commented Dec 20, 2025

Defines a new GraphQLSchema builder class that is more restrictive than the existing one but reduces time and space requirements by more than a factor of five. See Issue #4196 for details.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request introduces GraphQLSchema.FastBuilder, a high-performance alternative to the standard GraphQLSchema.Builder that achieves ~7x faster build times and 85% reduction in memory allocation for large schemas by eliminating multiple full-schema traversals.

Key Changes:

  • New GraphQLSchema.FastBuilder class that requires explicit type registration and eliminates traversal-based type collection
  • ShallowTypeRefCollector for efficient type reference resolution without deep traversal
  • FindDetachedTypes utility to identify types not reachable from root types
  • FastSchemaGenerator that uses FastBuilder for improved performance
  • Comprehensive test coverage including comparison tests against standard builder

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
GraphQLSchema.java Adds FastBuilder inner class with streamlined schema construction
ShallowTypeRefCollector.java New utility for collecting and resolving type references without recursive traversal
FindDetachedTypes.java New utility for identifying detached types via DFS from roots
FastSchemaGenerator.java Schema generator implementation using FastBuilder
SchemaGeneratorHelper.java Adds getTypes() method to expose built types
Assert.java Optimizes name validation by replacing regex with character-by-character validation
FindDetachedTypesTest.groovy 1017 lines of tests for detached type detection
FastBuilderTest.groovy 2048 lines of tests for FastBuilder functionality
FastBuilderComparison*.groovy Multiple test files comparing FastBuilder with standard builder
CreateSchemaBenchmark.java Adds benchmark for FastBuilder performance
BuildSchemaBenchmark.java New benchmark isolating schema building from parsing
build.gradle Adds JMH configuration options

Comment on lines 1027 to 1029
if (!(unwrapped instanceof GraphQLNamedType)) {
return this;
}
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to test whether an instance of GraphQLUnmodifiedType is also an instance of GraphQLNamedType - it always is.

Suggested change
if (!(unwrapped instanceof GraphQLNamedType)) {
return this;
}

Copilot uses AI. Check for mistakes.
@andimarek
Copy link
Member

hey @rstata ... thanks a lot of this PR.

I think the fundamental idea and your use case makes a lot of sense and the implementation also looks acceptable broadly.

Is this PR shape what would solve your problem? Do you want to work through this PR to get it merged or you would prefer opening a new one?

@rstata
Copy link
Author

rstata commented Jan 8, 2026

This PR works for us. We would use the GraphQLSchema.FastBuilder directly because we'd want to use it in conjunction with a (fast to read) binary-schema representation we've developed for Viaduct.

You should take a look at the following:

/**
 * A schema generator that uses GraphQLSchema.FastBuilder for improved performance.
 * This is intended for benchmarking and performance testing purposes.
 */
@Internal
public class FastSchemaGenerator {

This class converts a TypeDefinitionRegistry into a GraphQLSchema using the FastBuilder. On the one hand, this could be of interest to the broader GraphQL Java community. On the other hand, the Viaduct use case isn't for executable schemas (our initial use cases are for build-time use cases) and i'm reluctant to vouch for the correctness of the wiring part. Maybe we merge with the above limitation, and then remove that limitation if there's demand and also more confidence?

Raymie Stata and others added 11 commits January 11, 2026 08:20
…er check

Replace Pattern.matcher().matches() with direct character validation in
Assert.assertValidName(). This method is called in the constructor of every
GraphQL type (objects, interfaces, unions, enums, inputs, scalars), field,
argument, and directive during schema construction.

For large schemas (18k+ types), this eliminates tens of thousands of regex
compilations and matches, providing significant performance improvement:
- FastBuilder: 25% faster (364ms → 272ms on 18,837-type schema)
- Standard Builder: 4% faster (1967ms → 1883ms on same schema)

The character-by-character check maintains identical validation semantics
while avoiding regex Pattern compilation and Matcher object allocation
overhead on every name validation.
Add GraphQLSchema.FastBuilder, a high-performance schema builder that
avoids full-schema traversals. This initial implementation includes:

- FastBuilder class with constructor accepting code registry builder
  and root types (query, mutation, subscription)
- New private GraphQLSchema constructor for FastBuilder
- ShallowTypeRefCollector stub class for future type reference handling
- Support for adding scalar types via additionalType()
- Automatic addition of built-in directives
- Optional validation support (disabled by default)
- Comprehensive test suite in FastBuilderTest.groovy

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Implement directive handling in FastBuilder with support for type
references in directive arguments:

- Move ShallowTypeRefCollector to graphql.schema package (for access to
  package-private replaceType methods)
- Implement additionalDirective() with duplicate detection
- Implement ShallowTypeRefCollector.handleDirective() to scan arguments
- Implement type reference resolution for directive arguments
- Support List and NonNull wrapped type references
- Throw AssertException for missing type references
- Add comprehensive tests for directive type reference resolution

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Enumeration types work with the existing additionalType() implementation.
Add tests to verify:

- Enum types can be added to schema
- Enum type matches standard builder output
- Directive arguments can reference enum types via GraphQLTypeReference

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Extend ShallowTypeRefCollector to handle GraphQLInputObjectType:

- Scan input object fields for type references
- Implement replaceInputFieldType() for field type resolution
- Support nested input types via type references
- Support List and NonNull wrapped type references in input fields
- Add comprehensive tests for input object type handling

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Extend ShallowTypeRefCollector to handle applied directives:

- Add scanAppliedDirectives() method for scanning applied directive args
- Scan applied directives on directive container types (enum, scalar, etc.)
- Scan applied directives on input object fields
- Implement replaceAppliedDirectiveArgumentType() for type resolution
- Update FastBuilder.withSchemaAppliedDirective() to scan for type refs
- Add comprehensive tests for applied directive type reference resolution

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Extended ShallowTypeRefCollector to handle GraphQLObjectType:
  - Scans field types for type references
  - Scans field arguments for type references
  - Scans applied directives on fields
  - Scans interfaces for type references
- Added resolveOutputType() for output type reference resolution
- Added replaceFieldType() for field type replacement
- Added ObjectInterfaceReplaceTarget wrapper and replaceObjectInterfaces()
- Updated FastBuilder.additionalType() to maintain interface→implementations map
- Added comprehensive tests for object type handling:
  - Field type reference resolution (direct, NonNull, List)
  - Interface type reference resolution
  - Interface→implementations map building
  - Field argument type reference resolution
  - Applied directive on fields
  - Error cases for missing types/interfaces

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Extended ShallowTypeRefCollector to handle GraphQLInterfaceType:
  - Scans field types for type references
  - Scans field arguments for type references
  - Scans applied directives on fields
  - Scans extended interfaces for type references
- Added handleInterfaceType() method
- Added InterfaceInterfaceReplaceTarget wrapper class
- Added replaceInterfaceInterfaces() for interface extension resolution
- Updated FastBuilder.additionalType() to wire type resolvers from interfaces
- Added comprehensive tests for interface type handling:
  - Basic interface type addition
  - Field type reference resolution
  - Interface extending interface via type reference
  - Type resolver wiring from interface
  - Field argument type reference resolution
  - Applied directive on interface fields
  - Error cases for missing extended interfaces

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Extended ShallowTypeRefCollector to handle GraphQLUnionType:
  - Scans possible types for type references
  - Applied directives already handled by directive container scan
- Added handleUnionType() method
- Added UnionTypesReplaceTarget wrapper class
- Added replaceUnionTypes() for union member type resolution
- Updated FastBuilder.additionalType() to wire type resolvers from unions
- Added comprehensive tests for union type handling:
  - Basic union type addition
  - Union member type reference resolution
  - Type resolver wiring from union
  - Error cases for missing member types
  - Applied directive on union types

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
- Fixed additionalTypes not being set in FastBuilder path:
  - Added additionalTypes parameter to FastBuilder private constructor
  - Added buildAdditionalTypes() method to compute additionalTypes from typeMap
  - Validation now properly traverses all types in the schema
- Added comprehensive tests for validation and edge cases:
  - withValidation(false) skips validation
  - withValidation(true) runs validation and catches errors
  - Circular type reference resolution
  - Deeply nested type reference resolution (NonNull + List + NonNull)
  - Complex schema with interfaces, unions, input types
  - Null handling for types/directives
  - Built-in directives are added automatically

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Add BuildSchemaBenchmark to measure schema construction performance in isolation
from SDL parsing. This benchmark compares standard SchemaGenerator against
FastSchemaGenerator using the large-schema-4.graphqls test schema (~18,800 types).

Changes:
- Add FastSchemaGenerator that uses FastBuilder for optimized schema
  construction from pre-parsed TypeDefinitionRegistry
- Add BuildSchemaBenchmark.java with side-by-side comparison of standard vs fast
  schema generation, parsing SDL once in @setup to isolate build performance
- Add jmhProfilers gradle property support for GC profiling (e.g., -PjmhProfilers="gc")
- Add FastSchemaGeneratorTest to verify equivalence with standard SchemaGenerator

Usage:
  ./gradlew jmh -PjmhInclude=".*BuildSchemaBenchmark.*"
  ./gradlew jmh -PjmhInclude=".*BuildSchemaBenchmark.*" -PjmhProfilers="gc"
A reviewer pointed out an unnecessary check was being performed,
the commit removes it.  Also, in re-reading that code
I noticed that the behavior of `Builder.additionalType` and
`FastBuilder.additionalType` were subtly different.  To prevent
potential confusion, this commit renames it to `addType` and
documents its behavior more explicitly.
@andimarek
Copy link
Member

@rstata one thought: we don't need the FindDetachedTypes traversal I think. Additionally types don't have to be restricted, but can contain all types (minus the root types of query/mutation/subscription)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants