Skip to content

Commit 9e5df54

Browse files
sachindshindesachindshinde
committed
Separate fields-fragment memoization from fragment-fragment memoization
1 parent 4c0a2d1 commit 9e5df54

File tree

1 file changed

+64
-30
lines changed

1 file changed

+64
-30
lines changed

‎src/validation/rules/OverlappingFieldsCanBeMergedRule.ts‎

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,14 @@ function reasonMessage(reason: ConflictReasonMessage): string {
5959
export function OverlappingFieldsCanBeMergedRule(
6060
context: ValidationContext,
6161
): ASTVisitor {
62-
// A memoization for when two fragments are compared "between" each other for
63-
// conflicts. Two fragments may be compared many times, so memoizing this can
64-
// dramatically improve the performance of this validator.
65-
const comparedFragmentPairs = new PairSet();
62+
// A memoization for when fields and a fragment or two fragments are compared
63+
// "between" each other for conflicts. Comparisons made be made many times,
64+
// so memoizing this can dramatically improve the performance of this validator.
65+
const comparedFieldsAndFragmentPairs = new PairSet<
66+
NodeAndDefCollection,
67+
string
68+
>((a, _) => typeof a === 'string');
69+
const comparedFragmentPairs = new PairSet<string, string>((a, b) => a < b);
6670

6771
// A cache for the "field map" and list of fragment names found in any given
6872
// selection set. Selection sets may be asked for this information multiple
@@ -74,6 +78,7 @@ export function OverlappingFieldsCanBeMergedRule(
7478
const conflicts = findConflictsWithinSelectionSet(
7579
context,
7680
cachedFieldsAndFragmentNames,
81+
comparedFieldsAndFragmentPairs,
7782
comparedFragmentPairs,
7883
context.getParentType(),
7984
selectionSet,
@@ -168,7 +173,8 @@ type FieldsAndFragmentNames = readonly [NodeAndDefCollection, FragmentNames];
168173
function findConflictsWithinSelectionSet(
169174
context: ValidationContext,
170175
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
171-
comparedFragmentPairs: PairSet,
176+
comparedFieldsAndFragmentPairs: PairSet<NodeAndDefCollection, string>,
177+
comparedFragmentPairs: PairSet<string, string>,
172178
parentType: Maybe<GraphQLNamedType>,
173179
selectionSet: SelectionSetNode,
174180
): Array<Conflict> {
@@ -187,6 +193,7 @@ function findConflictsWithinSelectionSet(
187193
context,
188194
conflicts,
189195
cachedFieldsAndFragmentNames,
196+
comparedFieldsAndFragmentPairs,
190197
comparedFragmentPairs,
191198
fieldMap,
192199
);
@@ -199,6 +206,7 @@ function findConflictsWithinSelectionSet(
199206
context,
200207
conflicts,
201208
cachedFieldsAndFragmentNames,
209+
comparedFieldsAndFragmentPairs,
202210
comparedFragmentPairs,
203211
false,
204212
fieldMap,
@@ -213,6 +221,7 @@ function findConflictsWithinSelectionSet(
213221
context,
214222
conflicts,
215223
cachedFieldsAndFragmentNames,
224+
comparedFieldsAndFragmentPairs,
216225
comparedFragmentPairs,
217226
false,
218227
fragmentNames[i],
@@ -230,17 +239,28 @@ function collectConflictsBetweenFieldsAndFragment(
230239
context: ValidationContext,
231240
conflicts: Array<Conflict>,
232241
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
233-
comparedFragmentPairs: PairSet,
242+
comparedFieldsAndFragmentPairs: PairSet<NodeAndDefCollection, string>,
243+
comparedFragmentPairs: PairSet<string, string>,
234244
areMutuallyExclusive: boolean,
235245
fieldMap: NodeAndDefCollection,
236246
fragmentName: string,
237247
): void {
238248
// Memoize so the fields and fragments are not compared for conflicts more
239249
// than once.
240-
if (comparedFragmentPairs.has(fieldMap, fragmentName, areMutuallyExclusive)) {
250+
if (
251+
comparedFieldsAndFragmentPairs.has(
252+
fieldMap,
253+
fragmentName,
254+
areMutuallyExclusive,
255+
)
256+
) {
241257
return;
242258
}
243-
comparedFragmentPairs.add(fieldMap, fragmentName, areMutuallyExclusive);
259+
comparedFieldsAndFragmentPairs.add(
260+
fieldMap,
261+
fragmentName,
262+
areMutuallyExclusive,
263+
);
244264

245265
const fragment = context.getFragment(fragmentName);
246266
if (!fragment) {
@@ -265,6 +285,7 @@ function collectConflictsBetweenFieldsAndFragment(
265285
context,
266286
conflicts,
267287
cachedFieldsAndFragmentNames,
288+
comparedFieldsAndFragmentPairs,
268289
comparedFragmentPairs,
269290
areMutuallyExclusive,
270291
fieldMap,
@@ -278,6 +299,7 @@ function collectConflictsBetweenFieldsAndFragment(
278299
context,
279300
conflicts,
280301
cachedFieldsAndFragmentNames,
302+
comparedFieldsAndFragmentPairs,
281303
comparedFragmentPairs,
282304
areMutuallyExclusive,
283305
fieldMap,
@@ -292,7 +314,8 @@ function collectConflictsBetweenFragments(
292314
context: ValidationContext,
293315
conflicts: Array<Conflict>,
294316
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
295-
comparedFragmentPairs: PairSet,
317+
comparedFieldsAndFragmentPairs: PairSet<NodeAndDefCollection, string>,
318+
comparedFragmentPairs: PairSet<string, string>,
296319
areMutuallyExclusive: boolean,
297320
fragmentName1: string,
298321
fragmentName2: string,
@@ -339,6 +362,7 @@ function collectConflictsBetweenFragments(
339362
context,
340363
conflicts,
341364
cachedFieldsAndFragmentNames,
365+
comparedFieldsAndFragmentPairs,
342366
comparedFragmentPairs,
343367
areMutuallyExclusive,
344368
fieldMap1,
@@ -352,6 +376,7 @@ function collectConflictsBetweenFragments(
352376
context,
353377
conflicts,
354378
cachedFieldsAndFragmentNames,
379+
comparedFieldsAndFragmentPairs,
355380
comparedFragmentPairs,
356381
areMutuallyExclusive,
357382
fragmentName1,
@@ -366,6 +391,7 @@ function collectConflictsBetweenFragments(
366391
context,
367392
conflicts,
368393
cachedFieldsAndFragmentNames,
394+
comparedFieldsAndFragmentPairs,
369395
comparedFragmentPairs,
370396
areMutuallyExclusive,
371397
referencedFragmentName1,
@@ -380,7 +406,8 @@ function collectConflictsBetweenFragments(
380406
function findConflictsBetweenSubSelectionSets(
381407
context: ValidationContext,
382408
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
383-
comparedFragmentPairs: PairSet,
409+
comparedFieldsAndFragmentPairs: PairSet<NodeAndDefCollection, string>,
410+
comparedFragmentPairs: PairSet<string, string>,
384411
areMutuallyExclusive: boolean,
385412
parentType1: Maybe<GraphQLNamedType>,
386413
selectionSet1: SelectionSetNode,
@@ -407,6 +434,7 @@ function findConflictsBetweenSubSelectionSets(
407434
context,
408435
conflicts,
409436
cachedFieldsAndFragmentNames,
437+
comparedFieldsAndFragmentPairs,
410438
comparedFragmentPairs,
411439
areMutuallyExclusive,
412440
fieldMap1,
@@ -420,6 +448,7 @@ function findConflictsBetweenSubSelectionSets(
420448
context,
421449
conflicts,
422450
cachedFieldsAndFragmentNames,
451+
comparedFieldsAndFragmentPairs,
423452
comparedFragmentPairs,
424453
areMutuallyExclusive,
425454
fieldMap1,
@@ -434,6 +463,7 @@ function findConflictsBetweenSubSelectionSets(
434463
context,
435464
conflicts,
436465
cachedFieldsAndFragmentNames,
466+
comparedFieldsAndFragmentPairs,
437467
comparedFragmentPairs,
438468
areMutuallyExclusive,
439469
fieldMap2,
@@ -450,6 +480,7 @@ function findConflictsBetweenSubSelectionSets(
450480
context,
451481
conflicts,
452482
cachedFieldsAndFragmentNames,
483+
comparedFieldsAndFragmentPairs,
453484
comparedFragmentPairs,
454485
areMutuallyExclusive,
455486
fragmentName1,
@@ -465,7 +496,8 @@ function collectConflictsWithin(
465496
context: ValidationContext,
466497
conflicts: Array<Conflict>,
467498
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
468-
comparedFragmentPairs: PairSet,
499+
comparedFieldsAndFragmentPairs: PairSet<NodeAndDefCollection, string>,
500+
comparedFragmentPairs: PairSet<string, string>,
469501
fieldMap: NodeAndDefCollection,
470502
): void {
471503
// A field map is a keyed collection, where each key represents a response
@@ -482,6 +514,7 @@ function collectConflictsWithin(
482514
const conflict = findConflict(
483515
context,
484516
cachedFieldsAndFragmentNames,
517+
comparedFieldsAndFragmentPairs,
485518
comparedFragmentPairs,
486519
false, // within one collection is never mutually exclusive
487520
responseName,
@@ -506,7 +539,8 @@ function collectConflictsBetween(
506539
context: ValidationContext,
507540
conflicts: Array<Conflict>,
508541
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
509-
comparedFragmentPairs: PairSet,
542+
comparedFieldsAndFragmentPairs: PairSet<NodeAndDefCollection, string>,
543+
comparedFragmentPairs: PairSet<string, string>,
510544
parentFieldsAreMutuallyExclusive: boolean,
511545
fieldMap1: NodeAndDefCollection,
512546
fieldMap2: NodeAndDefCollection,
@@ -524,6 +558,7 @@ function collectConflictsBetween(
524558
const conflict = findConflict(
525559
context,
526560
cachedFieldsAndFragmentNames,
561+
comparedFieldsAndFragmentPairs,
527562
comparedFragmentPairs,
528563
parentFieldsAreMutuallyExclusive,
529564
responseName,
@@ -544,7 +579,8 @@ function collectConflictsBetween(
544579
function findConflict(
545580
context: ValidationContext,
546581
cachedFieldsAndFragmentNames: Map<SelectionSetNode, FieldsAndFragmentNames>,
547-
comparedFragmentPairs: PairSet,
582+
comparedFieldsAndFragmentPairs: PairSet<NodeAndDefCollection, string>,
583+
comparedFragmentPairs: PairSet<string, string>,
548584
parentFieldsAreMutuallyExclusive: boolean,
549585
responseName: string,
550586
field1: NodeAndDef,
@@ -615,6 +651,7 @@ function findConflict(
615651
const conflicts = findConflictsBetweenSubSelectionSets(
616652
context,
617653
cachedFieldsAndFragmentNames,
654+
comparedFieldsAndFragmentPairs,
618655
comparedFragmentPairs,
619656
areMutuallyExclusive,
620657
getNamedType(type1),
@@ -806,20 +843,20 @@ function subfieldConflicts(
806843
/**
807844
* A way to keep track of pairs of things when the ordering of the pair does not matter.
808845
*/
809-
class PairSet {
810-
_data: Map<string | NodeAndDefCollection, Map<string, boolean>>;
846+
class PairSet<T, U> {
847+
_data: Map<T | U, Map<T | U, boolean>>;
848+
_orderer: (a: T | U, b: T | U) => [T | U, T | U];
811849

812-
constructor() {
850+
constructor(comparator: (a: T | U, b: T | U) => boolean) {
813851
this._data = new Map();
852+
this._orderer = (a: T | U, b: T | U) =>
853+
comparator(a, b) ? [a, b] : [b, a];
814854
}
815855

816-
has(
817-
a: string | NodeAndDefCollection,
818-
b: string,
819-
areMutuallyExclusive: boolean,
820-
): boolean {
821-
const [key1, key2] =
822-
typeof a !== 'string' ? [a, b] : a < b ? [a, b] : [b, a];
856+
has(a: T, b: U, areMutuallyExclusive: boolean): boolean;
857+
has(a: U, b: T, areMutuallyExclusive: boolean): boolean;
858+
has(a: T | U, b: T | U, areMutuallyExclusive: boolean): boolean {
859+
const [key1, key2] = this._orderer(a, b);
823860

824861
const result = this._data.get(key1)?.get(key2);
825862
if (result === undefined) {
@@ -832,13 +869,10 @@ class PairSet {
832869
return areMutuallyExclusive ? true : areMutuallyExclusive === result;
833870
}
834871

835-
add(
836-
a: string | NodeAndDefCollection,
837-
b: string,
838-
areMutuallyExclusive: boolean,
839-
): void {
840-
const [key1, key2] =
841-
typeof a !== 'string' ? [a, b] : a < b ? [a, b] : [b, a];
872+
add(a: T, b: U, areMutuallyExclusive: boolean): void;
873+
add(a: U, b: T, areMutuallyExclusive: boolean): void;
874+
add(a: T | U, b: T | U, areMutuallyExclusive: boolean): void {
875+
const [key1, key2] = this._orderer(a, b);
842876

843877
const map = this._data.get(key1);
844878
if (map === undefined) {

0 commit comments

Comments
 (0)