Skip to content

Commit 2520867

Browse files
feat(batching): avoid calculating document ast height in advance (#1800)
### 📝 Description Batching by LEVEL requires to calculate the height of all graphql documents in a batch request to be calculated before deciding of a certain level was dispatched, the reason is because a batch request could contain multiple and potentially different operations, with different levels, so, to decide if a level was dispatched, we need to know which operations in the batch request share the same level. This PR will remove that logic, which was take it from the graphql-java [MaxDepthInstrumentation](https://github.com/graphql-java/graphql-java/blob/master/src/main/java/graphql/analysis/MaxQueryDepthInstrumentation.java#L56). Instead we will calculate the height on runtime, simplifying the dispatch by level instrumentation and removing some CPU usage by not doing an AST traversal to calculate height.
1 parent bb183b6 commit 2520867

File tree

6 files changed

+47
-73
lines changed

6 files changed

+47
-73
lines changed

executions/graphql-kotlin-dataloader-instrumentation/build.gradle.kts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ tasks {
2020
limit {
2121
counter = "INSTRUCTION"
2222
value = "COVEREDRATIO"
23-
minimum = "0.93".toBigDecimal()
23+
minimum = "0.92".toBigDecimal()
2424
}
2525
limit {
2626
counter = "BRANCH"

executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/extensions/ExecutionContextExtensions.kt

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 Expedia, Inc
2+
* Copyright 2023 Expedia, Inc
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,32 +16,8 @@
1616

1717
package com.expediagroup.graphql.dataloader.instrumentation.extensions
1818

19-
import graphql.analysis.QueryTraverser
20-
import graphql.analysis.QueryVisitorFieldEnvironment
2119
import graphql.execution.ExecutionContext
2220
import graphql.language.OperationDefinition
23-
import kotlin.math.max
24-
25-
/**
26-
* Calculate the longest path of the [ExecutionContext] AST Document from the root node to a leaf node
27-
* @return the height of the AST Document
28-
*/
29-
internal fun ExecutionContext.getDocumentHeight(): Int {
30-
val getFieldDepth: (QueryVisitorFieldEnvironment?) -> Int = { queryVisitor ->
31-
var hasQueryVisitor = queryVisitor
32-
var height = 1
33-
while (hasQueryVisitor != null) {
34-
hasQueryVisitor = hasQueryVisitor.parentEnvironment
35-
height++
36-
}
37-
height
38-
}
39-
return QueryTraverser.Builder().schema(graphQLSchema).document(document).variables(coercedVariables.toMap()).build()
40-
.reducePreOrder(
41-
{ queryVisitor, height -> max(getFieldDepth(queryVisitor.parentEnvironment), height) },
42-
0
43-
)
44-
}
4521

4622
/**
4723
* Checks if the [ExecutionContext] is a [OperationDefinition.Operation.MUTATION]

executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/state/ExecutionBatchState.kt

Lines changed: 28 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 Expedia, Inc
2+
* Copyright 2023 Expedia, Inc
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,37 +24,34 @@ enum class LevelState { NOT_DISPATCHED, DISPATCHED }
2424
/**
2525
* Handle the state of an [graphql.ExecutionInput]
2626
*/
27-
class ExecutionBatchState(documentHeight: Int) {
28-
29-
private val levelsState: MutableMap<Level, LevelState> = mutableMapOf(
30-
*Array(documentHeight) { number -> Level(number + 1) to LevelState.NOT_DISPATCHED }
31-
)
32-
33-
private val expectedFetches: MutableMap<Level, Int> = mutableMapOf(
34-
*Array(documentHeight) { number -> Level(number + 1) to 0 }
35-
)
36-
private val dispatchedFetches: MutableMap<Level, Int> = mutableMapOf(
37-
*Array(documentHeight) { number -> Level(number + 1) to 0 }
38-
)
39-
40-
private val expectedExecutionStrategies: MutableMap<Level, Int> = mutableMapOf(
41-
*Array(documentHeight) { number ->
42-
val level = Level(number + 1)
43-
level to if (level.isFirst()) 1 else 0
27+
class ExecutionBatchState {
28+
private val levelsState: MutableMap<Level, LevelState> = mutableMapOf()
29+
30+
private val expectedFetches: MutableMap<Level, Int> = mutableMapOf()
31+
private val dispatchedFetches: MutableMap<Level, Int> = mutableMapOf()
32+
33+
private val expectedExecutionStrategies: MutableMap<Level, Int> = mutableMapOf()
34+
private val dispatchedExecutionStrategies: MutableMap<Level, Int> = mutableMapOf()
35+
36+
private val onFieldValueInfos: MutableMap<Level, Int> = mutableMapOf()
37+
38+
private val manuallyCompletableDataFetchers: MutableMap<Level, MutableList<ManualDataFetcher>> = mutableMapOf()
39+
40+
/**
41+
* Initializes a level state in the [ExecutionBatchState]
42+
* @param level to be initialized
43+
*/
44+
fun initializeLevelStateIfNeeded(level: Level) {
45+
if (!this.contains(level)) {
46+
levelsState[level] = LevelState.NOT_DISPATCHED
47+
expectedFetches[level] = 0
48+
dispatchedFetches[level] = 0
49+
expectedExecutionStrategies[level] = if (level.isFirst()) 1 else 0
50+
dispatchedExecutionStrategies[level] = 0
51+
onFieldValueInfos[level] = 0
52+
manuallyCompletableDataFetchers[level] = mutableListOf()
4453
}
45-
)
46-
private val dispatchedExecutionStrategies: MutableMap<Level, Int> = mutableMapOf(
47-
*Array(documentHeight) { number -> Level(number + 1) to 0 }
48-
)
49-
50-
private val onFieldValueInfos: MutableMap<Level, Int> = mutableMapOf(
51-
*Array(documentHeight) { number -> Level(number + 1) to 0 }
52-
)
53-
54-
private val manuallyCompletableDataFetchers: MutableMap<Level, MutableList<ManualDataFetcher>> =
55-
mutableMapOf(
56-
*Array(documentHeight) { number -> Level(number + 1) to mutableListOf() }
57-
)
54+
}
5855

5956
/**
6057
* Check if the [ExecutionBatchState] contains a level

executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/state/ExecutionLevelDispatchedState.kt

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 Expedia, Inc
2+
* Copyright 2023 Expedia, Inc
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,7 +16,6 @@
1616

1717
package com.expediagroup.graphql.dataloader.instrumentation.level.state
1818

19-
import com.expediagroup.graphql.dataloader.instrumentation.extensions.getDocumentHeight
2019
import com.expediagroup.graphql.dataloader.instrumentation.extensions.getExpectedStrategyCalls
2120
import com.expediagroup.graphql.dataloader.instrumentation.level.execution.OnLevelDispatchedCallback
2221
import graphql.ExecutionInput
@@ -41,16 +40,16 @@ class ExecutionLevelDispatchedState(
4140
val executions = ConcurrentHashMap<ExecutionInput, ExecutionBatchState>()
4241

4342
/**
44-
* When a specific [ExecutionInput] starts his execution, calculate the height of the AST Document
43+
* Initialize the [ExecutionBatchState] of this [ExecutionInput]
4544
*
4645
* @param parameters contains information of which [ExecutionInput] will start his execution
47-
* @return a non null [InstrumentationContext] object
46+
* @return a nullable [InstrumentationContext]
4847
*/
4948
fun beginExecuteOperation(
5049
parameters: InstrumentationExecuteOperationParameters
5150
): InstrumentationContext<ExecutionResult>? {
5251
executions.computeIfAbsent(parameters.executionContext.executionInput) {
53-
ExecutionBatchState(parameters.executionContext.getDocumentHeight())
52+
ExecutionBatchState()
5453
}
5554
return null
5655
}
@@ -71,6 +70,7 @@ class ExecutionLevelDispatchedState(
7170

7271
executions.computeIfPresent(executionInput) { _, executionState ->
7372
executionState.also {
73+
it.initializeLevelStateIfNeeded(level)
7474
it.increaseExpectedFetches(level, fieldCount)
7575
it.increaseDispatchedExecutionStrategies(level)
7676
}
@@ -174,18 +174,15 @@ class ExecutionLevelDispatchedState(
174174

175175
/**
176176
* calculate if all executions sharing a graphQLContext was dispatched, by
177-
* 1. Checking if the height of all executions was already calculated.
178-
* 2. Filter all executions sharing the same Level
179-
* 3. check if all executions sharing the same level dispatched that level.
177+
* 1. Checking if all executions started.
178+
* 3. check if all executions dispatched the provided [level].
180179
*
181180
* @param level that execution state will be calculated
182181
* @return Boolean for allExecutionsDispatched statement
183182
*/
184183
fun allExecutionsDispatched(level: Level): Boolean =
185184
executions
186185
.takeIf { executions -> executions.size == totalExecutions }
187-
?.filter { (_, executionState) -> executionState.contains(level) }
188-
?.takeIf { executionsWithSameLevel -> executionsWithSameLevel.isNotEmpty() }
189186
?.all { (_, executionState) -> executionState.isLevelDispatched(level) }
190187
?: false
191188
}

executions/graphql-kotlin-dataloader-instrumentation/src/main/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/state/Level.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 Expedia, Inc
2+
* Copyright 2023 Expedia, Inc
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -34,7 +34,7 @@ data class Level(private val number: Int) {
3434

3535
/**
3636
* calculate if this [Level] is the first
37-
* @return whether or not this is the first [Level]
37+
* @return whether this is the first [Level]
3838
*/
3939
fun isFirst(): Boolean = number == 1
4040
}

executions/graphql-kotlin-dataloader-instrumentation/src/test/kotlin/com/expediagroup/graphql/dataloader/instrumentation/level/DataLoaderLevelDispatchedInstrumentationTest.kt

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2022 Expedia, Inc
2+
* Copyright 2023 Expedia, Inc
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -63,7 +63,7 @@ class DataLoaderLevelDispatchedInstrumentationTest {
6363
assertEquals(1, missionStatistics?.batchInvokeCount)
6464
assertEquals(2, missionStatistics?.batchLoadCount)
6565

66-
verify(exactly = 2) {
66+
verify(exactly = 2 + ONE_LEVEL) {
6767
kotlinDataLoaderRegistry.dispatchAll()
6868
}
6969
}
@@ -94,7 +94,7 @@ class DataLoaderLevelDispatchedInstrumentationTest {
9494
assertEquals(1, missionStatistics?.batchInvokeCount)
9595
assertEquals(2, missionStatistics?.batchLoadCount)
9696

97-
verify(exactly = 3) {
97+
verify(exactly = 3 + ONE_LEVEL) {
9898
kotlinDataLoaderRegistry.dispatchAll()
9999
}
100100
}
@@ -136,7 +136,7 @@ class DataLoaderLevelDispatchedInstrumentationTest {
136136
assertEquals(2, missionsByAstronautStatistics?.batchInvokeCount)
137137
assertEquals(2, missionsByAstronautStatistics?.batchLoadCount)
138138

139-
verify(exactly = 4) {
139+
verify(exactly = 4 + ONE_LEVEL) {
140140
kotlinDataLoaderRegistry.dispatchAll()
141141
}
142142
}
@@ -158,4 +158,8 @@ class DataLoaderLevelDispatchedInstrumentationTest {
158158
kotlinDataLoaderRegistry.dispatchAll()
159159
}
160160
}
161+
162+
companion object {
163+
private const val ONE_LEVEL = 1
164+
}
161165
}

0 commit comments

Comments
 (0)