Skip to content

Commit 0bc9088

Browse files
committed
[Validation] Performance improvements
Fairly dramatic improvement of some validation rules by short-circuiting branches which do not need to be checked and memoizing the process of collecting fragment spreads from a given context - which is necessary by multiple rules.
1 parent 7278b7c commit 0bc9088

9 files changed

+57
-43
lines changed

src/validation/rules/ArgumentsOfCorrectType.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export function ArgumentsOfCorrectType(context: ValidationContext): any {
3838
[ argAST.value ]
3939
);
4040
}
41+
return false;
4142
}
4243
};
4344
}

src/validation/rules/DefaultValuesOfCorrectType.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ export function DefaultValuesOfCorrectType(context: ValidationContext): any {
5757
[ defaultValue ]
5858
);
5959
}
60-
}
60+
return false;
61+
},
62+
SelectionSet: () => false,
63+
FragmentDefinition: () => false,
6164
};
6265
}

src/validation/rules/NoFragmentCycles.js

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,7 @@
1010

1111
import type { ValidationContext } from '../index';
1212
import { GraphQLError } from '../../error';
13-
import { FRAGMENT_SPREAD } from '../../language/kinds';
14-
import type {
15-
SelectionSet,
16-
FragmentSpread,
17-
FragmentDefinition
18-
} from '../../language/ast';
13+
import type { FragmentDefinition } from '../../language/ast';
1914

2015

2116
export function cycleErrorMessage(
@@ -63,8 +58,7 @@ export function NoFragmentCycles(context: ValidationContext): any {
6358
const fragmentName = fragment.name.value;
6459
visitedFrags[fragmentName] = true;
6560

66-
const spreadNodes = [];
67-
gatherSpreads(spreadNodes, fragment.selectionSet);
61+
const spreadNodes = context.getFragmentSpreads(fragment);
6862
if (spreadNodes.length === 0) {
6963
return;
7064
}
@@ -100,19 +94,3 @@ export function NoFragmentCycles(context: ValidationContext): any {
10094
spreadPathIndexByName[fragmentName] = undefined;
10195
}
10296
}
103-
104-
/**
105-
* Given an operation or fragment AST node, gather all the
106-
* named spreads defined within the scope of the fragment
107-
* or operation
108-
*/
109-
function gatherSpreads(spreads: Array<FragmentSpread>, node: SelectionSet) {
110-
for (let i = 0; i < node.selections.length; i++) {
111-
const selection = node.selections[i];
112-
if (selection.kind === FRAGMENT_SPREAD) {
113-
spreads.push(selection);
114-
} else if (selection.selectionSet) {
115-
gatherSpreads(spreads, selection.selectionSet);
116-
}
117-
}
118-
}

src/validation/rules/NoUnusedFragments.js

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
* of patent rights can be found in the PATENTS file in the same directory.
99
*/
1010

11+
import type { ValidationContext } from '../index';
1112
import { GraphQLError } from '../../error';
1213

1314

@@ -21,36 +22,30 @@ export function unusedFragMessage(fragName: any): string {
2122
* A GraphQL document is only valid if all fragment definitions are spread
2223
* within operations, or spread within other fragments spread within operations.
2324
*/
24-
export function NoUnusedFragments(): any {
25-
var fragmentDefs = [];
25+
export function NoUnusedFragments(context: ValidationContext): any {
2626
var spreadsWithinOperation = [];
27-
var fragAdjacencies = {};
28-
var spreadNames = {};
27+
var fragmentDefs = [];
2928

3029
return {
31-
OperationDefinition() {
32-
spreadNames = {};
33-
spreadsWithinOperation.push(spreadNames);
30+
OperationDefinition(node) {
31+
spreadsWithinOperation.push(context.getFragmentSpreads(node));
32+
return false;
3433
},
3534
FragmentDefinition(def) {
3635
fragmentDefs.push(def);
37-
spreadNames = {};
38-
fragAdjacencies[def.name.value] = spreadNames;
39-
},
40-
FragmentSpread(spread) {
41-
spreadNames[spread.name.value] = true;
36+
return false;
4237
},
4338
Document: {
4439
leave() {
4540
var fragmentNameUsed = {};
4641
var reduceSpreadFragments = function (spreads) {
47-
var keys = Object.keys(spreads);
48-
keys.forEach(fragName => {
42+
spreads.forEach(spread => {
43+
const fragName = spread.name.value;
4944
if (fragmentNameUsed[fragName] !== true) {
5045
fragmentNameUsed[fragName] = true;
51-
var adjacencies = fragAdjacencies[fragName];
52-
if (adjacencies) {
53-
reduceSpreadFragments(adjacencies);
46+
const fragment = context.getFragment(fragName);
47+
if (fragment) {
48+
reduceSpreadFragments(context.getFragmentSpreads(fragment));
5449
}
5550
}
5651
});

src/validation/rules/UniqueArgumentNames.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export function UniqueArgumentNames(): any {
3939
);
4040
}
4141
knownArgNames[argName] = node.name;
42+
return false;
4243
}
4344
};
4445
}

src/validation/rules/UniqueFragmentNames.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export function duplicateFragmentNameMessage(fragName: any): string {
2323
export function UniqueFragmentNames(): any {
2424
var knownFragmentNames = Object.create(null);
2525
return {
26+
OperationDefinition: () => false,
2627
FragmentDefinition(node) {
2728
var fragmentName = node.name.value;
2829
if (knownFragmentNames[fragmentName]) {
@@ -32,6 +33,7 @@ export function UniqueFragmentNames(): any {
3233
);
3334
}
3435
knownFragmentNames[fragmentName] = node.name;
36+
return false;
3537
}
3638
};
3739
}

src/validation/rules/UniqueInputFieldNames.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export function UniqueInputFieldNames(): any {
4444
);
4545
}
4646
knownNames[fieldName] = node.name;
47+
return false;
4748
}
4849
};
4950
}

src/validation/rules/UniqueOperationNames.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ export function UniqueOperationNames(): any {
3434
}
3535
knownOperationNames[operationName.value] = operationName;
3636
}
37-
}
37+
return false;
38+
},
39+
FragmentDefinition: () => false,
3840
};
3941
}

src/validation/validate.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import { visit, getVisitFn } from '../language/visitor';
1414
import * as Kind from '../language/kinds';
1515
import type {
1616
Document,
17+
OperationDefinition,
18+
SelectionSet,
19+
FragmentSpread,
1720
FragmentDefinition,
1821
} from '../language/ast';
1922
import { GraphQLSchema } from '../type/schema';
@@ -171,6 +174,8 @@ function append(arr, items) {
171174
}
172175
}
173176

177+
type HasSelectionSet = OperationDefinition | FragmentDefinition;
178+
174179
/**
175180
* An instance of this class is passed as the "this" context to all validators,
176181
* allowing access to commonly useful contextual information from within a
@@ -181,11 +186,13 @@ export class ValidationContext {
181186
_ast: Document;
182187
_typeInfo: TypeInfo;
183188
_fragments: {[name: string]: FragmentDefinition};
189+
_fragmentSpreads: Map<HasSelectionSet, Array<FragmentSpread>>;
184190

185191
constructor(schema: GraphQLSchema, ast: Document, typeInfo: TypeInfo) {
186192
this._schema = schema;
187193
this._ast = ast;
188194
this._typeInfo = typeInfo;
195+
this._fragmentSpreads = new Map();
189196
}
190197

191198
getSchema(): GraphQLSchema {
@@ -210,6 +217,16 @@ export class ValidationContext {
210217
return fragments[name];
211218
}
212219

220+
getFragmentSpreads(node: HasSelectionSet): Array<FragmentSpread> {
221+
let spreads = this._fragmentSpreads.get(node);
222+
if (!spreads) {
223+
spreads = [];
224+
gatherSpreads(spreads, node.selectionSet);
225+
this._fragmentSpreads.set(node, spreads);
226+
}
227+
return spreads;
228+
}
229+
213230
getType(): ?GraphQLOutputType {
214231
return this._typeInfo.getType();
215232
}
@@ -234,3 +251,17 @@ export class ValidationContext {
234251
return this._typeInfo.getArgument();
235252
}
236253
}
254+
255+
/**
256+
* Given a selection set, gather all the named spreads defined within.
257+
*/
258+
function gatherSpreads(spreads: Array<FragmentSpread>, node: SelectionSet) {
259+
for (let i = 0; i < node.selections.length; i++) {
260+
const selection = node.selections[i];
261+
if (selection.kind === Kind.FRAGMENT_SPREAD) {
262+
spreads.push(selection);
263+
} else if (selection.selectionSet) {
264+
gatherSpreads(spreads, selection.selectionSet);
265+
}
266+
}
267+
}

0 commit comments

Comments
 (0)