Repository navigation
Add parser cache - #950
Add parser cache#950hugo-vrijswijk wants to merge 1 commit into
Conversation
a97019b to
e1425f7
Compare
milessabin
left a comment
There was a problem hiding this comment.
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.
|
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) 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 For a separate module, do you mean the in-memory implementation to be separate? Or the caching interface? Something like |
0e68e24 to
51f1b05
Compare
51f1b05 to
50a5f51
Compare
|
@milessabin I've simplified this PR quite a bit to add a cache that only caches parsing. Either for successes or failures. I've also done some performance checks with it, and though it speeds up parsing a bit with in-memory (the validation from before is pretty fast), remote caches are almost never worth it. Regardless, it is probably best to keep support for it open. Caching is still in the Some rough benchmarks below. "Remote" is a valkey server running on the same machine. Times are in µs per request:
|
Adds a `parse` and `compileParsed` to `QueryCompiler`. Add `CachingQueryCompiler` and `QueryCache`, which cache the `parse` result by document text, including parse failures. Default: 1024 documents, LRU eviction. Both are configurable, and callers can supply their own `QueryCache`. `ParsedDocument` is a plain data representation of the parse result, with `Codec` instances to store it in an external store. Add docs for setup and configuration.
50a5f51 to
c196504
Compare
Adds a
parseandcompileParsedtoQueryCompiler.Add
CachingQueryCompilerandQueryCache, which cache theparseresult by document text, including parse failures. Default: 1024 documents, LRU eviction. Both are configurable, and callers can supply their ownQueryCache.ParsedDocumentis a plain data representation of the parse result, withCodecinstances to store it in an external store.Add docs for setup and configuration.