From 3ae6c733670a597abe3058a993bc39d422d9f230 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Fri, 10 Jun 2016 11:57:43 +0200 Subject: [PATCH] Removed memoization on executor resolveField (see #43) --- src/Executor/Executor.php | 121 ++++++++++++-------------------- tests/Executor/ExecutorTest.php | 95 ------------------------- 2 files changed, 45 insertions(+), 171 deletions(-) diff --git a/src/Executor/Executor.php b/src/Executor/Executor.php index 9de5acb..ffa7714 100644 --- a/src/Executor/Executor.php +++ b/src/Executor/Executor.php @@ -360,90 +360,59 @@ class Executor { $fieldAST = $fieldASTs[0]; - $uid = self::getFieldUid($fieldAST, $parentType); + $fieldName = $fieldAST->name->value; - // Get memoized variables if they exist - if (isset($exeContext->memoized['resolveField'][$uid])) { - $memoized = $exeContext->memoized['resolveField'][$uid]; - $fieldDef = $memoized['fieldDef']; - $returnType = $fieldDef->getType(); - $args = $memoized['args']; - $info = $memoized['info']; - } - else { - $fieldName = $fieldAST->name->value; + $fieldDef = self::getFieldDef($exeContext->schema, $parentType, $fieldName); - $fieldDef = self::getFieldDef($exeContext->schema, $parentType, $fieldName); - - if (!$fieldDef) { - return self::$UNDEFINED; - } - - $returnType = $fieldDef->getType(); - - // Build hash of arguments from the field.arguments AST, using the - // variables scope to fulfill any variable references. - $args = Values::getArgumentValues( - $fieldDef->args, - $fieldAST->arguments, - $exeContext->variableValues - ); - - // The resolve function's optional third argument is a collection of - // information about the current execution state. - $info = new ResolveInfo([ - 'fieldName' => $fieldName, - 'fieldASTs' => $fieldASTs, - 'returnType' => $returnType, - 'parentType' => $parentType, - 'schema' => $exeContext->schema, - 'fragments' => $exeContext->fragments, - 'rootValue' => $exeContext->rootValue, - 'operation' => $exeContext->operation, - 'variableValues' => $exeContext->variableValues, - ]); - - // Memoizing results for same query field - // (useful for lists when several values are resolved against the same field) - $exeContext->memoized['resolveField'][$uid] = $memoized = [ - 'fieldDef' => $fieldDef, - 'args' => $args, - 'info' => $info, - 'results' => new \SplObjectStorage - ]; + if (!$fieldDef) { + return self::$UNDEFINED; } - // When source value is object it is possible to memoize certain subset of results - $isObject = is_object($source); + $returnType = $fieldDef->getType(); - if ($isObject && isset($memoized['results'][$source])) { - $result = $memoized['results'][$source]; + // Build hash of arguments from the field.arguments AST, using the + // variables scope to fulfill any variable references. + $args = Values::getArgumentValues( + $fieldDef->args, + $fieldAST->arguments, + $exeContext->variableValues + ); + + // The resolve function's optional third argument is a collection of + // information about the current execution state. + $info = new ResolveInfo([ + 'fieldName' => $fieldName, + 'fieldASTs' => $fieldASTs, + 'returnType' => $returnType, + 'parentType' => $parentType, + 'schema' => $exeContext->schema, + 'fragments' => $exeContext->fragments, + 'rootValue' => $exeContext->rootValue, + 'operation' => $exeContext->operation, + 'variableValues' => $exeContext->variableValues, + ]); + + + if (isset($fieldDef->resolveFn)) { + $resolveFn = $fieldDef->resolveFn; + } else if (isset($parentType->resolveFieldFn)) { + $resolveFn = $parentType->resolveFieldFn; } else { - if (isset($fieldDef->resolveFn)) { - $resolveFn = $fieldDef->resolveFn; - } else if (isset($parentType->resolveFieldFn)) { - $resolveFn = $parentType->resolveFieldFn; - } else { - $resolveFn = self::$defaultResolveFn; - } - - // Get the resolve function, regardless of if its result is normal - // or abrupt (error). - $result = self::resolveOrError($resolveFn, $source, $args, $info); - - $result = self::completeValueCatchingError( - $exeContext, - $returnType, - $fieldASTs, - $info, - $result - ); - - if ($isObject) { - $memoized['results'][$source] = $result; - } + $resolveFn = self::$defaultResolveFn; } + // Get the resolve function, regardless of if its result is normal + // or abrupt (error). + $result = self::resolveOrError($resolveFn, $source, $args, $info); + + $result = self::completeValueCatchingError( + $exeContext, + $returnType, + $fieldASTs, + $info, + $result + ); + return $result; } diff --git a/tests/Executor/ExecutorTest.php b/tests/Executor/ExecutorTest.php index 8fdfda1..9f8c39f 100644 --- a/tests/Executor/ExecutorTest.php +++ b/tests/Executor/ExecutorTest.php @@ -547,99 +547,4 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase $this->assertEquals($expected, $result->toArray()); } - - public function testResolvedValueIsMemoized() - { - $doc = ' - query Q { - a { - b { - c - d - } - } - } - '; - - $memoizedValue = new \ArrayObject([ - 'b' => 'id1' - ]); - - $A = null; - - $Test = new ObjectType([ - 'name' => 'Test', - 'fields' => [ - 'a' => [ - 'type' => function() use (&$A) {return Type::listOf($A);}, - 'resolve' => function() use ($memoizedValue) { - return [ - $memoizedValue, - new \ArrayObject([ - 'b' => 'id2', - ]), - $memoizedValue, - new \ArrayObject([ - 'b' => 'id2', - ]) - ]; - } - ] - ] - ]); - - $callCounts = ['id1' => 0, 'id2' => 0]; - - $A = new ObjectType([ - 'name' => 'A', - 'fields' => [ - 'b' => [ - 'type' => new ObjectType([ - 'name' => 'B', - 'fields' => [ - 'c' => ['type' => Type::string()], - 'd' => ['type' => Type::string()] - ] - ]), - 'resolve' => function($value) use (&$callCounts) { - $callCounts[$value['b']]++; - - switch ($value['b']) { - case 'id1': - return [ - 'c' => 'c1', - 'd' => 'd1' - ]; - case 'id2': - return [ - 'c' => 'c2', - 'd' => 'd2' - ]; - } - } - ] - ] - ]); - - // Test that value resolved once is memoized for same query field - $schema = new Schema($Test); - - $query = Parser::parse($doc); - $result = Executor::execute($schema, $query); - $expected = [ - 'data' => [ - 'a' => [ - ['b' => ['c' => 'c1', 'd' => 'd1']], - ['b' => ['c' => 'c2', 'd' => 'd2']], - ['b' => ['c' => 'c1', 'd' => 'd1']], - ['b' => ['c' => 'c2', 'd' => 'd2']], - ] - ] - ]; - - $this->assertEquals($expected, $result->toArray()); - - $this->assertSame($callCounts['id1'], 1); // Result for id1 is expected to be memoized after first call - $this->assertSame($callCounts['id2'], 2); - } }