From 63e895785a52e75020b15c553008ab4246fc9a8c Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sat, 6 Apr 2024 22:10:08 +0300 Subject: [PATCH] perf: check if streamUsage is defined outside the loop introducing separate looping functions for when streamUsage is defined --- src/execution/execute.ts | 180 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 173 insertions(+), 7 deletions(-) diff --git a/src/execution/execute.ts b/src/execution/execute.ts index 7ed4c29409a..02b5d2290dd 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -1014,7 +1014,6 @@ function getStreamUsage( return streamUsage; } - /** * Complete a async iterator value by completing the result and calling * recursively until all the results are completed. @@ -1033,10 +1032,87 @@ async function completeAsyncIteratorValue( const completedResults: Array = []; const acc: GraphQLResult> = [completedResults, []]; let index = 0; - const streamUsage = getStreamUsage(exeContext, fieldGroup, path); // eslint-disable-next-line no-constant-condition while (true) { - if (streamUsage && index >= streamUsage.initialCount) { + const itemPath = addPath(path, index, undefined); + let iteration; + try { + // eslint-disable-next-line no-await-in-loop + iteration = await asyncIterator.next(); + } catch (rawError) { + throw locatedError(rawError, toNodes(fieldGroup), pathToArray(path)); + } + + if (iteration.done) { + break; + } + + const item = iteration.value; + // TODO: add test case for asyncIterator returning a promise + /* c8 ignore start */ + if (isPromise(item)) { + completedResults.push( + completePromisedListItemValue( + item, + acc, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ), + ); + containsPromise = true; + } else if ( + /* c8 ignore stop */ + completeListItemValue( + item, + completedResults, + acc, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ) + ) { + containsPromise = true; + } + index++; + } + + return containsPromise + ? Promise.all(completedResults).then((resolved) => [resolved, acc[1]]) + : acc; +} + +/** + * Complete a async iterator value by completing the result and calling + * recursively until all the results are completed. + */ +async function completeAsyncIteratorValueWithPossibleStream( + exeContext: ExecutionContext, + itemType: GraphQLOutputType, + fieldGroup: FieldGroup, + info: GraphQLResolveInfo, + path: Path, + asyncIterator: AsyncIterator, + streamUsage: StreamUsage, + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): Promise>> { + let containsPromise = false; + const completedResults: Array = []; + const acc: GraphQLResult> = [completedResults, []]; + let index = 0; + const initialCount = streamUsage.initialCount; + // eslint-disable-next-line no-constant-condition + while (true) { + if (index >= initialCount) { const streamRecord = new StreamRecord({ label: streamUsage.label, path, @@ -1147,17 +1223,32 @@ function completeListValue( deferMap: ReadonlyMap | undefined, ): PromiseOrValue>> { const itemType = returnType.ofType; + const streamUsage = getStreamUsage(exeContext, fieldGroup, path); if (isAsyncIterable(result)) { const asyncIterator = result[Symbol.asyncIterator](); - return completeAsyncIteratorValue( + if (streamUsage === undefined) { + return completeAsyncIteratorValue( + exeContext, + itemType, + fieldGroup, + info, + path, + asyncIterator, + incrementalContext, + deferMap, + ); + } + + return completeAsyncIteratorValueWithPossibleStream( exeContext, itemType, fieldGroup, info, path, asyncIterator, + streamUsage, incrementalContext, deferMap, ); @@ -1169,13 +1260,27 @@ function completeListValue( ); } - return completeIterableValue( + if (streamUsage === undefined) { + return completeIterableValue( + exeContext, + itemType, + fieldGroup, + info, + path, + result, + incrementalContext, + deferMap, + ); + } + + return completeIterableValueWithPossibleStream( exeContext, itemType, fieldGroup, info, path, result, + streamUsage, incrementalContext, deferMap, ); @@ -1197,13 +1302,74 @@ function completeIterableValue( const completedResults: Array = []; const acc: GraphQLResult> = [completedResults, []]; let index = 0; - const streamUsage = getStreamUsage(exeContext, fieldGroup, path); + for (const item of items) { + // No need to modify the info object containing the path, + // since from here on it is not ever accessed by resolver functions. + const itemPath = addPath(path, index, undefined); + + if (isPromise(item)) { + completedResults.push( + completePromisedListItemValue( + item, + acc, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ), + ); + containsPromise = true; + } else if ( + completeListItemValue( + item, + completedResults, + acc, + exeContext, + itemType, + fieldGroup, + info, + itemPath, + incrementalContext, + deferMap, + ) + ) { + containsPromise = true; + } + index++; + } + + return containsPromise + ? Promise.all(completedResults).then((resolved) => [resolved, acc[1]]) + : acc; +} + +function completeIterableValueWithPossibleStream( + exeContext: ExecutionContext, + itemType: GraphQLOutputType, + fieldGroup: FieldGroup, + info: GraphQLResolveInfo, + path: Path, + items: Iterable, + streamUsage: StreamUsage, + incrementalContext: IncrementalContext | undefined, + deferMap: ReadonlyMap | undefined, +): PromiseOrValue>> { + // This is specified as a simple map, however we're optimizing the path + // where the list contains no Promises by avoiding creating another Promise. + let containsPromise = false; + const completedResults: Array = []; + const acc: GraphQLResult> = [completedResults, []]; + let index = 0; + const initialCount = streamUsage.initialCount; const iterator = items[Symbol.iterator](); let iteration = iterator.next(); while (!iteration.done) { const item = iteration.value; - if (streamUsage && index >= streamUsage.initialCount) { + if (index >= initialCount) { const streamRecord = new StreamRecord({ label: streamUsage.label, path,