Skip to content

Commit

Permalink
memoize Path generation so paths can be used as keys directly across …
Browse files Browse the repository at this point in the history
…branches
  • Loading branch information
yaacovCR committed Feb 6, 2023
1 parent 5b6ad7d commit b057818
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 50 deletions.
4 changes: 2 additions & 2 deletions src/execution/__tests__/defer-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ describe('Execute: defer directive', () => {
]);
});

it('Can deduplicate initial fields with deferred fragments at multiple levels', async () => {
it('Can deduplicate fields with deferred fragments at multiple levels', async () => {
const document = parse(`
query {
hero {
Expand Down Expand Up @@ -732,7 +732,7 @@ describe('Execute: defer directive', () => {
]);
});

it('can deduplicate initial fields with deferred fragments in different branches at multiple non-overlapping levels', async () => {
it('can deduplicate fields with deferred fragments in different branches at multiple non-overlapping levels', async () => {
const document = parse(`
query {
a {
Expand Down
35 changes: 22 additions & 13 deletions src/execution/__tests__/executor-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import { expectJSON } from '../../__testUtils__/expectJSON.js';
import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';

import { inspect } from '../../jsutils/inspect.js';
import type { Path } from '../../jsutils/Path.js';

import { Kind } from '../../language/kinds.js';
import { parse } from '../../language/parser.js';

import type { GraphQLResolveInfo } from '../../type/definition.js';
import {
GraphQLInterfaceType,
GraphQLList,
Expand Down Expand Up @@ -191,7 +193,7 @@ describe('Execute: Handles basic execution tasks', () => {
});

it('provides info about current execution state', () => {
let resolvedInfo;
let resolvedInfo: GraphQLResolveInfo | undefined;
const testType = new GraphQLObjectType({
name: 'Test',
fields: {
Expand Down Expand Up @@ -239,13 +241,18 @@ describe('Execute: Handles basic execution tasks', () => {
const field = operation.selectionSet.selections[0];
expect(resolvedInfo).to.deep.include({
fieldNodes: [field],
path: { prev: undefined, key: 'result', typename: 'Test' },
variableValues: { var: 'abc' },
});

expect(resolvedInfo?.path).to.deep.include({
prev: undefined,
key: 'result',
typename: 'Test',
});
});

it('populates path correctly with complex types', () => {
let path;
let path: Path | undefined;
const someObject = new GraphQLObjectType({
name: 'SomeObject',
fields: {
Expand Down Expand Up @@ -288,18 +295,20 @@ describe('Execute: Handles basic execution tasks', () => {

executeSync({ schema, document, rootValue });

expect(path).to.deep.equal({
expect(path).to.deep.include({
key: 'l2',
typename: 'SomeObject',
prev: {
key: 0,
typename: undefined,
prev: {
key: 'l1',
typename: 'SomeQuery',
prev: undefined,
},
},
});

expect(path?.prev).to.deep.include({
key: 0,
typename: undefined,
});

expect(path?.prev?.prev).to.deep.include({
key: 'l1',
typename: 'SomeQuery',
prev: undefined,
});
});

Expand Down
28 changes: 15 additions & 13 deletions src/execution/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ import { isPromise } from '../jsutils/isPromise.js';
import type { Maybe } from '../jsutils/Maybe.js';
import { memoize3 } from '../jsutils/memoize3.js';
import type { ObjMap } from '../jsutils/ObjMap.js';
import type { Path } from '../jsutils/Path.js';
import { addPath, pathToArray } from '../jsutils/Path.js';
import type { Path, PathFactory } from '../jsutils/Path.js';
import { createPathFactory, pathToArray } from '../jsutils/Path.js';
import { promiseForObject } from '../jsutils/promiseForObject.js';
import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js';
import { promiseReduce } from '../jsutils/promiseReduce.js';
Expand Down Expand Up @@ -122,9 +122,10 @@ export interface ExecutionContext {
fieldResolver: GraphQLFieldResolver<any, any>;
typeResolver: GraphQLTypeResolver<any, any>;
subscribeFieldResolver: GraphQLFieldResolver<any, any>;
addPath: PathFactory;
errors: Array<GraphQLError>;
subsequentPayloads: Set<AsyncPayloadRecord>;
branches: WeakMap<GroupedFieldSet, Set<string>>;
branches: WeakMap<GroupedFieldSet, Set<Path | undefined>>;
}

/**
Expand Down Expand Up @@ -502,6 +503,7 @@ export function buildExecutionContext(
fieldResolver: fieldResolver ?? defaultFieldResolver,
typeResolver: typeResolver ?? defaultTypeResolver,
subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
addPath: createPathFactory(),
subsequentPayloads: new Set(),
branches: new WeakMap(),
errors: [],
Expand All @@ -515,6 +517,7 @@ function buildPerEventExecutionContext(
return {
...exeContext,
rootValue: payload,
addPath: createPathFactory(),
subsequentPayloads: new Set(),
branches: new WeakMap(),
errors: [],
Expand All @@ -527,13 +530,12 @@ function shouldBranch(
path: Path | undefined,
): boolean {
const set = exeContext.branches.get(groupedFieldSet);
const key = pathToArray(path).join('.');
if (set === undefined) {
exeContext.branches.set(groupedFieldSet, new Set([key]));
exeContext.branches.set(groupedFieldSet, new Set([path]));
return true;
}
if (!set.has(key)) {
set.add(key);
if (!set.has(path)) {
set.add(path);
return true;
}
return false;
Expand Down Expand Up @@ -627,7 +629,7 @@ function executeFieldsSerially(
return promiseReduce(
groupedFieldSet,
(results, [responseName, fieldGroup]) => {
const fieldPath = addPath(path, responseName, parentType.name);
const fieldPath = exeContext.addPath(path, responseName, parentType.name);

const fieldName = fieldGroup[0].fieldNode.name.value;
const fieldDef = exeContext.schema.getField(parentType, fieldName);
Expand Down Expand Up @@ -702,7 +704,7 @@ function executeFields(

try {
for (const [responseName, fieldGroup] of groupedFieldSet) {
const fieldPath = addPath(path, responseName, parentType.name);
const fieldPath = exeContext.addPath(path, responseName, parentType.name);

const fieldName = fieldGroup[0].fieldNode.name.value;
const fieldDef = exeContext.schema.getField(parentType, fieldName);
Expand Down Expand Up @@ -1141,7 +1143,7 @@ async function completeAsyncIteratorValue(
break;
}

const itemPath = addPath(path, index, undefined);
const itemPath = exeContext.addPath(path, index, undefined);
let iteration;
try {
// eslint-disable-next-line no-await-in-loop
Expand Down Expand Up @@ -1227,7 +1229,7 @@ function completeListValue(
for (const item of result) {
// 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);
const itemPath = exeContext.addPath(path, index, undefined);

if (
stream &&
Expand Down Expand Up @@ -1815,7 +1817,7 @@ function executeSubscription(
);
}

const path = addPath(undefined, responseName, rootType.name);
const path = exeContext.addPath(undefined, responseName, rootType.name);
const info = buildResolveInfo(
exeContext,
fieldDef,
Expand Down Expand Up @@ -2100,7 +2102,7 @@ async function executeStreamIterator(
let previousAsyncPayloadRecord = parentContext ?? undefined;
// eslint-disable-next-line no-constant-condition
while (true) {
const itemPath = addPath(path, index, undefined);
const itemPath = exeContext.addPath(path, index, undefined);
const asyncPayloadRecord = new StreamRecord({
path: itemPath,
deferDepth,
Expand Down
65 changes: 54 additions & 11 deletions src/jsutils/Path.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,39 @@
import type { Maybe } from './Maybe.js';

export interface Path {
/**
* @internal
*/
export class Path {
readonly prev: Path | undefined;
readonly key: string | number;
readonly typename: string | undefined;
}

/**
* Given a Path and a key, return a new Path containing the new key.
*/
export function addPath(
prev: Readonly<Path> | undefined,
key: string | number,
typename: string | undefined,
): Path {
return { prev, key, typename };
readonly _subPaths: Map<string | number, Path>;

constructor(
prev: Path | undefined,
key: string | number,
typename: string | undefined,
) {
this.prev = prev;
this.key = key;
this.typename = typename;
this._subPaths = new Map();
}

/**
* Given a Path and a key, return a new Path containing the new key.
*/
addPath(key: string | number, typeName: string | undefined): Path {
let path = this._subPaths.get(key);
if (path !== undefined) {
return path;
}

path = new Path(this, key, typeName);
this._subPaths.set(key, path);
return path;
}
}

/**
Expand All @@ -31,3 +50,27 @@ export function pathToArray(
}
return flattened.reverse();
}

export type PathFactory = (
path: Path | undefined,
key: string | number,
typeName: string | undefined,
) => Path;

export function createPathFactory(): PathFactory {
const paths = new Map<string, Path>();
return (path, key, typeName) => {
if (path !== undefined) {
return path.addPath(key, typeName);
}

let newPath = paths.get(key as string);
if (newPath !== undefined) {
return newPath;
}

newPath = new Path(undefined, key, typeName);
paths.set(key as string, newPath);
return newPath;
};
}
18 changes: 9 additions & 9 deletions src/jsutils/__tests__/Path-test.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,34 @@
import { expect } from 'chai';
import { describe, it } from 'mocha';

import { addPath, pathToArray } from '../Path.js';
import { Path, pathToArray } from '../Path.js';

describe('Path', () => {
it('can create a Path', () => {
const first = addPath(undefined, 1, 'First');
const first = new Path(undefined, 1, 'First');

expect(first).to.deep.equal({
expect(first).to.deep.include({
prev: undefined,
key: 1,
typename: 'First',
});
});

it('can add a new key to an existing Path', () => {
const first = addPath(undefined, 1, 'First');
const second = addPath(first, 'two', 'Second');
const first = new Path(undefined, 1, 'First');
const second = first.addPath('two', 'Second');

expect(second).to.deep.equal({
expect(second).to.deep.include({
prev: first,
key: 'two',
typename: 'Second',
});
});

it('can convert a Path to an array of its keys', () => {
const root = addPath(undefined, 0, 'Root');
const first = addPath(root, 'one', 'First');
const second = addPath(first, 2, 'Second');
const root = new Path(undefined, 0, 'Root');
const first = root.addPath('one', 'First');
const second = first.addPath(2, 'Second');

const path = pathToArray(second);
expect(path).to.deep.equal([0, 'one', 2]);
Expand Down
13 changes: 11 additions & 2 deletions src/utilities/coerceInputValue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import { inspect } from '../jsutils/inspect.js';
import { invariant } from '../jsutils/invariant.js';
import { isIterableObject } from '../jsutils/isIterableObject.js';
import { isObjectLike } from '../jsutils/isObjectLike.js';
import type { Path } from '../jsutils/Path.js';
import { addPath, pathToArray } from '../jsutils/Path.js';
import { Path, pathToArray } from '../jsutils/Path.js';
import { printPathArray } from '../jsutils/printPathArray.js';
import { suggestionList } from '../jsutils/suggestionList.js';

Expand Down Expand Up @@ -48,6 +47,16 @@ function defaultOnError(
throw error;
}

function addPath(
path: Path | undefined,
key: string | number,
typeName: string | undefined,
): Path {
return path
? path.addPath(key, typeName)
: new Path(undefined, key, typeName);
}

function coerceInputValueImpl(
inputValue: unknown,
type: GraphQLInputType,
Expand Down

0 comments on commit b057818

Please sign in to comment.