perf: implement request parsing result caching #192
Conversation
mervyn-mccreight
changed the title
mervyn-mccreight
changed the title
| fun parseValueLiteral(isConst: Boolean): ValueNode { | ||
| val token = lexer.token | ||
| internal fun Lexer.parseValueLiteral(isConst: Boolean): ValueNode { | ||
| val token = token |
There was a problem hiding this comment.
This looks very confusing, maybe you can reword assignments like this.
| } | ||
|
|
||
| /** | ||
| * Converts a name lex token into a name parse node. | ||
| */ | ||
| private fun parseName(): NameNode { | ||
| private fun Lexer.parseName(): NameNode { |
There was a problem hiding this comment.
Seeing all these extension functions somehow feels strange (especially since I wouldn't necessarily consider them semantically belonging to a lexer) but I get where this comes from. Not sure if creating a new Parser instance would be so bad but I'm fine either way - haven't looked at the Parser/Lexer too closely so far.
| import kotlin.reflect.KClass | ||
|
|
||
| class DefaultSchema( | ||
| override val configuration: SchemaConfiguration, | ||
| internal val model: SchemaModel | ||
| ) : Schema, __Schema by model, LookupSchema { | ||
| ) : Schema, __Schema by model, LookupSchema, CoroutineScope { |
There was a problem hiding this comment.
Feels a bit strange that a schema also is a coroutine scope but that can either be just me or a bad name for DefaultSchema 😉 (but I wouldn't suggest renaming the class now)
| fun `calls function once on multiple invocations with same input`() = runTest { | ||
| val memoized = memoize(this, 1, ::slowPlusOne) | ||
| repeat(2) { assertEquals(2, memoized(1)) } | ||
| assertEquals(slowFunctionDuration.inWholeMilliseconds, testScheduler.currentTime) |
There was a problem hiding this comment.
| assertEquals(slowFunctionDuration.inWholeMilliseconds, testScheduler.currentTime) | |
| // Only one delay of [slowFunctionDuration] should have happened, indicating that | |
| // `slowPlusOne` was executed only once | |
| assertEquals(slowFunctionDuration.inWholeMilliseconds, testScheduler.currentTime) |
Maybe a comment like that helps for future understanding
| @@ -14,7 +14,7 @@ import java.util.concurrent.TimeUnit | |||
| @State(Scope.Benchmark) | |||
| @Warmup(iterations = 10) | |||
| @Measurement(iterations = 5) | |||
| @OutputTimeUnit(TimeUnit.MILLISECONDS) | |||
| @OutputTimeUnit(TimeUnit.SECONDS) | |||
| open class RequestCachingBenchmark { | |||
There was a problem hiding this comment.
I know this is old code but it looks like this benchmark is primarily measuring the execution, not the parsing. Can we have a dedicated benchmark for parsing (as well), maybe with different kinds of requests?
No description provided.