Grafast: move to bitwise flag-based system internally (#2014)

.changeset/soft-nails-speak.md

@@ -0,0 +1,5 @@
+---
+"grafast": patch
+---
+
+Refactor internal planning logic to use new bitwise flags-based filtering.

grafast/grafast/src/bucket.ts

@@ -1,8 +1,13 @@
 // import type { GraphQLScalarType } from "graphql";
 
+import type { ExecutableStep } from ".";
 import type { LayerPlan } from "./engine/LayerPlan";
 import type { MetaByMetaKey } from "./engine/OperationPlan";
-import type { ExecutionEventEmitter, ExecutionValue } from "./interfaces.js";
+import type {
+ ExecutionEntryFlags,
+ ExecutionEventEmitter,
+ ExecutionValue,
+} from "./interfaces.js";
 
 /**
  * @internal
@@ -45,6 +50,9 @@ export interface RequestTools {
  * @internal
  */
 export interface Bucket {
+ /** @internal */
+ toString?(): string;
+
  /**
  * The LayerPlan definition this bucket adheres to
  */
@@ -91,22 +99,27 @@ export interface Bucket {
  */
  store: Map<number, ExecutionValue>;
 
+ setResult(
+ step: ExecutableStep,
+ index: number,
+ value: any,
+ flags: ExecutionEntryFlags,
+ ): void;
+
  /**
  * Set this true when the bucket is fully executed.
  *
  * Initialize it to false.
  */
  isComplete: boolean;
 
- // PERF: we should be able to convert this into a set of planIds that have
- // errors, then we can use this as we cascade forward to the next bucket.
  /**
- * If an error occurred at any stage we need to drop down to more careful
- * (and slower) handling.
+ * A union of all the ExecutionEntryStates in this bucket. Generally, if it's
+ * non-zero then we need to perform careful (and slower) handling.
  *
- * Initialize it to false.
+ * Initialize it to 0.
  */
- hasErrors: boolean;
+ flagUnion: ExecutionEntryFlags;
 
  /**
  * The child buckets of this bucket.

grafast/grafast/src/engine/LayerPlan.ts

@@ -7,6 +7,11 @@ import type { Bucket } from "../bucket.js";
 import type { GrafastError } from "../error.js";
 import { isGrafastError } from "../error.js";
 import { inspect } from "../inspect.js";
+import type { UnaryExecutionValue } from "../interfaces.js";
+import {
+ FORBIDDEN_BY_NULLABLE_BOUNDARY_FLAGS,
+ NO_FLAGS,
+} from "../interfaces.js";
 import { resolveType } from "../polymorphic.js";
 import type {
  ExecutableStep,
@@ -399,8 +404,12 @@ export class LayerPlan<TReason extends LayerPlanReason = LayerPlanReason> {
  const hasNoNullsOrErrors = false;
 
  if (this.rootStep._isUnary) {
- const fieldValue = parentBucket.store.get(itemStepId)!;
- if (fieldValue.value == null) {
+ const fieldValue = parentBucket.store.get(
+ itemStepId,
+ )! as UnaryExecutionValue;
+ const forbiddenFlags =
+ fieldValue._entryFlags & FORBIDDEN_BY_NULLABLE_BOUNDARY_FLAGS;
+ if (forbiddenFlags !== NO_FLAGS) {
  size = 0;
  } else {
  size = parentBucket.size;
@@ -482,9 +491,8 @@ export class LayerPlan<TReason extends LayerPlanReason = LayerPlanReason> {
  for (const stepId of copyStepIds) {
  const ev = store.get(stepId)!;
  if (ev.isBatch) {
- (ev.entries as any[])[newIndex] = parentBucket.store
- .get(stepId)!
- .at(originalIndex);
+ const orig = parentBucket.store.get(stepId)!;
+ ev._copyResult(newIndex, orig, originalIndex);
  }
  }
  }
@@ -558,9 +566,8 @@ export class LayerPlan<TReason extends LayerPlanReason = LayerPlanReason> {
  for (const stepId of copyStepIds) {
  const ev = store.get(stepId)!;
  if (ev.isBatch) {
- (ev.entries as any[])[newIndex] = parentBucket.store
- .get(stepId)!
- .at(originalIndex);
+ const orig = parentBucket.store.get(stepId)!;
+ ev._copyResult(newIndex, orig, originalIndex);
  }
  }
  }
@@ -644,9 +651,8 @@ export class LayerPlan<TReason extends LayerPlanReason = LayerPlanReason> {
  for (const planId of copyStepIds) {
  const ev = store.get(planId)!;
  if (ev.isBatch) {
- (ev.entries as any[])[newIndex] = parentBucket.store
- .get(planId)!
- .at(originalIndex);
+ const orig = parentBucket.store.get(planId)!;
+ ev._copyResult(newIndex, orig, originalIndex);
  }
  }
  }
@@ -687,7 +693,7 @@ export class LayerPlan<TReason extends LayerPlanReason = LayerPlanReason> {
  size,
  store,
  // PERF: not necessarily, if we don't copy the errors, we don't have the errors.
- hasErrors: parentBucket.hasErrors,
+ flagUnion: parentBucket.flagUnion,
  polymorphicPathList,
  iterators,
  },
@@ -751,7 +757,7 @@ ${inner}
  size,
  store,
  // PERF: not necessarily, if we don't copy the errors, we don't have the errors.
- hasErrors: parentBucket.hasErrors,
+ flagUnion: parentBucket.flagUnion,
  polymorphicPathList,
  iterators,
  }, parentBucket.metaByMetaKey);

grafast/grafast/src/engine/StepTracker.ts

@@ -1,15 +1,17 @@
 import { isDev } from "../dev.js";
 import type { OperationPlan } from "../index.js";
-import { $$subroutine } from "../interfaces.js";
-import type { ExecutableStep } from "../step";
+import { inspect } from "../inspect.js";
+import type { AddStepDependencyOptions } from "../interfaces.js";
+import { $$subroutine, ALL_FLAGS, TRAPPABLE_FLAGS } from "../interfaces.js";
+import { ExecutableStep } from "../step.js";
 import { sudo } from "../utils.js";
 import type {
  LayerPlan,
  LayerPlanReasonSubroutine,
  LayerPlanReasonsWithParentStep,
-} from "./LayerPlan";
+} from "./LayerPlan.js";
 import { lock } from "./lock.js";
-import type { OutputPlan } from "./OutputPlan";
+import type { OutputPlan } from "./OutputPlan.js";
 
 /**
  * We want everything else to treat things like `dependencies` as read only,
@@ -274,9 +276,12 @@ export class StepTracker {
  }
 
  public addStepDependency(
- $dependent: ExecutableStep,
- $dependency: ExecutableStep,
+ raw$dependent: ExecutableStep,
+ raw$dependency: ExecutableStep,
+ options: AddStepDependencyOptions = {},
  ): number {
+ const $dependent = sudo(raw$dependent);
+ const $dependency = sudo(raw$dependency);
  if (!this.activeSteps.has($dependent)) {
  throw new Error(
  `Cannot add ${$dependency} as a dependency of ${$dependent}; the latter is deleted!`,
@@ -287,37 +292,82 @@ export class StepTracker {
  `Cannot add ${$dependency} as a dependency of ${$dependent}; the former is deleted!`,
  );
  }
+ if ($dependent.isFinalized) {
+ throw new Error(
+ "You cannot add a dependency after the step is finalized.",
+ );
+ }
+ if (!($dependency instanceof ExecutableStep)) {
+ throw new Error(
+ `Error occurred when adding dependency for '${$dependent}', value passed was not a step, it was '${inspect(
+ $dependency,
+ )}'`,
+ );
+ }
+ if (isDev) {
+ // Check that we can actually add this as a dependency
+ if (!$dependent.layerPlan.ancestry.includes($dependency.layerPlan)) {
+ throw new Error(
+ //console.error(
+ // This is not a GrafastInternalError
+ `Attempted to add '${$dependency}' (${$dependency.layerPlan}) as a dependency of '${$dependent}' (${$dependent.layerPlan}), but we cannot because that LayerPlan isn't an ancestor`,
+ );
+ }
+ }
+
+ const dependentDependencies = writeableArray($dependent.dependencies);
+ const dependentDependencyForbiddenFlags = writeableArray(
+ $dependent.dependencyForbiddenFlags,
+ );
+ const {
+ skipDeduplication,
+ acceptFlags = ALL_FLAGS & ~$dependent.defaultForbiddenFlags,
+ } = options;
+ // When copying dependencies between classes, we might not want to
+ // deduplicate because we might refer to the dependency by its index. As
+ // such, we should only dedupe by default but allow opting out.
+ // TODO: change this to `!skipDeduplication`
+ if (skipDeduplication === false) {
+ const existingIndex = dependentDependencies.indexOf($dependency);
+ if (existingIndex >= 0) {
+ return existingIndex;
+ }
+ }
+
+ if (!$dependency._isUnary && $dependent._isUnary) {
+ if ($dependent._isUnaryLocked) {
+ throw new Error(
+ `Attempted to add non-unary step ${$dependency} as a dependency of ${$dependent}; but the latter is unary, so it cannot depend on batch steps`,
+ );
+ }
+ $dependent._isUnary = false;
+ }
+
+ const forbiddenFlags = ALL_FLAGS & ~(acceptFlags & TRAPPABLE_FLAGS);
+
  this.stepsWithNoDependencies.delete($dependent);
- const dependencyIndex =
-  writeableArray(sudo($dependent).dependencies).push($dependency) - 1;
+ const dependencyIndex = dependentDependencies.push($dependency) - 1;
+ dependentDependencyForbiddenFlags[dependencyIndex] = forbiddenFlags;
  writeableArray($dependency.dependents).push({
  step: $dependent,
  dependencyIndex,
  });
- if (!$dependency._isUnary) {
- if ($dependent._isUnary) {
- if ($dependent._isUnaryLocked) {
- throw new Error(
- `Attempted to add non-unary step ${$dependency} as a dependency of ${$dependent}; but the latter is unary, so it cannot depend on batch steps`,
- );
- }
- $dependent._isUnary = false;
- }
- }
+
  return dependencyIndex;
  }
 
  public addStepUnaryDependency(
  $dependent: ExecutableStep,
  $dependency: ExecutableStep,
+ options: AddStepDependencyOptions = {},
  ): number {
  if (!$dependency._isUnary) {
  throw new Error(
  `${$dependent} attempted to create a unary step dependency on ${$dependency}, but that step is not unary. You may use \`.addDependency()\` instead of \`.addUnaryDependency()\`.`,
  );
  }
  $dependency._isUnaryLocked = true;
- return this.addStepDependency($dependent, $dependency);
+ return this.addStepDependency($dependent, $dependency, options);
  }
 
  public setOutputPlanRootStep(