Skip to content

Commit 0c1ad08

Browse files
authored
Merge pull request #44 from mcg-web/removed_memoization_on_executor_resolve_field
Removed memoization on executor resolveField (see #43)
2 parents 0e1929f + 3ae6c73 commit 0c1ad08

File tree

2 files changed

+49
-175
lines changed

2 files changed

+49
-175
lines changed

src/Executor/Executor.php

Lines changed: 49 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -360,89 +360,58 @@ private static function resolveField(ExecutionContext $exeContext, ObjectType $p
360360
{
361361
$fieldAST = $fieldASTs[0];
362362

363-
$uid = self::getFieldUid($fieldAST, $parentType);
364-
365-
// Get memoized variables if they exist
366-
if (isset($exeContext->memoized['resolveField'][$uid])) {
367-
$memoized = $exeContext->memoized['resolveField'][$uid];
368-
$fieldDef = $memoized['fieldDef'];
369-
$returnType = $fieldDef->getType();
370-
$args = $memoized['args'];
371-
$info = $memoized['info'];
372-
}
373-
else {
374-
$fieldName = $fieldAST->name->value;
375-
376-
$fieldDef = self::getFieldDef($exeContext->schema, $parentType, $fieldName);
377-
378-
if (!$fieldDef) {
379-
return self::$UNDEFINED;
380-
}
381-
382-
$returnType = $fieldDef->getType();
383-
384-
// Build hash of arguments from the field.arguments AST, using the
385-
// variables scope to fulfill any variable references.
386-
$args = Values::getArgumentValues(
387-
$fieldDef->args,
388-
$fieldAST->arguments,
389-
$exeContext->variableValues
390-
);
391-
392-
// The resolve function's optional third argument is a collection of
393-
// information about the current execution state.
394-
$info = new ResolveInfo([
395-
'fieldName' => $fieldName,
396-
'fieldASTs' => $fieldASTs,
397-
'returnType' => $returnType,
398-
'parentType' => $parentType,
399-
'schema' => $exeContext->schema,
400-
'fragments' => $exeContext->fragments,
401-
'rootValue' => $exeContext->rootValue,
402-
'operation' => $exeContext->operation,
403-
'variableValues' => $exeContext->variableValues,
404-
]);
405-
406-
// Memoizing results for same query field
407-
// (useful for lists when several values are resolved against the same field)
408-
$exeContext->memoized['resolveField'][$uid] = $memoized = [
409-
'fieldDef' => $fieldDef,
410-
'args' => $args,
411-
'info' => $info,
412-
'results' => new \SplObjectStorage
413-
];
414-
}
415-
416-
// When source value is object it is possible to memoize certain subset of results
417-
$isObject = is_object($source);
418-
419-
if ($isObject && isset($memoized['results'][$source])) {
420-
$result = $memoized['results'][$source];
363+
$fieldName = $fieldAST->name->value;
364+
365+
$fieldDef = self::getFieldDef($exeContext->schema, $parentType, $fieldName);
366+
367+
if (!$fieldDef) {
368+
return self::$UNDEFINED;
369+
}
370+
371+
$returnType = $fieldDef->getType();
372+
373+
// Build hash of arguments from the field.arguments AST, using the
374+
// variables scope to fulfill any variable references.
375+
$args = Values::getArgumentValues(
376+
$fieldDef->args,
377+
$fieldAST->arguments,
378+
$exeContext->variableValues
379+
);
380+
381+
// The resolve function's optional third argument is a collection of
382+
// information about the current execution state.
383+
$info = new ResolveInfo([
384+
'fieldName' => $fieldName,
385+
'fieldASTs' => $fieldASTs,
386+
'returnType' => $returnType,
387+
'parentType' => $parentType,
388+
'schema' => $exeContext->schema,
389+
'fragments' => $exeContext->fragments,
390+
'rootValue' => $exeContext->rootValue,
391+
'operation' => $exeContext->operation,
392+
'variableValues' => $exeContext->variableValues,
393+
]);
394+
395+
396+
if (isset($fieldDef->resolveFn)) {
397+
$resolveFn = $fieldDef->resolveFn;
398+
} else if (isset($parentType->resolveFieldFn)) {
399+
$resolveFn = $parentType->resolveFieldFn;
421400
} else {
422-
if (isset($fieldDef->resolveFn)) {
423-
$resolveFn = $fieldDef->resolveFn;
424-
} else if (isset($parentType->resolveFieldFn)) {
425-
$resolveFn = $parentType->resolveFieldFn;
426-
} else {
427-
$resolveFn = self::$defaultResolveFn;
428-
}
401+
$resolveFn = self::$defaultResolveFn;
402+
}
429403

430-
// Get the resolve function, regardless of if its result is normal
431-
// or abrupt (error).
432-
$result = self::resolveOrError($resolveFn, $source, $args, $info);
404+
// Get the resolve function, regardless of if its result is normal
405+
// or abrupt (error).
406+
$result = self::resolveOrError($resolveFn, $source, $args, $info);
433407

434-
$result = self::completeValueCatchingError(
435-
$exeContext,
436-
$returnType,
437-
$fieldASTs,
438-
$info,
439-
$result
440-
);
441-
442-
if ($isObject) {
443-
$memoized['results'][$source] = $result;
444-
}
445-
}
408+
$result = self::completeValueCatchingError(
409+
$exeContext,
410+
$returnType,
411+
$fieldASTs,
412+
$info,
413+
$result
414+
);
446415

447416
return $result;
448417
}

tests/Executor/ExecutorTest.php

Lines changed: 0 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -547,99 +547,4 @@ public function testSubstitutesArgumentWithDefaultValue()
547547

548548
$this->assertEquals($expected, $result->toArray());
549549
}
550-
551-
public function testResolvedValueIsMemoized()
552-
{
553-
$doc = '
554-
query Q {
555-
a {
556-
b {
557-
c
558-
d
559-
}
560-
}
561-
}
562-
';
563-
564-
$memoizedValue = new \ArrayObject([
565-
'b' => 'id1'
566-
]);
567-
568-
$A = null;
569-
570-
$Test = new ObjectType([
571-
'name' => 'Test',
572-
'fields' => [
573-
'a' => [
574-
'type' => function() use (&$A) {return Type::listOf($A);},
575-
'resolve' => function() use ($memoizedValue) {
576-
return [
577-
$memoizedValue,
578-
new \ArrayObject([
579-
'b' => 'id2',
580-
]),
581-
$memoizedValue,
582-
new \ArrayObject([
583-
'b' => 'id2',
584-
])
585-
];
586-
}
587-
]
588-
]
589-
]);
590-
591-
$callCounts = ['id1' => 0, 'id2' => 0];
592-
593-
$A = new ObjectType([
594-
'name' => 'A',
595-
'fields' => [
596-
'b' => [
597-
'type' => new ObjectType([
598-
'name' => 'B',
599-
'fields' => [
600-
'c' => ['type' => Type::string()],
601-
'd' => ['type' => Type::string()]
602-
]
603-
]),
604-
'resolve' => function($value) use (&$callCounts) {
605-
$callCounts[$value['b']]++;
606-
607-
switch ($value['b']) {
608-
case 'id1':
609-
return [
610-
'c' => 'c1',
611-
'd' => 'd1'
612-
];
613-
case 'id2':
614-
return [
615-
'c' => 'c2',
616-
'd' => 'd2'
617-
];
618-
}
619-
}
620-
]
621-
]
622-
]);
623-
624-
// Test that value resolved once is memoized for same query field
625-
$schema = new Schema($Test);
626-
627-
$query = Parser::parse($doc);
628-
$result = Executor::execute($schema, $query);
629-
$expected = [
630-
'data' => [
631-
'a' => [
632-
['b' => ['c' => 'c1', 'd' => 'd1']],
633-
['b' => ['c' => 'c2', 'd' => 'd2']],
634-
['b' => ['c' => 'c1', 'd' => 'd1']],
635-
['b' => ['c' => 'c2', 'd' => 'd2']],
636-
]
637-
]
638-
];
639-
640-
$this->assertEquals($expected, $result->toArray());
641-
642-
$this->assertSame($callCounts['id1'], 1); // Result for id1 is expected to be memoized after first call
643-
$this->assertSame($callCounts['id2'], 2);
644-
}
645550
}

0 commit comments

Comments
 (0)