do not store initialResultRecord on context

as then memory for the result record tree cannot be freed
graphql · Jul 3, 2023 · f4b118e · f4b118e
1 parent 6e3eecd
commit f4b118e
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 93 deletions.
diff --git a/src/execution/IncrementalPublisher.ts b/src/execution/IncrementalPublisher.ts
@@ -97,34 +97,17 @@ export type FormattedIncrementalResult<
  * parents have completed so that they can no longer be filtered. This includes all Incremental
  * Data records in `released`, as well as Incremental Data records that have not yet completed.
  *
- * `_initialResult`: a record containing the state of the initial result, as follows:
- * `isCompleted`: indicates whether the initial result has completed.
- * `children`: the set of Incremental Data records that can be be published when the initial
- * result is completed.
- *
- * Each Incremental Data record also contains similar metadata, i.e. these records also contain
- * similar `isCompleted` and `children` properties.
- *
  * @internal
  */
 export class IncrementalPublisher {
-  private _initialResult: {
-    children: Set<IncrementalDataRecord>;
-    isCompleted: boolean;
-  };
-
-  private _released: Set<IncrementalDataRecord>;
-  private _pending: Set<IncrementalDataRecord>;
+  private _released: Set<SubsequentDataRecord>;
+  private _pending: Set<SubsequentDataRecord>;
 
   // these are assigned within the Promise executor called synchronously within the constructor
   private _signalled!: Promise<unknown>;
   private _resolve!: () => void;
 
   constructor() {
-    this._initialResult = {
-      children: new Set(),
-      isCompleted: false,
-    };
     this._released = new Set();
     this._pending = new Set();
     this._reset();
@@ -210,19 +193,23 @@ export class IncrementalPublisher {
     };
   }
 
+  prepareInitialResultRecord(): InitialResultRecord {
+    return {
+      errors: [],
+      children: new Set(),
+      isCompleted: false,
+    };
+  }
+
   prepareNewDeferredFragmentRecord(opts: {
     label: string | undefined;
     path: Path | undefined;
-    parentContext: IncrementalDataRecord | undefined;
+    parentContext: IncrementalDataRecord;
   }): DeferredFragmentRecord {
     const deferredFragmentRecord = new DeferredFragmentRecord(opts);
 
     const parentContext = opts.parentContext;
-    if (parentContext) {
-      parentContext.children.add(deferredFragmentRecord);
-    } else {
-      this._initialResult.children.add(deferredFragmentRecord);
-    }
+    parentContext.children.add(deferredFragmentRecord);
 
     return deferredFragmentRecord;
   }
@@ -231,16 +218,12 @@ export class IncrementalPublisher {
     label: string | undefined;
     path: Path | undefined;
     asyncIterator?: AsyncIterator<unknown>;
-    parentContext: IncrementalDataRecord | undefined;
+    parentContext: IncrementalDataRecord;
   }): StreamItemsRecord {
     const streamItemsRecord = new StreamItemsRecord(opts);
 
     const parentContext = opts.parentContext;
-    if (parentContext) {
-      parentContext.children.add(streamItemsRecord);
-    } else {
-      this._initialResult.children.add(streamItemsRecord);
-    }
+    parentContext.children.add(streamItemsRecord);
 
     return streamItemsRecord;
   }
@@ -274,29 +257,31 @@ export class IncrementalPublisher {
     incrementalDataRecord.errors.push(error);
   }
 
-  publishInitial() {
-    for (const child of this._initialResult.children) {
+  publishInitial(initialResult: InitialResultRecord) {
+    for (const child of initialResult.children) {
       if (child.filtered) {
         continue;
       }
       this._publish(child);
     }
   }
 
-  filter(
-    nullPath: Path,
-    erroringIncrementalDataRecord: IncrementalDataRecord | undefined,
-  ) {
+  getInitialErrors(
+    initialResult: InitialResultRecord,
+  ): ReadonlyArray<GraphQLError> {
+    return initialResult.errors;
+  }
+
+  filter(nullPath: Path, erroringIncrementalDataRecord: IncrementalDataRecord) {
     const nullPathArray = pathToArray(nullPath);
 
     const asyncIterators = new Set<AsyncIterator<unknown>>();
 
-    const children =
-      erroringIncrementalDataRecord === undefined
-        ? this._initialResult.children
-        : erroringIncrementalDataRecord.children;
+    const descendants = this._getDescendants(
+      erroringIncrementalDataRecord.children,
+    );
 
-    for (const child of this._getDescendants(children)) {
+    for (const child of descendants) {
       if (!this._matchesPath(child.path, nullPathArray)) {
         continue;
       }
@@ -332,31 +317,31 @@ export class IncrementalPublisher {
     this._signalled = signalled;
   }
 
-  private _introduce(item: IncrementalDataRecord) {
+  private _introduce(item: SubsequentDataRecord) {
     this._pending.add(item);
   }
 
-  private _release(item: IncrementalDataRecord): void {
+  private _release(item: SubsequentDataRecord): void {
     if (this._pending.has(item)) {
       this._released.add(item);
       this._trigger();
     }
   }
 
-  private _push(item: IncrementalDataRecord): void {
+  private _push(item: SubsequentDataRecord): void {
     this._released.add(item);
     this._pending.add(item);
     this._trigger();
   }
 
-  private _delete(item: IncrementalDataRecord) {
+  private _delete(item: SubsequentDataRecord) {
     this._released.delete(item);
     this._pending.delete(item);
     this._trigger();
   }
 
   private _getIncrementalResult(
-    completedRecords: ReadonlySet<IncrementalDataRecord>,
+    completedRecords: ReadonlySet<SubsequentDataRecord>,
   ): SubsequentIncrementalExecutionResult | undefined {
     const incrementalResults: Array<IncrementalResult> = [];
     let encounteredCompletedAsyncIterator = false;
@@ -398,18 +383,18 @@ export class IncrementalPublisher {
       : undefined;
   }
 
-  private _publish(incrementalDataRecord: IncrementalDataRecord) {
-    if (incrementalDataRecord.isCompleted) {
-      this._push(incrementalDataRecord);
+  private _publish(subsequentResultRecord: SubsequentDataRecord) {
+    if (subsequentResultRecord.isCompleted) {
+      this._push(subsequentResultRecord);
     } else {
-      this._introduce(incrementalDataRecord);
+      this._introduce(subsequentResultRecord);
     }
   }
 
   private _getDescendants(
-    children: ReadonlySet<IncrementalDataRecord>,
-    descendants = new Set<IncrementalDataRecord>(),
-  ): ReadonlySet<IncrementalDataRecord> {
+    children: ReadonlySet<SubsequentDataRecord>,
+    descendants = new Set<SubsequentDataRecord>(),
+  ): ReadonlySet<SubsequentDataRecord> {
     for (const child of children) {
       descendants.add(child);
       this._getDescendants(child.children, descendants);
@@ -431,13 +416,19 @@ export class IncrementalPublisher {
   }
 }
 
+export interface InitialResultRecord {
+  errors: Array<GraphQLError>;
+  children: Set<SubsequentDataRecord>;
+  isCompleted: boolean;
+}
+
 /** @internal */
 export class DeferredFragmentRecord {
   errors: Array<GraphQLError>;
   label: string | undefined;
   path: Array<string | number>;
   data: ObjMap<unknown> | null;
-  children: Set<IncrementalDataRecord>;
+  children: Set<SubsequentDataRecord>;
   isCompleted: boolean;
   filtered: boolean;
   constructor(opts: { label: string | undefined; path: Path | undefined }) {
@@ -457,7 +448,7 @@ export class StreamItemsRecord {
   label: string | undefined;
   path: Array<string | number>;
   items: Array<unknown> | null;
-  children: Set<IncrementalDataRecord>;
+  children: Set<SubsequentDataRecord>;
   asyncIterator: AsyncIterator<unknown> | undefined;
   isCompletedAsyncIterator?: boolean;
   isCompleted: boolean;
@@ -479,10 +470,12 @@ export class StreamItemsRecord {
   }
 }
 
-export type IncrementalDataRecord = DeferredFragmentRecord | StreamItemsRecord;
+export type SubsequentDataRecord = DeferredFragmentRecord | StreamItemsRecord;
+
+export type IncrementalDataRecord = InitialResultRecord | SubsequentDataRecord;
 
 function isStreamItemsRecord(
-  incrementalDataRecord: IncrementalDataRecord,
-): incrementalDataRecord is StreamItemsRecord {
-  return incrementalDataRecord instanceof StreamItemsRecord;
+  subsequentResultRecord: SubsequentDataRecord,
+): subsequentResultRecord is StreamItemsRecord {
+  return subsequentResultRecord instanceof StreamItemsRecord;
 }