Skip to content

Add parser and validation cache - #950

Open
hugo-vrijswijk wants to merge 1 commit into
mainfrom
feature/parser-and-validation-cache
Open

hugo-vrijswijk wants to merge 1 commit into
mainfrom
feature/parser-and-validation-cache

Conversation

@hugo-vrijswijk

Copy link
Copy Markdown
Contributor

Split QueryCompiler.compile into prepare (parsing and document-level validation, depends only on document text and schema) and compilePrepared (variable coercion, directive validation, elaboration, depends on the per-request Env).

Add CachingQueryCompiler and QueryCache, which cache the prepare result by document text, including parse failures. Default: 1024 documents, one-hour TTL, LRU eviction. Both are configurable, and callers can supply their own QueryCache.

Add docs for setup and configuration.

Split `QueryCompiler.compile` into `prepare` (parsing and document-level validation, depends only on document text and schema) and `compilePrepared` (variable coercion, directive validation, elaboration, depends on the per-request `Env`).

Add `CachingQueryCompiler` and `QueryCache`, which cache the `prepare` result by document text, including parse failures. Default: 1024 documents, one-hour TTL, LRU eviction. Both are configurable, and callers can supply their own `QueryCache`.

Add docs for setup and configuration.
@hugo-vrijswijk
hugo-vrijswijk force-pushed the feature/parser-and-validation-cache branch from a97019b to e1425f7 Compare September 15, 2026 15:12

@milessabin milessabin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm happy with the general idea, but I think the implementation here is too specific. Maybe move the concrete implementation out of core? It should be possible to back the cache with caffeine on the JVM, for instance, or Redis.

Also, using the raw document text as a cache key is problematic IME. Normalizing it first (ie. running it through the minimzer) and hashing is a better bet.

But I think this all belongs in separate, or even external modules ... there are many different choices you might make.

@hugo-vrijswijk

Copy link
Copy Markdown
Contributor Author

Good points, I initially leaned towards a serializable format too. But because of all the references to the schema and validations. The validation results could be dropped, but that is quite a large part of what is cached in this PR. Actual pluggable remote caches would be a big win.

For normalization, I agree this would be great. But the (current) QueryMinimizer first parses the document to its AST, and then minimizes from the AST. Which kind of defeats the point of the cache if every document still has to be parsed first. I can't really see a why to normalize without parsing. Dropping all whitespace would cause conflicts in different documents that get the same normalized output. For example, query { a b } would get the same cache hit as query { ab } while being very different documents.

I think different cache hits on different whitespace is fine. The cost is just a single reparse per document, and most applications will be built to send the same documents in most cases.

What we could do instead is only cache the parsed AST result as long as it is a Success/Warning/Failure (InternalError would need to serialize Throwable), on the (hash of) the document string, and don't cache any validation. If we provide a io.circe.Codec[Result[Ast.Document]] that would make remote caches easy to plug in to.

For a separate module, do you mean the in-memory implementation to be separate? Or the caching interface? Something like grackle-cache?

This branch has not been deployed

No deployments
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