Skip to content

Commit

Permalink
visitor: speed up visitInParallel by dropping support for unknown AST…
Browse files Browse the repository at this point in the history
… nodes
  • Loading branch information
IvanGoncharov committed Sep 23, 2021
1 parent 0091421 commit 2617b6d
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 102 deletions.
25 changes: 25 additions & 0 deletions benchmark/visit-benchmark.js
@@ -0,0 +1,25 @@
'use strict';

const { parse } = require('graphql/language/parser.js');
const { visit } = require('graphql/language/visitor.js');

const { bigSchemaSDL } = require('./fixtures.js');

const documentAST = parse(bigSchemaSDL);

const visitor = {
enter() {
/* do nothing */
},
leave() {
/* do nothing */
},
};

module.exports = {
name: 'Visit all AST nodes',
count: 10,
measure() {
visit(documentAST, visitor);
},
};
25 changes: 25 additions & 0 deletions benchmark/visitInParallel-benchmark.js
@@ -0,0 +1,25 @@
'use strict';

const { parse } = require('graphql/language/parser.js');
const { visit, visitInParallel } = require('graphql/language/visitor.js');

const { bigSchemaSDL } = require('./fixtures.js');

const documentAST = parse(bigSchemaSDL);

const visitors = new Array(50).fill({
enter() {
/* do nothing */
},
leave() {
/* do nothing */
},
});

module.exports = {
name: 'Visit all AST nodes in parallel',
count: 10,
measure() {
visit(documentAST, visitInParallel(visitors));
},
};
1 change: 1 addition & 0 deletions src/index.ts
Expand Up @@ -209,6 +209,7 @@ export {
visit,
visitInParallel,
getVisitFn,
getEnterLeaveForKind,
BREAK,
Kind,
DirectiveLocation,
Expand Down
43 changes: 0 additions & 43 deletions src/language/__tests__/visitor-test.ts
Expand Up @@ -420,49 +420,6 @@ describe('Visitor', () => {
]);
});

it('visit nodes with unknown kinds but does not traverse deeper', () => {
const customAST = parse('{ a }');
// @ts-expect-error
customAST.definitions[0].selectionSet.selections.push({
kind: 'CustomField',
name: { kind: 'Name', value: 'NamedNodeToBeSkipped' },
selectionSet: {
kind: 'SelectionSet',
selections: [
{
kind: 'CustomField',
name: { kind: 'Name', value: 'NamedNodeToBeSkipped' },
},
],
},
});

const visited: Array<any> = [];
visit(customAST, {
enter(node) {
visited.push(['enter', node.kind, getValue(node)]);
},
leave(node) {
visited.push(['leave', node.kind, getValue(node)]);
},
});

expect(visited).to.deep.equal([
['enter', 'Document', undefined],
['enter', 'OperationDefinition', undefined],
['enter', 'SelectionSet', undefined],
['enter', 'Field', undefined],
['enter', 'Name', 'a'],
['leave', 'Name', 'a'],
['leave', 'Field', undefined],
['enter', 'CustomField', undefined],
['leave', 'CustomField', undefined],
['leave', 'SelectionSet', undefined],
['leave', 'OperationDefinition', undefined],
['leave', 'Document', undefined],
]);
});

it('visits only the specified `Kind` in visitorKeyMap', () => {
const visited: Array<any> = [];

Expand Down
8 changes: 7 additions & 1 deletion src/language/index.ts
Expand Up @@ -18,7 +18,13 @@ export type { ParseOptions } from './parser';

export { print } from './printer';

export { visit, visitInParallel, getVisitFn, BREAK } from './visitor';
export {
visit,
visitInParallel,
getVisitFn,
getEnterLeaveForKind,
BREAK,
} from './visitor';
export type { ASTVisitor, ASTVisitFn, ASTVisitorKeyMap } from './visitor';

export { Location, Token } from './ast';
Expand Down
150 changes: 95 additions & 55 deletions src/language/visitor.ts
@@ -1,8 +1,8 @@
import { inspect } from '../jsutils/inspect';
import type { Maybe } from '../jsutils/Maybe';

import type { ASTNode, ASTKindToNode } from './ast';
import { isNode } from './ast';
import { Kind } from './kinds';

/**
* A visitor is provided to visit, it contains the collection of
Expand Down Expand Up @@ -256,6 +256,15 @@ export function visit(
visitor: ASTVisitor | ASTReducer<any>,
visitorKeys: ASTVisitorKeyMap = QueryDocumentKeys,
): any {
const enterLeaveMap = new Map<
keyof ASTKindToNode,
EnterLeaveVisitor<ASTNode>
>();

for (const kind of Object.values(Kind)) {
enterLeaveMap.set(kind, getEnterLeaveForKind(visitor, kind));
}

/* eslint-disable no-undef-init */
let stack: any = undefined;
let inArray = Array.isArray(root);
Expand Down Expand Up @@ -318,29 +327,30 @@ export function visit(
if (!isNode(node)) {
throw new Error(`Invalid AST Node: ${inspect(node)}.`);
}
const visitFn = getVisitFn(visitor, node.kind, isLeaving);
if (visitFn) {
result = visitFn.call(visitor, node, key, parent, path, ancestors);
const visitFn = isLeaving
? enterLeaveMap.get(node.kind)?.leave
: enterLeaveMap.get(node.kind)?.enter;

if (result === BREAK) {
break;
}
result = visitFn?.call(visitor, node, key, parent, path, ancestors);

if (result === BREAK) {
break;
}

if (result === false) {
if (!isLeaving) {
if (result === false) {
if (!isLeaving) {
path.pop();
continue;
}
} else if (result !== undefined) {
edits.push([key, result]);
if (!isLeaving) {
if (isNode(result)) {
node = result;
} else {
path.pop();
continue;
}
} else if (result !== undefined) {
edits.push([key, result]);
if (!isLeaving) {
if (isNode(result)) {
node = result;
} else {
path.pop();
continue;
}
}
}
}
}
Expand Down Expand Up @@ -380,16 +390,31 @@ export function visit(
export function visitInParallel(
visitors: ReadonlyArray<ASTVisitor>,
): ASTVisitor {
const skipping = new Array(visitors.length);

return {
enter(...args) {
const node = args[0];
for (let i = 0; i < visitors.length; i++) {
if (skipping[i] == null) {
const fn = getVisitFn(visitors[i], node.kind, /* isLeaving */ false);
if (fn) {
const result = fn.apply(visitors[i], args);
const skipping = new Array(visitors.length).fill(null);
const mergedVisitor = Object.create(null);

for (const kind of Object.values(Kind)) {
let hasVisitor = false;
const enterList = new Array(visitors.length).fill(undefined);
const leaveList = new Array(visitors.length).fill(undefined);

for (let i = 0; i < visitors.length; ++i) {
const { enter, leave } = getEnterLeaveForKind(visitors[i], kind);
hasVisitor ||= enter != null || leave != null;
enterList[i] = enter;
leaveList[i] = leave;
}

if (!hasVisitor) {
continue;
}

const mergedEnterLeave: EnterLeaveVisitor<ASTNode> = {
enter(...args) {
const node = args[0];
for (let i = 0; i < visitors.length; i++) {
if (skipping[i] === null) {
const result = enterList[i]?.apply(visitors[i], args);
if (result === false) {
skipping[i] = node;
} else if (result === BREAK) {
Expand All @@ -399,50 +424,65 @@ export function visitInParallel(
}
}
}
}
},
leave(...args) {
const node = args[0];
for (let i = 0; i < visitors.length; i++) {
if (skipping[i] == null) {
const fn = getVisitFn(visitors[i], node.kind, /* isLeaving */ true);
if (fn) {
const result = fn.apply(visitors[i], args);
},
leave(...args) {
const node = args[0];
for (let i = 0; i < visitors.length; i++) {
if (skipping[i] === null) {
const result = leaveList[i]?.apply(visitors[i], args);
if (result === BREAK) {
skipping[i] = BREAK;
} else if (result !== undefined && result !== false) {
return result;
}
} else if (skipping[i] === node) {
skipping[i] = null;
}
} else if (skipping[i] === node) {
skipping[i] = null;
}
}
},
};
},
};

mergedVisitor[kind] = mergedEnterLeave;
}

return mergedVisitor;
}

/**
* Given a visitor instance, if it is leaving or not, and a node kind, return
* the function the visitor runtime should call.
* Given a visitor instance and a node kind, return EnterLeaveVisitor for that kind.
*/
export function getVisitFn(
export function getEnterLeaveForKind(
visitor: ASTVisitor,
kind: keyof ASTKindToNode,
isLeaving: boolean,
): Maybe<ASTVisitFn<ASTNode>> {
): EnterLeaveVisitor<ASTNode> {
const kindVisitor:
| ASTVisitFn<ASTNode>
| EnterLeaveVisitor<ASTNode>
| undefined = (visitor as any)[kind];
if (kindVisitor) {
if (typeof kindVisitor === 'function') {
// { Kind() {} }
return isLeaving ? undefined : kindVisitor;
}

if (typeof kindVisitor === 'object') {
// { Kind: { enter() {}, leave() {} } }
return isLeaving ? kindVisitor.leave : kindVisitor.enter;
return kindVisitor;
} else if (typeof kindVisitor === 'function') {
// { Kind() {} }
return { enter: kindVisitor, leave: undefined };
}

// { enter() {}, leave() {} }
return isLeaving ? (visitor as any).leave : (visitor as any).enter;
return { enter: (visitor as any).enter, leave: (visitor as any).leave };
}

/**
* Given a visitor instance, if it is leaving or not, and a node kind, return
* the function the visitor runtime should call.
*
* @deprecated Please use `getEnterLeaveForKind` instead. Will be removed in v17
*/
export function getVisitFn(
visitor: ASTVisitor,
kind: keyof ASTKindToNode,
isLeaving: boolean,
): ASTVisitFn<ASTNode> | undefined {
const { enter, leave } = getEnterLeaveForKind(visitor, kind);
return isLeaving ? leave : enter;
}
6 changes: 3 additions & 3 deletions src/utilities/TypeInfo.ts
Expand Up @@ -2,7 +2,7 @@ import type { ASTVisitor } from '../language/visitor';
import type { ASTNode, FieldNode } from '../language/ast';
import { Kind } from '../language/kinds';
import { isNode } from '../language/ast';
import { getVisitFn } from '../language/visitor';
import { getEnterLeaveForKind } from '../language/visitor';

import type { Maybe } from '../jsutils/Maybe';

Expand Down Expand Up @@ -344,7 +344,7 @@ export function visitWithTypeInfo(
enter(...args) {
const node = args[0];
typeInfo.enter(node);
const fn = getVisitFn(visitor, node.kind, /* isLeaving */ false);
const fn = getEnterLeaveForKind(visitor, node.kind).enter;
if (fn) {
const result = fn.apply(visitor, args);
if (result !== undefined) {
Expand All @@ -358,7 +358,7 @@ export function visitWithTypeInfo(
},
leave(...args) {
const node = args[0];
const fn = getVisitFn(visitor, node.kind, /* isLeaving */ true);
const fn = getEnterLeaveForKind(visitor, node.kind).leave;
let result;
if (fn) {
result = fn.apply(visitor, args);
Expand Down

0 comments on commit 2617b6d

Please sign in to comment.