Performance improvements around result transforming (#448)

* Use computed nodes if flatten is true

* Stop constructing JSON node paths

* More performance improvements to avoid extra memory allocations

* Delete JsonNodePath from main module

* Add Nadel prefix and more type safety

* Avoid list allocation

lib/src/main/java/graphql/nadel/NadelResultMerger.kt

@@ -5,6 +5,7 @@ import graphql.ExecutionResultImpl
 import graphql.GraphQLError
 import graphql.introspection.Introspection
 import graphql.nadel.engine.transform.query.NadelFieldAndService
+import graphql.nadel.engine.transform.result.NadelResultKey
 import graphql.nadel.engine.util.AnyMap
 import graphql.nadel.engine.util.JsonMap
 import graphql.nadel.engine.util.MutableJsonMap
@@ -16,9 +17,6 @@ import graphql.nadel.util.NamespacedUtil.isNamespacedField
 import graphql.normalized.ExecutableNormalizedField
 import graphql.schema.GraphQLSchema
 
-@JvmInline
-private value class ResultKey(val value: String)
-
 internal object NadelResultMerger {
     fun mergeResults(
         fields: List<NadelFieldAndService>,
@@ -118,14 +116,14 @@ internal object NadelResultMerger {
     private fun buildRequiredFieldMap(
         fields: List<NadelFieldAndService>,
         engineSchema: GraphQLSchema,
-    ): MutableMap<ResultKey, MutableList<ExecutableNormalizedField>> {
-        val requiredFields = mutableMapOf<ResultKey, MutableList<ExecutableNormalizedField>>()
+    ): MutableMap<NadelResultKey, MutableList<ExecutableNormalizedField>> {
+        val requiredFields = mutableMapOf<NadelResultKey, MutableList<ExecutableNormalizedField>>()
 
         // NOTE: please ensure all fields are from object types and will NOT have multiple field defs
         // Other code in this file relies on this contract
         for ((field) in fields) {
             val requiredChildFields = requiredFields
-                .computeIfAbsent(ResultKey(field.resultKey)) {
+                .computeIfAbsent(NadelResultKey(field.resultKey)) {
                     mutableListOf()
                 }
 

lib/src/main/java/graphql/nadel/engine/transform/NadelDeepRenameTransform.kt

@@ -12,6 +12,8 @@ import graphql.nadel.engine.transform.query.NFUtil
 import graphql.nadel.engine.transform.query.NadelQueryPath
 import graphql.nadel.engine.transform.query.NadelQueryTransformer
 import graphql.nadel.engine.transform.result.NadelResultInstruction
+import graphql.nadel.engine.transform.result.NadelResultKey
+import graphql.nadel.engine.transform.result.json.JsonNode
 import graphql.nadel.engine.transform.result.json.JsonNodeExtractor
 import graphql.nadel.engine.transform.result.json.JsonNodes
 import graphql.nadel.engine.util.emptyOrSingle
@@ -310,15 +312,17 @@ internal class NadelDeepRenameTransform : NadelTransform<NadelDeepRenameTransfor
             val sourceFieldNode = JsonNodeExtractor.getNodesAt(parentNode, queryPathForSourceField)
                 .emptyOrSingle()
 
-            val destinationPath = parentNode.resultPath + overallField.resultKey
             when (sourceFieldNode) {
                 null -> NadelResultInstruction.Set(
-                    subjectPath = destinationPath,
+                    subject = parentNode,
+                    key = NadelResultKey(overallField.resultKey),
                     newValue = null,
                 )
-                else -> NadelResultInstruction.Copy(
-                    subjectPath = sourceFieldNode.resultPath,
-                    destinationPath = destinationPath,
+
+                else -> NadelResultInstruction.Set(
+                    subject = parentNode,
+                    key = NadelResultKey(overallField.resultKey),
+                    newValue = sourceFieldNode,
                 )
             }
         }

lib/src/main/java/graphql/nadel/engine/transform/NadelRenameTransform.kt

@@ -13,6 +13,7 @@ import graphql.nadel.engine.transform.query.NFUtil.createField
 import graphql.nadel.engine.transform.query.NadelQueryPath
 import graphql.nadel.engine.transform.query.NadelQueryTransformer
 import graphql.nadel.engine.transform.result.NadelResultInstruction
+import graphql.nadel.engine.transform.result.NadelResultKey
 import graphql.nadel.engine.transform.result.json.JsonNode
 import graphql.nadel.engine.transform.result.json.JsonNodeExtractor
 import graphql.nadel.engine.transform.result.json.JsonNodes
@@ -196,9 +197,10 @@ internal class NadelRenameTransform : NadelTransform<State> {
             val sourceFieldNode = JsonNodeExtractor.getNodesAt(parentNode, queryPathForSourceField)
                 .emptyOrSingle() ?: return@instruction null
 
-            NadelResultInstruction.Copy(
-                subjectPath = sourceFieldNode.resultPath,
-                destinationPath = parentNode.resultPath + overallField.resultKey,
+            NadelResultInstruction.Set(
+                subject = parentNode,
+                key = NadelResultKey(overallField.resultKey),
+                newValue = sourceFieldNode,
             )
         }
     }

lib/src/main/java/graphql/nadel/engine/transform/NadelTransformUtil.kt

@@ -2,18 +2,13 @@ package graphql.nadel.engine.transform
 
 import graphql.introspection.Introspection.TypeNameMetaFieldDef
 import graphql.nadel.Service
-import graphql.nadel.ServiceExecutionResult
 import graphql.nadel.engine.blueprint.NadelFieldInstruction
 import graphql.nadel.engine.blueprint.NadelOverallExecutionBlueprint
 import graphql.nadel.engine.transform.artificial.NadelAliasHelper
-import graphql.nadel.engine.transform.query.NadelQueryPath
-import graphql.nadel.engine.transform.result.NadelResultInstruction
 import graphql.nadel.engine.transform.result.json.JsonNode
-import graphql.nadel.engine.transform.result.json.JsonNodes
 import graphql.nadel.engine.util.JsonMap
 import graphql.nadel.engine.util.getField
 import graphql.nadel.engine.util.makeFieldCoordinates
-import graphql.nadel.engine.util.queryPath
 import graphql.normalized.ExecutableNormalizedField
 import graphql.normalized.ExecutableNormalizedField.newNormalizedField
 import graphql.schema.GraphQLFieldDefinition
@@ -75,40 +70,6 @@ object NadelTransformUtil {
         val coordinates = makeFieldCoordinates(overallTypeName, overallField.name)
         return executionBlueprint.engineSchema.getField(coordinates)
     }
-
-    /**
-     * Creates a list of Set instructions that will be used to modified the values of the result data
-     * for a particular field.
-     *
-     * This function will call the transformerFunction for all result nodes associated with the overallField. So the
-     * caller doesn't need to worry about whether the overallField is a collection or an individual field.
-     *
-     * Note that the transformer will only be called for non-null values.
-     */
-    fun createSetInstructions(
-        nodes: JsonNodes,
-        underlyingParentField: ExecutableNormalizedField?,
-        result: ServiceExecutionResult,
-        overallField: ExecutableNormalizedField,
-        transformerFunction: (Any) -> Any?,
-    ): List<NadelResultInstruction> {
-        val parentQueryPath = underlyingParentField?.queryPath ?: NadelQueryPath.root
-
-        val valueNodes: List<JsonNode> = nodes.getNodesAt(
-            queryPath = parentQueryPath + overallField.resultKey,
-            flatten = true
-        )
-
-        return valueNodes
-            .mapNotNull { valueNode ->
-                nodes.getNodeAt(valueNode.resultPath)?.let { jsonNode ->
-                    NadelResultInstruction.Set(
-                        valueNode.resultPath,
-                        jsonNode.value?.let { value -> transformerFunction(value) },
-                    )
-                }
-            }
-    }
 }
 
 /**

lib/src/main/java/graphql/nadel/engine/transform/NadelTypeRenameResultTransform.kt

@@ -10,7 +10,10 @@ import graphql.nadel.engine.transform.NadelTypeRenameResultTransform.State
 import graphql.nadel.engine.transform.query.NadelQueryPath
 import graphql.nadel.engine.transform.query.NadelQueryTransformer
 import graphql.nadel.engine.transform.result.NadelResultInstruction
+import graphql.nadel.engine.transform.result.NadelResultKey
+import graphql.nadel.engine.transform.result.json.JsonNode
 import graphql.nadel.engine.transform.result.json.JsonNodes
+import graphql.nadel.engine.util.JsonMap
 import graphql.nadel.engine.util.queryPath
 import graphql.normalized.ExecutableNormalizedField
 
@@ -57,21 +60,27 @@ internal class NadelTypeRenameResultTransform : NadelTransform<State> {
         state: State,
         nodes: JsonNodes,
     ): List<NadelResultInstruction> {
-        val typeNameNodes = nodes.getNodesAt(
-            (underlyingParentField?.queryPath ?: NadelQueryPath.root) + overallField.resultKey,
+        val parentNodes = nodes.getNodesAt(
+            underlyingParentField?.queryPath ?: NadelQueryPath.root,
             flatten = true,
         )
 
-        return typeNameNodes.mapNotNull { typeNameNode ->
-            val underlyingTypeName = typeNameNode.value as String?
+        return parentNodes.mapNotNull { parentNode ->
+            @Suppress("UNCHECKED_CAST")
+            val parentMap = parentNode.value as? JsonMap
                 ?: return@mapNotNull null
+            val underlyingTypeName = parentMap[overallField.resultKey] as String?
+                ?: return@mapNotNull null
+
             val overallTypeName = executionBlueprint.getOverallTypeName(
                 service = service,
                 underlyingTypeName = underlyingTypeName,
             )
+
             NadelResultInstruction.Set(
-                subjectPath = typeNameNode.resultPath,
-                newValue = overallTypeName,
+                subject = parentNode,
+                key = NadelResultKey(overallField.resultKey),
+                newValue = JsonNode(overallTypeName),
             )
         }
     }

lib/src/main/java/graphql/nadel/engine/transform/hydration/NadelHydrationTransform.kt

@@ -22,6 +22,7 @@ import graphql.nadel.engine.transform.hydration.NadelHydrationUtil.getInstructio
 import graphql.nadel.engine.transform.query.NadelQueryPath
 import graphql.nadel.engine.transform.query.NadelQueryTransformer
 import graphql.nadel.engine.transform.result.NadelResultInstruction
+import graphql.nadel.engine.transform.result.NadelResultKey
 import graphql.nadel.engine.transform.result.json.JsonNode
 import graphql.nadel.engine.transform.result.json.JsonNodeExtractor
 import graphql.nadel.engine.transform.result.json.JsonNodes
@@ -191,7 +192,13 @@ internal class NadelHydrationTransform(
         }
 
         val instruction = getHydrationFieldInstruction(state, instructions, executionContext.hooks, parentNode)
-            ?: return listOf(NadelResultInstruction.Set(parentNode.resultPath + state.hydratedField.fieldName, null))
+            ?: return listOf(
+                NadelResultInstruction.Set(
+                    subject = parentNode,
+                    key = NadelResultKey(state.hydratedField.resultKey),
+                    newValue = null,
+                ),
+            )
 
         val actorQueryResults = coroutineScope {
             NadelHydrationFieldsBuilder.makeActorQueries(
@@ -238,8 +245,9 @@ internal class NadelHydrationTransform(
 
                 return listOf(
                     NadelResultInstruction.Set(
-                        subjectPath = parentNode.resultPath + fieldToHydrate.resultKey,
-                        newValue = data?.value,
+                        subject = parentNode,
+                        key = NadelResultKey(fieldToHydrate.resultKey),
+                        newValue = JsonNode(data?.value),
                     ),
                 ) + errors
             }
@@ -256,8 +264,9 @@ internal class NadelHydrationTransform(
 
                 return listOf(
                     NadelResultInstruction.Set(
-                        subjectPath = parentNode.resultPath + fieldToHydrate.resultKey,
-                        newValue = data,
+                        subject = parentNode,
+                        key = NadelResultKey(fieldToHydrate.resultKey),
+                        newValue = JsonNode(data),
                     ),
                 ) + addErrors
             }

lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByIndex.kt

@@ -7,6 +7,7 @@ import graphql.nadel.engine.transform.artificial.NadelAliasHelper
 import graphql.nadel.engine.transform.hydration.NadelHydrationUtil
 import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationInputBuilder.getBatchInputDef
 import graphql.nadel.engine.transform.result.NadelResultInstruction
+import graphql.nadel.engine.transform.result.NadelResultKey
 import graphql.nadel.engine.transform.result.json.JsonNode
 import graphql.nadel.engine.util.AnyList
 import graphql.nadel.engine.util.emptyOrSingle
@@ -48,14 +49,17 @@ internal class NadelBatchHydrationByIndex private constructor(
                 val inputValues = getInputValues(parentNode)
 
                 NadelResultInstruction.Set(
-                    subjectPath = parentNode.resultPath + fieldToHydrate.resultKey,
-                    newValue = if (isManyInputNodesToParentNodes) {
-                        chunker.take(inputValues)
-                    } else {
-                        chunker.takeOne(
-                            inputValue = inputValues.emptyOrSingle(),
-                        )
-                    },
+                    subject = parentNode,
+                    key = NadelResultKey(fieldToHydrate.resultKey),
+                    newValue = JsonNode(
+                        if (isManyInputNodesToParentNodes) {
+                            chunker.take(inputValues)
+                        } else {
+                            chunker.takeOne(
+                                inputValue = inputValues.emptyOrSingle(),
+                            )
+                        },
+                    ),
                 )
             }
     }
@@ -145,6 +149,7 @@ internal class NadelBatchHydrationByIndex private constructor(
                                 badCount()
                             }
                         }
+
                         else -> error("Unsupported batch result type")
                     }
                 }

lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrationByObjectId.kt

@@ -7,6 +7,7 @@ import graphql.nadel.engine.blueprint.hydration.NadelBatchHydrationMatchStrategy
 import graphql.nadel.engine.transform.NadelTransformUtil
 import graphql.nadel.engine.transform.hydration.NadelHydrationUtil.getHydrationActorNodes
 import graphql.nadel.engine.transform.result.NadelResultInstruction
+import graphql.nadel.engine.transform.result.NadelResultKey
 import graphql.nadel.engine.transform.result.asMutable
 import graphql.nadel.engine.transform.result.json.JsonNode
 import graphql.nadel.engine.transform.result.json.JsonNodeExtractor
@@ -160,8 +161,9 @@ internal object NadelBatchHydrationByObjectId {
         }
 
         return NadelResultInstruction.Set(
-            subjectPath = sourceNode.resultPath + state.hydratedField.resultKey,
-            newValue = newValue,
+            subject = sourceNode,
+            key = NadelResultKey(state.hydratedField.resultKey),
+            newValue = JsonNode(newValue),
         )
     }
 

lib/src/main/java/graphql/nadel/engine/transform/hydration/batch/NadelBatchHydrator.kt

@@ -15,6 +15,7 @@ import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationByObjec
 import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationByObjectId.getHydrateInstructionsMatchingObjectIds
 import graphql.nadel.engine.transform.hydration.batch.NadelBatchHydrationTransform.State
 import graphql.nadel.engine.transform.result.NadelResultInstruction
+import graphql.nadel.engine.transform.result.NadelResultKey
 import graphql.nadel.engine.transform.result.json.JsonNode
 import graphql.schema.FieldCoordinates
 import kotlinx.coroutines.Deferred
@@ -61,7 +62,8 @@ internal class NadelBatchHydrator(
                     when (instruction) {
                         null -> parentNodes.map {
                             NadelResultInstruction.Set(
-                                it.resultPath + state.hydratedField.fieldName,
+                                subject = it,
+                                key = NadelResultKey(state.hydratedField.resultKey),
                                 newValue = null,
                             )
                         }

lib/src/main/java/graphql/nadel/engine/transform/result/NadelResultInstruction.kt

@@ -1,34 +1,24 @@
 package graphql.nadel.engine.transform.result
 
 import graphql.GraphQLError
-import graphql.nadel.engine.transform.result.json.JsonNodePath
+import graphql.nadel.engine.transform.result.json.JsonNode
+
+// todo: should be a value class one day… can't because of Java interop
+data class NadelResultKey(val value: String)
 
 sealed class NadelResultInstruction {
     data class Set(
-        override val subjectPath: JsonNodePath,
-        val newValue: Any?,
-    ) : NadelResultInstruction(), NadelResultInstructionWithSubject
+        val subject: JsonNode,
+        val key: NadelResultKey,
+        val newValue: JsonNode?,
+    ) : NadelResultInstruction()
 
     data class Remove(
-        override val subjectPath: JsonNodePath,
-    ) : NadelResultInstruction(), NadelResultInstructionWithSubject
-
-    data class Copy(
-        override val subjectPath: JsonNodePath,
-        val destinationPath: JsonNodePath,
-    ) : NadelResultInstruction(), NadelResultInstructionWithSubject {
-        val destinationKey: String
-            get() = destinationPath.segments.last().value as String
-    }
+        val subject: JsonNode,
+        val key: NadelResultKey,
+    ) : NadelResultInstruction()
 
     data class AddError(
         val error: GraphQLError,
     ) : NadelResultInstruction()
 }
-
-interface NadelResultInstructionWithSubject {
-    val subjectPath: JsonNodePath
-
-    val subjectKey: String
-        get() = subjectPath.segments.last().value as String
-}