Performance improvements

atlassian-labs · Dec 19, 2024 · 1e2f9f5 · 1e2f9f5
1 parent 157423c
commit 1e2f9f5
Show file tree

Hide file tree

Showing 19 changed files with 478 additions and 170 deletions.
diff --git a/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt b/lib/src/main/java/graphql/nadel/engine/NadelExecutionContext.kt
@@ -28,10 +28,6 @@ data class NadelExecutionContext internal constructor(
 ) {
     private val serviceContexts = ConcurrentHashMap<String, CompletableFuture<Any?>>()
 
-    @Deprecated("Use incrementalSupport instead", ReplaceWith("incrementalResultSupport"))
-    internal val deferSupport: NadelIncrementalResultSupport
-        get() = incrementalResultSupport
-
     val userContext: Any?
         get() {
             return executionInput.context

diff --git a/lib/src/main/java/graphql/nadel/engine/NadelIncrementalResultAccumulator.kt b/lib/src/main/java/graphql/nadel/engine/NadelIncrementalResultAccumulator.kt
@@ -189,7 +189,7 @@ class NadelIncrementalResultAccumulator(
 /**
  * Similar to [java.io.File.walkTopDown] but for ENFs.
  */
-private fun ExecutableNormalizedOperation.walkTopDown(): Sequence<ExecutableNormalizedField> {
+internal fun ExecutableNormalizedOperation.walkTopDown(): Sequence<ExecutableNormalizedField> {
     return topLevelFields
         .asSequence()
         .flatMap {
@@ -200,7 +200,7 @@ private fun ExecutableNormalizedOperation.walkTopDown(): Sequence<ExecutableNorm
 /**
  * Similar to [java.io.File.walkTopDown] but for ENFs.
  */
-private fun ExecutableNormalizedField.walkTopDown(): Sequence<ExecutableNormalizedField> {
+internal fun ExecutableNormalizedField.walkTopDown(): Sequence<ExecutableNormalizedField> {
     return sequenceOf(this) + children.asSequence()
         .flatMap {
             it.walkTopDown()

diff --git a/lib/src/main/java/graphql/nadel/engine/NadelIncrementalResultSupport.kt b/lib/src/main/java/graphql/nadel/engine/NadelIncrementalResultSupport.kt
@@ -23,16 +23,20 @@ import java.util.concurrent.atomic.AtomicBoolean
 import java.util.concurrent.atomic.AtomicInteger
 
 class NadelIncrementalResultSupport internal constructor(
-    private val accumulator: NadelIncrementalResultAccumulator,
+    lazyAccumulator: Lazy<NadelIncrementalResultAccumulator>,
     private val delayedResultsChannel: Channel<DelayedIncrementalPartialResult> = makeDefaultChannel(),
 ) {
+    private val accumulator by lazyAccumulator
+
     internal constructor(
         operation: ExecutableNormalizedOperation,
         delayedResultsChannel: Channel<DelayedIncrementalPartialResult> = makeDefaultChannel(),
     ) : this(
-        accumulator = NadelIncrementalResultAccumulator(
-            operation = operation,
-        ),
+        lazyAccumulator = lazy {
+            NadelIncrementalResultAccumulator(
+                operation = operation,
+            )
+        },
         delayedResultsChannel = delayedResultsChannel,
     )
 

diff --git a/lib/src/main/java/graphql/nadel/engine/blueprint/NadelExecutionBlueprint.kt b/lib/src/main/java/graphql/nadel/engine/blueprint/NadelExecutionBlueprint.kt
@@ -5,8 +5,8 @@ import graphql.nadel.ServiceExecutionHydrationDetails
 import graphql.nadel.engine.transform.GraphQLObjectTypeName
 import graphql.nadel.engine.util.emptyOrSingle
 import graphql.nadel.engine.util.makeFieldCoordinates
-import graphql.nadel.engine.util.mapFrom
 import graphql.nadel.engine.util.strictAssociateBy
+import graphql.nadel.util.NadelFieldMap
 import graphql.normalized.ExecutableNormalizedField
 import graphql.schema.FieldCoordinates
 import graphql.schema.GraphQLSchema
@@ -16,7 +16,7 @@ import graphql.schema.GraphQLSchema
  */
 data class NadelOverallExecutionBlueprint(
     val engineSchema: GraphQLSchema,
-    val fieldInstructions: Map<FieldCoordinates, List<NadelFieldInstruction>>,
+    val fieldInstructions: NadelFieldMap<List<NadelFieldInstruction>>,
     private val underlyingTypeNamesByService: Map<Service, Set<String>>,
     private val overallTypeNamesByService: Map<Service, Set<String>>,
     private val underlyingBlueprints: Map<String, NadelUnderlyingExecutionBlueprint>,
@@ -72,6 +72,38 @@ data class NadelOverallExecutionBlueprint(
         return coordinatesToService[fieldCoordinates]
     }
 
+    inline fun <reified T : NadelFieldInstruction> getTypeNameToInstructionMap(
+        field: ExecutableNormalizedField,
+    ): Map<GraphQLObjectTypeName, T> {
+        val map: MutableMap<GraphQLObjectTypeName, T> = mutableMapOf()
+
+        field.objectTypeNames.forEach { objectTypeName ->
+            val instruction = fieldInstructions.get(objectTypeName, field.name)
+                ?.asSequence()
+                ?.filterIsInstance<T>()
+                ?.emptyOrSingle() ?: return@forEach
+            map[objectTypeName] = instruction
+        }
+
+        return map
+    }
+
+    inline fun <reified T : NadelFieldInstruction> getTypeNameToInstructionsMap(
+        field: ExecutableNormalizedField,
+    ): Map<GraphQLObjectTypeName, List<T>> {
+        val map: MutableMap<GraphQLObjectTypeName, List<T>> = mutableMapOf()
+
+        field.objectTypeNames.forEach { objectTypeName ->
+            val instructions = fieldInstructions.get(objectTypeName, field.name)
+                ?.filterIsInstance<T>()
+            if (instructions?.isNotEmpty() == true) {
+                map[objectTypeName] = instructions
+            }
+        }
+
+        return map
+    }
+
     inline fun <reified T : NadelFieldInstruction> getInstructionInsideVirtualType(
         hydrationDetails: ServiceExecutionHydrationDetails?,
         backingField: ExecutableNormalizedField,
@@ -158,41 +190,3 @@ data class NadelTypeRenameInstructions internal constructor(
         }
     }
 }
-
-/**
- * todo: why doesn't this belong inside [NadelOverallExecutionBlueprint]
- */
-inline fun <reified T : NadelFieldInstruction> Map<FieldCoordinates, List<NadelFieldInstruction>>.getTypeNameToInstructionMap(
-    field: ExecutableNormalizedField,
-): Map<GraphQLObjectTypeName, T> {
-    return mapFrom(
-        field.objectTypeNames
-            .mapNotNull { objectTypeName ->
-                val coordinates = makeFieldCoordinates(objectTypeName, field.name)
-                val instruction = this[coordinates]
-                    ?.filterIsInstance<T>()
-                    ?.emptyOrSingle() ?: return@mapNotNull null
-                objectTypeName to instruction
-            },
-    )
-}
-
-/**
- * todo: why doesn't this belong inside [NadelOverallExecutionBlueprint]
- */
-inline fun <reified T : NadelFieldInstruction> Map<FieldCoordinates, List<NadelFieldInstruction>>.getTypeNameToInstructionsMap(
-    field: ExecutableNormalizedField,
-): Map<GraphQLObjectTypeName, List<T>> {
-    return mapFrom(
-        field.objectTypeNames
-            .mapNotNull { objectTypeName ->
-                val coordinates = makeFieldCoordinates(objectTypeName, field.name)
-                val instructions = (this[coordinates] ?: return@mapNotNull null)
-                    .filterIsInstance<T>()
-                when {
-                    instructions.isEmpty() -> return@mapNotNull null
-                    else -> objectTypeName to instructions
-                }
-            },
-    )
-}
diff --git a/lib/src/main/java/graphql/nadel/engine/plan/NadelExecutionPlanFactory.kt b/lib/src/main/java/graphql/nadel/engine/plan/NadelExecutionPlanFactory.kt
@@ -22,6 +22,7 @@ import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParame
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters.RootStep.ExecutionPlanning
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters.RootStep.QueryTransforming
 import graphql.nadel.instrumentation.parameters.NadelInstrumentationTimingParameters.RootStep.ResultTransforming
+import graphql.nadel.util.dfs
 import graphql.normalized.ExecutableNormalizedField
 
 internal class NadelExecutionPlanFactory(
@@ -60,60 +61,64 @@ internal class NadelExecutionPlanFactory(
         rootField: ExecutableNormalizedField,
         serviceHydrationDetails: ServiceExecutionHydrationDetails?,
     ): NadelExecutionPlan {
-        val executionSteps = mutableListOf<AnyNadelExecutionPlanStep>()
+        val executionSteps: MutableMap<ExecutableNormalizedField, List<NadelExecutionPlan.Step<Any>>> =
+            mutableMapOf()
 
         executionContext.timer.batch { timer ->
             traverseQuery(rootField) { field ->
-                transformsWithTimingStepInfo.forEach { transformWithTimingInfo ->
+                val steps = transformsWithTimingStepInfo.mapNotNull { transformWithTimingInfo ->
                     val transform = transformWithTimingInfo.transform
                     // This is a patch to prevent errors
                     // Ideally this should not happen but the proper fix requires more refactoring
                     // See NadelSkipIncludeTransform.isApplicable for more details
                     if (isSkipIncludeSpecialField(field) && ((transform as NadelTransform<*>) !is NadelSkipIncludeTransform)) {
-                        return@forEach
-                    }
-
-                    val state = timer.time(step = transformWithTimingInfo.executionPlanTimingStep) {
-                        transform.isApplicable(
-                            executionContext,
-                            serviceExecutionContext,
-                            executionBlueprint,
-                            services,
-                            service,
-                            field,
-                            serviceHydrationDetails,
-                        )
-                    }
+                        null
+                    } else {
+                        val state = timer.time(step = transformWithTimingInfo.executionPlanTimingStep) {
+                            transform.isApplicable(
+                                executionContext,
+                                serviceExecutionContext,
+                                executionBlueprint,
+                                services,
+                                service,
+                                field,
+                                serviceHydrationDetails,
+                            )
+                        }
 
-                    if (state != null) {
-                        executionSteps.add(
+                        if (state == null) {
+                            null
+                        } else {
                             NadelExecutionPlan.Step(
                                 service = service,
                                 field = field,
                                 transform = transform,
                                 queryTransformTimingStep = transformWithTimingInfo.queryTransformTimingStep,
                                 resultTransformTimingStep = transformWithTimingInfo.resultTransformTimingStep,
                                 state = state,
-                            ),
-                        )
+                            )
+                        }
                     }
                 }
+
+                if (steps.isNotEmpty()) {
+                    executionSteps[field] = steps
+                }
             }
         }
 
-        return NadelExecutionPlan(
-            executionSteps.groupBy { it.field },
-        )
+        return NadelExecutionPlan(executionSteps)
     }
 
-    private suspend fun traverseQuery(
+    private inline fun traverseQuery(
         root: ExecutableNormalizedField,
-        consumer: suspend (ExecutableNormalizedField) -> Unit,
+        consumer: (ExecutableNormalizedField) -> Unit,
     ) {
-        consumer(root)
-        root.children.forEach {
-            traverseQuery(it, consumer)
-        }
+        dfs(
+            root = root,
+            getChildren = ExecutableNormalizedField::getChildren,
+            consumer = consumer,
+        )
     }
 
     companion object {

diff --git a/lib/src/main/java/graphql/nadel/engine/transform/NadelDeepRenameTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/NadelDeepRenameTransform.kt
@@ -7,7 +7,6 @@ import graphql.nadel.engine.NadelExecutionContext
 import graphql.nadel.engine.NadelServiceExecutionContext
 import graphql.nadel.engine.blueprint.NadelDeepRenameFieldInstruction
 import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint
-import graphql.nadel.engine.blueprint.getTypeNameToInstructionMap
 import graphql.nadel.engine.transform.artificial.NadelAliasHelper
 import graphql.nadel.engine.transform.query.NFUtil
 import graphql.nadel.engine.transform.query.NadelQueryPath
@@ -91,7 +90,7 @@ internal class NadelDeepRenameTransform : NadelTransform<NadelDeepRenameTransfor
         overallField: ExecutableNormalizedField,
         hydrationDetails: ServiceExecutionHydrationDetails?,
     ): State? {
-        val deepRenameInstructions = executionBlueprint.fieldInstructions
+        val deepRenameInstructions = executionBlueprint
             .getTypeNameToInstructionMap<NadelDeepRenameFieldInstruction>(overallField)
         if (deepRenameInstructions.isEmpty()) {
             return null

diff --git a/lib/src/main/java/graphql/nadel/engine/transform/NadelRenameTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/NadelRenameTransform.kt
@@ -7,7 +7,6 @@ import graphql.nadel.engine.NadelExecutionContext
 import graphql.nadel.engine.NadelServiceExecutionContext
 import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint
 import graphql.nadel.engine.blueprint.NadelRenameFieldInstruction
-import graphql.nadel.engine.blueprint.getTypeNameToInstructionMap
 import graphql.nadel.engine.transform.NadelRenameTransform.State
 import graphql.nadel.engine.transform.artificial.NadelAliasHelper
 import graphql.nadel.engine.transform.query.NFUtil.createField
@@ -46,7 +45,7 @@ internal class NadelRenameTransform : NadelTransform<State> {
         overallField: ExecutableNormalizedField,
         hydrationDetails: ServiceExecutionHydrationDetails?,
     ): State? {
-        val renameInstructions = executionBlueprint.fieldInstructions
+        val renameInstructions = executionBlueprint
             .getTypeNameToInstructionMap<NadelRenameFieldInstruction>(overallField)
         if (renameInstructions.isEmpty()) {
             return null

diff --git a/lib/src/main/java/graphql/nadel/engine/transform/NadelServiceTypeFilterTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/NadelServiceTypeFilterTransform.kt
@@ -90,22 +90,29 @@ class NadelServiceTypeFilterTransform : NadelTransform<State> {
 
         val typeNamesOwnedByService = executionBlueprint.getOverAllTypeNamesForService(service)
 
-        val fieldObjectTypeNamesOwnedByService = overallField.objectTypeNames
-            .filter { objectTypeName ->
-                isTypeOwnedByService(
-                    objectTypeName,
-                    executionContext,
-                    service,
-                    executionBlueprint,
-                )
+        // Add underlying type names as well to handle combination of hydration and renames.
+        // Transforms are applied to hydration fields as well, and those fields always reference
+        // elements from the underlying schema
+        val underlyingTypeNamesOwnedByService = executionBlueprint.getUnderlyingTypeNamesForService(service)
+
+        // Assume for most cases there aren't foreign types so there is no point filtering to a new List
+        val noForeignTypes = overallField.objectTypeNames
+            .all { objectTypeName ->
+                objectTypeName in typeNamesOwnedByService
+                    || objectTypeName in underlyingTypeNamesOwnedByService
+                    || (executionContext.hints.sharedTypeRenames(service) && executionBlueprint.getUnderlyingTypeName(objectTypeName) in underlyingTypeNamesOwnedByService)
             }
 
-        // All types are owned by service
-        // Note: one list is a subset of the other, so if size is same, contents are too
-        if (fieldObjectTypeNamesOwnedByService.size == overallField.objectTypeNames.size) {
+        if (noForeignTypes) {
             return null
         }
 
+        val fieldObjectTypeNamesOwnedByService = overallField.objectTypeNames.filter { objectTypeName ->
+            objectTypeName in typeNamesOwnedByService
+                || objectTypeName in underlyingTypeNamesOwnedByService
+                || (executionContext.hints.sharedTypeRenames(service) && executionBlueprint.getUnderlyingTypeName(objectTypeName) in underlyingTypeNamesOwnedByService)
+        }
+
         return State(
             aliasHelper = NadelAliasHelper.forField(
                 tag = "type_filter",
@@ -117,25 +124,6 @@ class NadelServiceTypeFilterTransform : NadelTransform<State> {
         )
     }
 
-    private fun isTypeOwnedByService(
-        objectTypeName: String,
-        executionContext: NadelExecutionContext,
-        service: Service,
-        executionBlueprint: NadelOverallExecutionBlueprint,
-    ): Boolean {
-        val typeNamesOwnedByService = executionBlueprint.getOverAllTypeNamesForService(service)
-        // Add underlying type names as well to handle combination of hydration and renames.
-        // Transforms are applied to hydration fields as well, and those fields always reference
-        // elements from the underlying schema
-        val underlyingTypeNamesOwnedByService = executionBlueprint.getUnderlyingTypeNamesForService(service)
-
-        // it is MUCH quicker to compare membership in 2 sets rather than
-        // concat 1 giant set and then check
-        return objectTypeName in typeNamesOwnedByService
-            || objectTypeName in underlyingTypeNamesOwnedByService
-            || (executionContext.hints.sharedTypeRenames(service) && executionBlueprint.getUnderlyingTypeName(objectTypeName) in underlyingTypeNamesOwnedByService)
-    }
-
     override suspend fun transformField(
         executionContext: NadelExecutionContext,
         serviceExecutionContext: NadelServiceExecutionContext,

diff --git a/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationTransform.kt b/lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationTransform.kt
@@ -11,7 +11,6 @@ import graphql.nadel.engine.NadelServiceExecutionContext
 import graphql.nadel.engine.blueprint.NadelGenericHydrationInstruction
 import graphql.nadel.engine.blueprint.NadelHydrationFieldInstruction
 import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint
-import graphql.nadel.engine.blueprint.getTypeNameToInstructionsMap
 import graphql.nadel.engine.blueprint.hydration.NadelHydrationStrategy
 import graphql.nadel.engine.transform.GraphQLObjectTypeName
 import graphql.nadel.engine.transform.NadelTransform
@@ -77,7 +76,7 @@ internal class NadelHydrationTransform(
         overallField: ExecutableNormalizedField,
         hydrationDetails: ServiceExecutionHydrationDetails?,
     ): State? {
-        val instructionsByObjectTypeName = executionBlueprint.fieldInstructions
+        val instructionsByObjectTypeName = executionBlueprint
             .getTypeNameToInstructionsMap<NadelHydrationFieldInstruction>(overallField)
             .ifEmpty {
                 if (executionContext.hints.virtualTypeSupport(service)) {

diff --git a/.../main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationTransform.kt b/.../main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationTransform.kt
@@ -8,7 +8,6 @@ import graphql.nadel.engine.NadelExecutionContext
 import graphql.nadel.engine.NadelServiceExecutionContext
 import graphql.nadel.engine.blueprint.NadelBatchHydrationFieldInstruction
 import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint
-import graphql.nadel.engine.blueprint.getTypeNameToInstructionsMap
 import graphql.nadel.engine.transform.GraphQLObjectTypeName
 import graphql.nadel.engine.transform.NadelTransform
 import graphql.nadel.engine.transform.NadelTransformFieldResult
@@ -47,7 +46,7 @@ internal class NadelBatchHydrationTransform(
         overallField: ExecutableNormalizedField,
         hydrationDetails: ServiceExecutionHydrationDetails?,
     ): State? {
-        val instructionsByObjectTypeName = executionBlueprint.fieldInstructions
+        val instructionsByObjectTypeName = executionBlueprint
             .getTypeNameToInstructionsMap<NadelBatchHydrationFieldInstruction>(overallField)
             .ifEmpty {
                 if (executionContext.hints.virtualTypeSupport(service)) {