Skip to content

Commit

Permalink
perf: use undefined for empty (#4046)
Browse files Browse the repository at this point in the history
As convention, we will lazily instantiate arrays/sets when adding the first item.

This applies to arrays/sets on execution/incremental context, as well as the second member of the GraphQLWrappedResult tuple holding the array of incremental data records.
  • Loading branch information
yaacovCR committed May 8, 2024
1 parent d245e65 commit 92f9bb0
Show file tree
Hide file tree
Showing 2 changed files with 150 additions and 77 deletions.
31 changes: 20 additions & 11 deletions src/execution/IncrementalPublisher.ts
@@ -1,3 +1,4 @@
import { invariant } from '../jsutils/invariant.js';
import { isPromise } from '../jsutils/isPromise.js';
import type { ObjMap } from '../jsutils/ObjMap.js';
import type { Path } from '../jsutils/Path.js';
Expand Down Expand Up @@ -172,7 +173,7 @@ export interface FormattedCompletedResult {
export function buildIncrementalResponse(
context: IncrementalPublisherContext,
result: ObjMap<unknown>,
errors: ReadonlyArray<GraphQLError>,
errors: ReadonlyArray<GraphQLError> | undefined,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
): ExperimentalIncrementalExecutionResults {
const incrementalPublisher = new IncrementalPublisher(context);
Expand All @@ -184,7 +185,7 @@ export function buildIncrementalResponse(
}

interface IncrementalPublisherContext {
cancellableStreams: Set<CancellableStreamRecord>;
cancellableStreams: Set<CancellableStreamRecord> | undefined;
}

/**
Expand Down Expand Up @@ -218,7 +219,7 @@ class IncrementalPublisher {

buildResponse(
data: ObjMap<unknown>,
errors: ReadonlyArray<GraphQLError>,
errors: ReadonlyArray<GraphQLError> | undefined,
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>,
): ExperimentalIncrementalExecutionResults {
this._addIncrementalDataRecords(incrementalDataRecords);
Expand All @@ -227,7 +228,7 @@ class IncrementalPublisher {
const pending = this._pendingSourcesToResults();

const initialResult: InitialIncrementalExecutionResult =
errors.length === 0
errors === undefined
? { data, pending, hasNext: true }
: { errors, data, pending, hasNext: true };

Expand Down Expand Up @@ -444,8 +445,12 @@ class IncrementalPublisher {
};

const returnStreamIterators = async (): Promise<void> => {
const cancellableStreams = this._context.cancellableStreams;
if (cancellableStreams === undefined) {
return;
}
const promises: Array<Promise<unknown>> = [];
for (const streamRecord of this._context.cancellableStreams) {
for (const streamRecord of cancellableStreams) {
if (streamRecord.earlyReturn !== undefined) {
promises.push(streamRecord.earlyReturn());
}
Expand Down Expand Up @@ -519,9 +524,11 @@ class IncrementalPublisher {
);
}

this._addIncrementalDataRecords(
deferredGroupedFieldSetResult.incrementalDataRecords,
);
const incrementalDataRecords =
deferredGroupedFieldSetResult.incrementalDataRecords;
if (incrementalDataRecords !== undefined) {
this._addIncrementalDataRecords(incrementalDataRecords);
}

for (const deferredFragmentRecord of deferredGroupedFieldSetResult.deferredFragmentRecords) {
const id = deferredFragmentRecord.id;
Expand Down Expand Up @@ -587,6 +594,7 @@ class IncrementalPublisher {
});
this._pending.delete(streamRecord);
if (isCancellableStreamRecord(streamRecord)) {
invariant(this._context.cancellableStreams !== undefined);
this._context.cancellableStreams.delete(streamRecord);
streamRecord.earlyReturn().catch(() => {
/* c8 ignore next 1 */
Expand All @@ -597,6 +605,7 @@ class IncrementalPublisher {
this._completed.push({ id });
this._pending.delete(streamRecord);
if (isCancellableStreamRecord(streamRecord)) {
invariant(this._context.cancellableStreams !== undefined);
this._context.cancellableStreams.delete(streamRecord);
}
} else {
Expand All @@ -607,7 +616,7 @@ class IncrementalPublisher {

this._incremental.push(incrementalEntry);

if (streamItemsResult.incrementalDataRecords.length > 0) {
if (streamItemsResult.incrementalDataRecords !== undefined) {
this._addIncrementalDataRecords(
streamItemsResult.incrementalDataRecords,
);
Expand Down Expand Up @@ -675,7 +684,7 @@ interface ReconcilableDeferredGroupedFieldSetResult {
deferredFragmentRecords: ReadonlyArray<DeferredFragmentRecord>;
path: Array<string | number>;
result: BareDeferredGroupedFieldSetResult;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord> | undefined;
sent?: true | undefined;
errors?: never;
}
Expand Down Expand Up @@ -743,7 +752,7 @@ function isCancellableStreamRecord(
interface ReconcilableStreamItemsResult {
streamRecord: SubsequentResultRecord;
result: BareStreamItemsResult;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord>;
incrementalDataRecords: ReadonlyArray<IncrementalDataRecord> | undefined;
errors?: never;
}

Expand Down

0 comments on commit 92f9bb0

Please sign in to comment.