From 8a60c6d20680fec194290b3d87c4127c1c23609e Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 5 Dec 2025 15:11:26 +0100 Subject: [PATCH 01/50] Optimize deferred Resolves https://github.com/webonyx/graphql-php/issues/1803 Follow up to https://github.com/webonyx/graphql-php/pull/1790 --- tests/Executor/DeferredFieldsTest.php | 304 ++++++++++++++++++++++++++ 1 file changed, 304 insertions(+) diff --git a/tests/Executor/DeferredFieldsTest.php b/tests/Executor/DeferredFieldsTest.php index a66a44952..53d50bfa7 100644 --- a/tests/Executor/DeferredFieldsTest.php +++ b/tests/Executor/DeferredFieldsTest.php @@ -668,4 +668,308 @@ private function assertPathsMatch(array $expectedPaths): void self::assertContains($expectedPath, $this->paths, 'Missing path: ' . json_encode($expectedPath, JSON_THROW_ON_ERROR)); } } + + /** + * Test that DataLoader-style batching works correctly with many Deferred objects. + * + * DataLoaders work by: + * 1. Collecting all entity IDs during field resolution (buffering) + * 2. Making a single batch query when the queue is processed + * 3. Distributing results to waiting promises + * + * This test verifies that all IDs are collected before any batch is executed, + * which is essential for the DataLoader pattern to work correctly. + * + * The test creates 600 items to exceed any reasonable batch size threshold, + * ensuring that incremental processing (if implemented) doesn't break batching. + * + * @see https://github.com/webonyx/graphql-php/issues/1803 + * @see https://github.com/webonyx/graphql-php/issues/972 + */ + public function testDataLoaderPatternWithManyDeferredObjects(): void + { + $itemCount = 600; + + // Simulate a data source (like a database) + $authorData = []; + for ($i = 1; $i <= 100; ++$i) { + $authorData[$i] = ['id' => $i, 'name' => "Author {$i}"]; + } + + $bookData = []; + for ($i = 1; $i <= $itemCount; ++$i) { + $bookData[$i] = [ + 'id' => $i, + 'title' => "Book {$i}", + 'authorId' => ($i % 100) + 1, + ]; + } + + // DataLoader simulation - buffers IDs and loads in batch + $authorBuffer = []; + $loadedAuthors = []; + $batchLoadCount = 0; + + $loadAuthor = function (int $authorId) use (&$authorBuffer, &$loadedAuthors, &$batchLoadCount, $authorData): Deferred { + $authorBuffer[] = $authorId; + + return new Deferred(static function () use ($authorId, &$authorBuffer, &$loadedAuthors, &$batchLoadCount, $authorData): ?array { + // On first execution, batch load all buffered IDs + if ($authorBuffer !== []) { + ++$batchLoadCount; + // Simulate batch database query: load all buffered authors at once + foreach ($authorBuffer as $id) { + $loadedAuthors[$id] = $authorData[$id] ?? null; + } + $authorBuffer = []; // Clear buffer after batch load + } + + return $loadedAuthors[$authorId] ?? null; + }); + }; + + $authorType = new ObjectType([ + 'name' => 'Author', + 'fields' => [ + 'id' => Type::int(), + 'name' => Type::string(), + ], + ]); + + $bookType = new ObjectType([ + 'name' => 'Book', + 'fields' => [ + 'id' => Type::int(), + 'title' => Type::string(), + 'author' => [ + 'type' => $authorType, + 'resolve' => static fn (array $book) => $loadAuthor($book['authorId']), + ], + ], + ]); + + $queryType = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'books' => [ + 'type' => Type::nonNull(Type::listOf(Type::nonNull($bookType))), + 'resolve' => static fn (): array => array_values($bookData), + ], + ], + ]); + + $schema = new Schema(['query' => $queryType]); + $query = Parser::parse('{ books { id title author { id name } } }'); + + $result = Executor::execute($schema, $query); + $resultArray = $result->toArray(); + + // Verify no errors + self::assertArrayNotHasKey('errors', $resultArray, 'Query should not produce errors'); + + // Verify all books are returned + self::assertCount($itemCount, $resultArray['data']['books']); + + // Verify no null authors (this was the bug in issue #1803) + $nullAuthorCount = 0; + foreach ($resultArray['data']['books'] as $index => $book) { + if ($book['author'] === null) { + ++$nullAuthorCount; + } + } + + self::assertSame( + 0, + $nullAuthorCount, + "Expected 0 null authors, but found {$nullAuthorCount}. " . + 'This indicates that batch loading was triggered before all IDs were collected.' + ); + + // Verify batch loading happened only once (DataLoader pattern) + // With proper batching, all 600 author loads should be batched into ONE query + self::assertSame( + 1, + $batchLoadCount, + "Expected exactly 1 batch load, but got {$batchLoadCount}. " . + 'Multiple batch loads indicate that the queue was processed before all Deferred objects were created.' + ); + } + + /** + * Test nested Deferred fields with DataLoader pattern. + * + * Reproduces the bug from issue #1803 where nested lists returned null + * when using DataLoader-style batching with many items. + * + * The scenario: eventDays -> event -> eventDays + * Each level uses Deferred with buffered batch loading. + * + * @see https://github.com/webonyx/graphql-php/issues/1803 + */ + public function testNestedDataLoaderPattern(): void + { + $eventCount = 50; + $daysPerEvent = 15; // Total: 50 * 15 = 750 items + + // Data sources + $events = []; + for ($i = 1; $i <= $eventCount; ++$i) { + $events[$i] = ['id' => $i, 'name' => "Event {$i}"]; + } + + $eventDays = []; + $dayId = 1; + for ($eventId = 1; $eventId <= $eventCount; ++$eventId) { + for ($day = 1; $day <= $daysPerEvent; ++$day) { + $eventDays[$dayId] = [ + 'id' => $dayId, + 'eventId' => $eventId, + 'date' => "2025-01-" . str_pad((string) $day, 2, '0', STR_PAD_LEFT), + ]; + ++$dayId; + } + } + + // DataLoader for events + $eventBuffer = []; + $loadedEvents = []; + $eventBatchCount = 0; + + $loadEvent = function (int $eventId) use (&$eventBuffer, &$loadedEvents, &$eventBatchCount, $events): Deferred { + $eventBuffer[] = $eventId; + + return new Deferred(static function () use ($eventId, &$eventBuffer, &$loadedEvents, &$eventBatchCount, $events): ?array { + if ($eventBuffer !== []) { + ++$eventBatchCount; + foreach ($eventBuffer as $id) { + $loadedEvents[$id] = $events[$id] ?? null; + } + $eventBuffer = []; + } + + return $loadedEvents[$eventId] ?? null; + }); + }; + + // DataLoader for event days by event ID + $eventDaysBuffer = []; + $loadedEventDays = []; + $eventDaysBatchCount = 0; + + $loadEventDays = function (int $eventId) use (&$eventDaysBuffer, &$loadedEventDays, &$eventDaysBatchCount, $eventDays): Deferred { + $eventDaysBuffer[] = $eventId; + + return new Deferred(static function () use ($eventId, &$eventDaysBuffer, &$loadedEventDays, &$eventDaysBatchCount, $eventDays): array { + if ($eventDaysBuffer !== []) { + ++$eventDaysBatchCount; + // Batch load: group event days by event ID + foreach ($eventDaysBuffer as $eId) { + $loadedEventDays[$eId] = array_values(array_filter( + $eventDays, + static fn (array $day): bool => $day['eventId'] === $eId + )); + } + $eventDaysBuffer = []; + } + + return $loadedEventDays[$eventId] ?? []; + }); + }; + + // Use by-reference to handle circular type dependencies + $eventType = null; + + $eventDayType = new ObjectType([ + 'name' => 'EventDay', + 'fields' => function () use (&$eventType, $loadEvent): array { + return [ + 'id' => Type::int(), + 'date' => Type::string(), + 'event' => [ + 'type' => $eventType, + 'resolve' => static fn (array $day) => $loadEvent($day['eventId']), + ], + ]; + }, + ]); + + $eventType = new ObjectType([ + 'name' => 'Event', + 'fields' => function () use ($eventDayType, $loadEventDays): array { + return [ + 'id' => Type::int(), + 'name' => Type::string(), + 'eventDays' => [ + 'type' => Type::nonNull(Type::listOf(Type::nonNull($eventDayType))), + 'resolve' => static fn (array $event) => $loadEventDays($event['id']), + ], + ]; + }, + ]); + + $queryType = new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'eventDays' => [ + 'type' => Type::nonNull(Type::listOf(Type::nonNull($eventDayType))), + 'resolve' => static fn (): array => array_values($eventDays), + ], + ], + ]); + + $schema = new Schema(['query' => $queryType]); + + // Query that mirrors the bug report: eventDays -> event -> eventDays + $query = Parser::parse(' + { + eventDays { + id + date + event { + id + name + eventDays { + id + date + } + } + } + } + '); + + $result = Executor::execute($schema, $query); + $resultArray = $result->toArray(); + + // Verify no errors + self::assertArrayNotHasKey('errors', $resultArray, 'Query should not produce errors'); + + $totalDays = $eventCount * $daysPerEvent; + self::assertCount($totalDays, $resultArray['data']['eventDays']); + + // Check for null events (the bug from issue #1803) + $nullEventCount = 0; + $nullNestedDaysCount = 0; + + foreach ($resultArray['data']['eventDays'] as $day) { + if ($day['event'] === null) { + ++$nullEventCount; + } elseif ($day['event']['eventDays'] === null) { + ++$nullNestedDaysCount; + } + } + + self::assertSame( + 0, + $nullEventCount, + "Expected 0 null events, but found {$nullEventCount}. " . + 'This indicates premature batch processing broke DataLoader batching.' + ); + + self::assertSame( + 0, + $nullNestedDaysCount, + "Expected 0 null nested eventDays, but found {$nullNestedDaysCount}. " . + 'This indicates premature batch processing broke nested DataLoader batching.' + ); + } } From 19efb1004d70c06dc06f6e2807c48f578da2bfc8 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Fri, 5 Dec 2025 14:12:52 +0000 Subject: [PATCH 02/50] Autofix --- tests/Executor/DeferredFieldsTest.php | 36 +++++++++++++-------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/tests/Executor/DeferredFieldsTest.php b/tests/Executor/DeferredFieldsTest.php index 53d50bfa7..093bde7d8 100644 --- a/tests/Executor/DeferredFieldsTest.php +++ b/tests/Executor/DeferredFieldsTest.php @@ -781,8 +781,8 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void self::assertSame( 0, $nullAuthorCount, - "Expected 0 null authors, but found {$nullAuthorCount}. " . - 'This indicates that batch loading was triggered before all IDs were collected.' + "Expected 0 null authors, but found {$nullAuthorCount}. " + . 'This indicates that batch loading was triggered before all IDs were collected.' ); // Verify batch loading happened only once (DataLoader pattern) @@ -790,8 +790,8 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void self::assertSame( 1, $batchLoadCount, - "Expected exactly 1 batch load, but got {$batchLoadCount}. " . - 'Multiple batch loads indicate that the queue was processed before all Deferred objects were created.' + "Expected exactly 1 batch load, but got {$batchLoadCount}. " + . 'Multiple batch loads indicate that the queue was processed before all Deferred objects were created.' ); } @@ -824,7 +824,7 @@ public function testNestedDataLoaderPattern(): void $eventDays[$dayId] = [ 'id' => $dayId, 'eventId' => $eventId, - 'date' => "2025-01-" . str_pad((string) $day, 2, '0', STR_PAD_LEFT), + 'date' => '2025-01-' . str_pad((string) $day, 2, '0', STR_PAD_LEFT), ]; ++$dayId; } @@ -895,16 +895,14 @@ public function testNestedDataLoaderPattern(): void $eventType = new ObjectType([ 'name' => 'Event', - 'fields' => function () use ($eventDayType, $loadEventDays): array { - return [ - 'id' => Type::int(), - 'name' => Type::string(), - 'eventDays' => [ - 'type' => Type::nonNull(Type::listOf(Type::nonNull($eventDayType))), - 'resolve' => static fn (array $event) => $loadEventDays($event['id']), - ], - ]; - }, + 'fields' => fn (): array => [ + 'id' => Type::int(), + 'name' => Type::string(), + 'eventDays' => [ + 'type' => Type::nonNull(Type::listOf(Type::nonNull($eventDayType))), + 'resolve' => static fn (array $event) => $loadEventDays($event['id']), + ], + ], ]); $queryType = new ObjectType([ @@ -961,15 +959,15 @@ public function testNestedDataLoaderPattern(): void self::assertSame( 0, $nullEventCount, - "Expected 0 null events, but found {$nullEventCount}. " . - 'This indicates premature batch processing broke DataLoader batching.' + "Expected 0 null events, but found {$nullEventCount}. " + . 'This indicates premature batch processing broke DataLoader batching.' ); self::assertSame( 0, $nullNestedDaysCount, - "Expected 0 null nested eventDays, but found {$nullNestedDaysCount}. " . - 'This indicates premature batch processing broke nested DataLoader batching.' + "Expected 0 null nested eventDays, but found {$nullNestedDaysCount}. " + . 'This indicates premature batch processing broke nested DataLoader batching.' ); } } From 91e050017c169bed0146a346d4634219a199edf8 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 9 Dec 2025 14:26:01 +0100 Subject: [PATCH 03/50] extend test --- tests/Executor/DeferredFieldsTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/Executor/DeferredFieldsTest.php b/tests/Executor/DeferredFieldsTest.php index 093bde7d8..fd20c851e 100644 --- a/tests/Executor/DeferredFieldsTest.php +++ b/tests/Executor/DeferredFieldsTest.php @@ -766,6 +766,7 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void // Verify no errors self::assertArrayNotHasKey('errors', $resultArray, 'Query should not produce errors'); + self::assertArrayHasKey('data', $resultArray); // Verify all books are returned self::assertCount($itemCount, $resultArray['data']['books']); @@ -940,6 +941,7 @@ public function testNestedDataLoaderPattern(): void // Verify no errors self::assertArrayNotHasKey('errors', $resultArray, 'Query should not produce errors'); + self::assertArrayHasKey('data', $resultArray); $totalDays = $eventCount * $daysPerEvent; self::assertCount($totalDays, $resultArray['data']['eventDays']); From dc45cf75a77346f049a05d8dad46da49b1fc7815 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 9 Dec 2025 14:26:12 +0100 Subject: [PATCH 04/50] Optimize Deferred memory usage with compact queue storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store promise references and arrays in the queue instead of closures to reduce memory footprint. This approach: - Stores the executor callable on the SyncPromise instance rather than capturing it in a closure - Queues the promise reference itself (smaller than a closure) - Uses arrays instead of closures in enqueueWaitingPromises() - Preserves DataLoader batching pattern (no premature queue processing) Key changes: - Add $executor property to SyncPromise for storing the executor - Update __construct() to store executor and queue $this - Update runQueue() to handle promise refs, arrays, and callables - Add processWaitingTask() for processing waiting promise arrays - Update enqueueWaitingPromises() to queue arrays instead of closures This maintains full compatibility with the DataLoader pattern since all Deferred executors are still queued BEFORE any are executed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/Executor/Promise/Adapter/SyncPromise.php | 96 ++++++++++++++------ 1 file changed, 66 insertions(+), 30 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 9bc1ac079..301eb81c2 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -25,6 +25,14 @@ class SyncPromise public string $state = self::PENDING; + /** + * Stored executor for deferred execution. + * Storing on the promise instead of in a closure reduces memory usage. + * + * @var (callable(): mixed)|null + */ + private $executor; + /** @var mixed */ public $result; @@ -47,7 +55,27 @@ public static function runQueue(): void $q = self::getQueue(); while (! $q->isEmpty()) { $task = $q->dequeue(); - $task(); + + if ($task instanceof self) { + // Execute promise's stored executor + $executor = $task->executor; + $task->executor = null; // Clear reference before execution for GC + assert(is_callable($executor)); + try { + $task->resolve($executor()); + } catch (\Throwable $e) { + $task->reject($e); // @phpstan-ignore missingType.checkedException (invariant violation - won't happen in normal operation) + } + } elseif (is_array($task)) { + // Handle waiting promise callbacks (from enqueueWaitingPromises) + /** @var array{self, (callable(mixed): mixed)|null, (callable(\Throwable): mixed)|null, string, mixed} $task */ + self::processWaitingTask($task); + } else { + // Backward compatibility: execute as callable + $task(); + } + + unset($task); } } @@ -58,13 +86,10 @@ public function __construct(?callable $executor = null) return; } - self::getQueue()->enqueue(function () use ($executor): void { - try { - $this->resolve($executor()); - } catch (\Throwable $e) { - $this->reject($e); - } - }); + // Store executor on the promise instead of in a closure to reduce memory usage + $this->executor = $executor; + // Queue the promise reference (smaller than a closure) + self::getQueue()->enqueue($this); } /** @@ -143,34 +168,45 @@ private function enqueueWaitingPromises(): void throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); } + // Capture state and result as values (not $this reference) to reduce memory footprint + $state = $this->state; + $result = $this->result; + $queue = self::getQueue(); + foreach ($this->waiting as $descriptor) { - self::getQueue()->enqueue(function () use ($descriptor): void { - [$promise, $onFulfilled, $onRejected] = $descriptor; - - if ($this->state === self::FULFILLED) { - try { - $promise->resolve($onFulfilled === null ? $this->result : $onFulfilled($this->result)); - } catch (\Throwable $e) { - $promise->reject($e); - } - } elseif ($this->state === self::REJECTED) { - try { - if ($onRejected === null) { - $promise->reject($this->result); - } else { - $promise->resolve($onRejected($this->result)); - } - } catch (\Throwable $e) { - $promise->reject($e); - } - } - }); + [$promise, $onFulfilled, $onRejected] = $descriptor; + // Queue an array instead of a closure - smaller memory footprint + $queue->enqueue([$promise, $onFulfilled, $onRejected, $state, $result]); } $this->waiting = []; } - /** @return \SplQueue */ + /** + * Process a waiting promise task from the queue. + * + * @param array{self, (callable(mixed): mixed)|null, (callable(\Throwable): mixed)|null, string, mixed} $task + */ + private static function processWaitingTask(array $task): void + { + [$promise, $onFulfilled, $onRejected, $state, $result] = $task; + + try { + if ($state === self::FULFILLED) { + $promise->resolve($onFulfilled === null ? $result : $onFulfilled($result)); + } elseif ($state === self::REJECTED) { + if ($onRejected === null) { + $promise->reject($result); + } else { + $promise->resolve($onRejected($result)); + } + } + } catch (\Throwable $e) { + $promise->reject($e); // @phpstan-ignore missingType.checkedException (invariant violation - won't happen in normal operation) + } + } + + /** @return \SplQueue */ public static function getQueue(): \SplQueue { static $queue; From 87866bdad167e87fe7d41e02796cbb01fe21d16c Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 9 Dec 2025 14:53:53 +0100 Subject: [PATCH 05/50] Apply latest rector changes --- tests/Executor/DeferredFieldsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Executor/DeferredFieldsTest.php b/tests/Executor/DeferredFieldsTest.php index fd20c851e..8311977a0 100644 --- a/tests/Executor/DeferredFieldsTest.php +++ b/tests/Executor/DeferredFieldsTest.php @@ -773,7 +773,7 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void // Verify no null authors (this was the bug in issue #1803) $nullAuthorCount = 0; - foreach ($resultArray['data']['books'] as $index => $book) { + foreach ($resultArray['data']['books'] as $book) { if ($book['author'] === null) { ++$nullAuthorCount; } From 42256fc8d0fe6ae12394dda926746c85bd967d58 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 9 Dec 2025 15:07:11 +0100 Subject: [PATCH 06/50] clean up comments --- src/Executor/Promise/Adapter/SyncPromise.php | 15 ++++------- tests/Executor/DeferredFieldsTest.php | 27 +++----------------- 2 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 301eb81c2..3961e1282 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -57,21 +57,19 @@ public static function runQueue(): void $task = $q->dequeue(); if ($task instanceof self) { - // Execute promise's stored executor $executor = $task->executor; - $task->executor = null; // Clear reference before execution for GC + // Clear reference before execution to allow GC to free memory during long-running queues + $task->executor = null; assert(is_callable($executor)); try { $task->resolve($executor()); } catch (\Throwable $e) { - $task->reject($e); // @phpstan-ignore missingType.checkedException (invariant violation - won't happen in normal operation) + $task->reject($e); // @phpstan-ignore missingType.checkedException } } elseif (is_array($task)) { - // Handle waiting promise callbacks (from enqueueWaitingPromises) /** @var array{self, (callable(mixed): mixed)|null, (callable(\Throwable): mixed)|null, string, mixed} $task */ self::processWaitingTask($task); } else { - // Backward compatibility: execute as callable $task(); } @@ -86,9 +84,7 @@ public function __construct(?callable $executor = null) return; } - // Store executor on the promise instead of in a closure to reduce memory usage $this->executor = $executor; - // Queue the promise reference (smaller than a closure) self::getQueue()->enqueue($this); } @@ -168,14 +164,13 @@ private function enqueueWaitingPromises(): void throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); } - // Capture state and result as values (not $this reference) to reduce memory footprint + // Capture state and result as values rather than $this reference to reduce memory per queued item $state = $this->state; $result = $this->result; $queue = self::getQueue(); foreach ($this->waiting as $descriptor) { [$promise, $onFulfilled, $onRejected] = $descriptor; - // Queue an array instead of a closure - smaller memory footprint $queue->enqueue([$promise, $onFulfilled, $onRejected, $state, $result]); } @@ -202,7 +197,7 @@ private static function processWaitingTask(array $task): void } } } catch (\Throwable $e) { - $promise->reject($e); // @phpstan-ignore missingType.checkedException (invariant violation - won't happen in normal operation) + $promise->reject($e); // @phpstan-ignore missingType.checkedException } } diff --git a/tests/Executor/DeferredFieldsTest.php b/tests/Executor/DeferredFieldsTest.php index 8311977a0..788bd8c95 100644 --- a/tests/Executor/DeferredFieldsTest.php +++ b/tests/Executor/DeferredFieldsTest.php @@ -690,7 +690,6 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void { $itemCount = 600; - // Simulate a data source (like a database) $authorData = []; for ($i = 1; $i <= 100; ++$i) { $authorData[$i] = ['id' => $i, 'name' => "Author {$i}"]; @@ -705,7 +704,6 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void ]; } - // DataLoader simulation - buffers IDs and loads in batch $authorBuffer = []; $loadedAuthors = []; $batchLoadCount = 0; @@ -714,14 +712,12 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void $authorBuffer[] = $authorId; return new Deferred(static function () use ($authorId, &$authorBuffer, &$loadedAuthors, &$batchLoadCount, $authorData): ?array { - // On first execution, batch load all buffered IDs if ($authorBuffer !== []) { ++$batchLoadCount; - // Simulate batch database query: load all buffered authors at once foreach ($authorBuffer as $id) { $loadedAuthors[$id] = $authorData[$id] ?? null; } - $authorBuffer = []; // Clear buffer after batch load + $authorBuffer = []; } return $loadedAuthors[$authorId] ?? null; @@ -764,14 +760,10 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void $result = Executor::execute($schema, $query); $resultArray = $result->toArray(); - // Verify no errors self::assertArrayNotHasKey('errors', $resultArray, 'Query should not produce errors'); self::assertArrayHasKey('data', $resultArray); - - // Verify all books are returned self::assertCount($itemCount, $resultArray['data']['books']); - // Verify no null authors (this was the bug in issue #1803) $nullAuthorCount = 0; foreach ($resultArray['data']['books'] as $book) { if ($book['author'] === null) { @@ -786,8 +778,6 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void . 'This indicates that batch loading was triggered before all IDs were collected.' ); - // Verify batch loading happened only once (DataLoader pattern) - // With proper batching, all 600 author loads should be batched into ONE query self::assertSame( 1, $batchLoadCount, @@ -810,9 +800,8 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void public function testNestedDataLoaderPattern(): void { $eventCount = 50; - $daysPerEvent = 15; // Total: 50 * 15 = 750 items + $daysPerEvent = 15; - // Data sources $events = []; for ($i = 1; $i <= $eventCount; ++$i) { $events[$i] = ['id' => $i, 'name' => "Event {$i}"]; @@ -831,7 +820,6 @@ public function testNestedDataLoaderPattern(): void } } - // DataLoader for events $eventBuffer = []; $loadedEvents = []; $eventBatchCount = 0; @@ -852,7 +840,6 @@ public function testNestedDataLoaderPattern(): void }); }; - // DataLoader for event days by event ID $eventDaysBuffer = []; $loadedEventDays = []; $eventDaysBatchCount = 0; @@ -863,7 +850,6 @@ public function testNestedDataLoaderPattern(): void return new Deferred(static function () use ($eventId, &$eventDaysBuffer, &$loadedEventDays, &$eventDaysBatchCount, $eventDays): array { if ($eventDaysBuffer !== []) { ++$eventDaysBatchCount; - // Batch load: group event days by event ID foreach ($eventDaysBuffer as $eId) { $loadedEventDays[$eId] = array_values(array_filter( $eventDays, @@ -877,7 +863,6 @@ public function testNestedDataLoaderPattern(): void }); }; - // Use by-reference to handle circular type dependencies $eventType = null; $eventDayType = new ObjectType([ @@ -917,8 +902,6 @@ public function testNestedDataLoaderPattern(): void ]); $schema = new Schema(['query' => $queryType]); - - // Query that mirrors the bug report: eventDays -> event -> eventDays $query = Parser::parse(' { eventDays { @@ -939,14 +922,10 @@ public function testNestedDataLoaderPattern(): void $result = Executor::execute($schema, $query); $resultArray = $result->toArray(); - // Verify no errors self::assertArrayNotHasKey('errors', $resultArray, 'Query should not produce errors'); self::assertArrayHasKey('data', $resultArray); + self::assertCount($eventCount * $daysPerEvent, $resultArray['data']['eventDays']); - $totalDays = $eventCount * $daysPerEvent; - self::assertCount($totalDays, $resultArray['data']['eventDays']); - - // Check for null events (the bug from issue #1803) $nullEventCount = 0; $nullNestedDaysCount = 0; From 1954f81fe437d910c0283307d8f17d125b4a3a11 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 9 Dec 2025 16:38:39 +0100 Subject: [PATCH 07/50] link issue --- tests/Executor/DeferredFieldsTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Executor/DeferredFieldsTest.php b/tests/Executor/DeferredFieldsTest.php index 788bd8c95..c6b6867ef 100644 --- a/tests/Executor/DeferredFieldsTest.php +++ b/tests/Executor/DeferredFieldsTest.php @@ -789,7 +789,7 @@ public function testDataLoaderPatternWithManyDeferredObjects(): void /** * Test nested Deferred fields with DataLoader pattern. * - * Reproduces the bug from issue #1803 where nested lists returned null + * Reproduces the bug from https://github.com/webonyx/graphql-php/issues/1803 where nested lists returned null * when using DataLoader-style batching with many items. * * The scenario: eventDays -> event -> eventDays From 69a13ad4fdc2e1ee58fbe0ea32a045a59379531d Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 11 Dec 2025 17:37:08 +0100 Subject: [PATCH 08/50] Use SplFixedArray for waiting queue items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reduces peak memory usage by using SplFixedArray instead of regular arrays for promise waiting queue items. SplFixedArray has a lower memory footprint for fixed-size data. Addresses feedback from @zimzat in https://github.com/webonyx/graphql-php/pull/1805#issuecomment-3640030003 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/Executor/Promise/Adapter/SyncPromise.php | 27 ++++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 3961e1282..13541d89a 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -39,14 +39,7 @@ class SyncPromise /** * Promises created in `then` method of this promise and awaiting resolution of this promise. * - * @var array< - * int, - * array{ - * self, - * (callable(mixed): mixed)|null, - * (callable(\Throwable): mixed)|null - * } - * > + * @var array> */ protected array $waiting = []; @@ -66,8 +59,7 @@ public static function runQueue(): void } catch (\Throwable $e) { $task->reject($e); // @phpstan-ignore missingType.checkedException } - } elseif (is_array($task)) { - /** @var array{self, (callable(mixed): mixed)|null, (callable(\Throwable): mixed)|null, string, mixed} $task */ + } elseif ($task instanceof \SplFixedArray) { self::processWaitingTask($task); } else { $task(); @@ -170,8 +162,9 @@ private function enqueueWaitingPromises(): void $queue = self::getQueue(); foreach ($this->waiting as $descriptor) { - [$promise, $onFulfilled, $onRejected] = $descriptor; - $queue->enqueue([$promise, $onFulfilled, $onRejected, $state, $result]); + $descriptor[3] = $state; + $descriptor[4] = $result; + $queue->enqueue($descriptor); } $this->waiting = []; @@ -180,9 +173,9 @@ private function enqueueWaitingPromises(): void /** * Process a waiting promise task from the queue. * - * @param array{self, (callable(mixed): mixed)|null, (callable(\Throwable): mixed)|null, string, mixed} $task + * @param \SplFixedArray $task */ - private static function processWaitingTask(array $task): void + private static function processWaitingTask(\SplFixedArray $task): void { [$promise, $onFulfilled, $onRejected, $state, $result] = $task; @@ -197,11 +190,11 @@ private static function processWaitingTask(array $task): void } } } catch (\Throwable $e) { - $promise->reject($e); // @phpstan-ignore missingType.checkedException + $promise->reject($e); } } - /** @return \SplQueue */ + /** @return \SplQueue|callable(): void> */ public static function getQueue(): \SplQueue { static $queue; @@ -226,7 +219,7 @@ public function then(?callable $onFulfilled = null, ?callable $onRejected = null } $tmp = new self(); - $this->waiting[] = [$tmp, $onFulfilled, $onRejected]; + $this->waiting[] = \SplFixedArray::fromArray([$tmp, $onFulfilled, $onRejected, null, null]); if ($this->state !== self::PENDING) { $this->enqueueWaitingPromises(); From 60525b490e0c878e77dbf2510267517946e75478 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 08:37:22 +0100 Subject: [PATCH 09/50] rename $q to $queue --- src/Executor/Promise/Adapter/SyncPromise.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 13541d89a..dbbf0c9d8 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -45,9 +45,9 @@ class SyncPromise public static function runQueue(): void { - $q = self::getQueue(); - while (! $q->isEmpty()) { - $task = $q->dequeue(); + $queue = self::getQueue(); + while (! $queue->isEmpty()) { + $task = $queue->dequeue(); if ($task instanceof self) { $executor = $task->executor; @@ -77,7 +77,9 @@ public function __construct(?callable $executor = null) } $this->executor = $executor; - self::getQueue()->enqueue($this); + + $queue = self::getQueue(); + $queue->enqueue($this); } /** From 97c08cf9bbc6ccf4efc96dd2965b8c11254dbf25 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 08:50:45 +0100 Subject: [PATCH 10/50] remove dead code --- src/Executor/Promise/Adapter/SyncPromise.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index dbbf0c9d8..3df21a8d0 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -59,10 +59,8 @@ public static function runQueue(): void } catch (\Throwable $e) { $task->reject($e); // @phpstan-ignore missingType.checkedException } - } elseif ($task instanceof \SplFixedArray) { - self::processWaitingTask($task); } else { - $task(); + self::processWaitingTask($task); } unset($task); @@ -196,7 +194,7 @@ private static function processWaitingTask(\SplFixedArray $task): void } } - /** @return \SplQueue|callable(): void> */ + /** @return \SplQueue> */ public static function getQueue(): \SplQueue { static $queue; From a5cda3179a8d6cdffd4c1edae0610a0a4454fb41 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 10:45:59 +0100 Subject: [PATCH 11/50] Simplify queue to hold only SyncPromise instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store waiting task data (callbacks, state, result) directly on the child SyncPromise instead of wrapping in SplFixedArray. This simplifies the queue's generic type from `SplQueue>` to `SplQueue`. Memory benchmark shows ~6.5% reduction for Deferred + then() chains: - Single Deferred: 528 → 608 bytes (+80, due to new properties) - Single then(): 888 → 792 bytes (-96, no SplFixedArray wrapper) - 5000 Deferred + then(): 9.6MB → 9.0MB (-620KB net savings) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/Executor/Promise/Adapter/SyncPromise.php | 70 +++++++++++++++----- 1 file changed, 52 insertions(+), 18 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 3df21a8d0..a8756b6f9 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -39,28 +39,51 @@ class SyncPromise /** * Promises created in `then` method of this promise and awaiting resolution of this promise. * - * @var array> + * @var array */ protected array $waiting = []; + /** + * Callback to execute when parent promise fulfills (for waiting promises). + * + * @var (callable(mixed): mixed)|null + */ + private $waitingOnFulfilled; + + /** + * Callback to execute when parent promise rejects (for waiting promises). + * + * @var (callable(\Throwable): mixed)|null + */ + private $waitingOnRejected; + + /** State of the parent promise when this waiting promise was enqueued. */ + private ?string $waitingState = null; + + /** + * Result of the parent promise when this waiting promise was enqueued. + * + * @var mixed + */ + private $waitingResult; + public static function runQueue(): void { $queue = self::getQueue(); while (! $queue->isEmpty()) { $task = $queue->dequeue(); - if ($task instanceof self) { + if ($task->executor !== null) { $executor = $task->executor; // Clear reference before execution to allow GC to free memory during long-running queues $task->executor = null; - assert(is_callable($executor)); try { $task->resolve($executor()); } catch (\Throwable $e) { $task->reject($e); // @phpstan-ignore missingType.checkedException } - } else { - self::processWaitingTask($task); + } elseif ($task->waitingState !== null) { + self::processWaitingTask($task); // @phpstan-ignore missingType.checkedException } unset($task); @@ -161,10 +184,10 @@ private function enqueueWaitingPromises(): void $result = $this->result; $queue = self::getQueue(); - foreach ($this->waiting as $descriptor) { - $descriptor[3] = $state; - $descriptor[4] = $result; - $queue->enqueue($descriptor); + foreach ($this->waiting as $promise) { + $promise->waitingState = $state; + $promise->waitingResult = $result; + $queue->enqueue($promise); } $this->waiting = []; @@ -173,28 +196,37 @@ private function enqueueWaitingPromises(): void /** * Process a waiting promise task from the queue. * - * @param \SplFixedArray $task + * @throws \Exception */ - private static function processWaitingTask(\SplFixedArray $task): void + private static function processWaitingTask(self $task): void { - [$promise, $onFulfilled, $onRejected, $state, $result] = $task; + $onFulfilled = $task->waitingOnFulfilled; + $onRejected = $task->waitingOnRejected; + $state = $task->waitingState; + $result = $task->waitingResult; + + // Clear references to allow GC + $task->waitingOnFulfilled = null; + $task->waitingOnRejected = null; + $task->waitingState = null; + $task->waitingResult = null; try { if ($state === self::FULFILLED) { - $promise->resolve($onFulfilled === null ? $result : $onFulfilled($result)); + $task->resolve($onFulfilled === null ? $result : $onFulfilled($result)); } elseif ($state === self::REJECTED) { if ($onRejected === null) { - $promise->reject($result); + $task->reject($result); } else { - $promise->resolve($onRejected($result)); + $task->resolve($onRejected($result)); } } } catch (\Throwable $e) { - $promise->reject($e); + $task->reject($e); } } - /** @return \SplQueue> */ + /** @return \SplQueue */ public static function getQueue(): \SplQueue { static $queue; @@ -219,7 +251,9 @@ public function then(?callable $onFulfilled = null, ?callable $onRejected = null } $tmp = new self(); - $this->waiting[] = \SplFixedArray::fromArray([$tmp, $onFulfilled, $onRejected, null, null]); + $tmp->waitingOnFulfilled = $onFulfilled; + $tmp->waitingOnRejected = $onRejected; + $this->waiting[] = $tmp; if ($this->state !== self::PENDING) { $this->enqueueWaitingPromises(); From f80ea172f4fe68775f98d8c40d8853c03627ffed Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 15:06:24 +0100 Subject: [PATCH 12/50] readable ternary --- src/Executor/Promise/Adapter/SyncPromise.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index a8756b6f9..a9176d952 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -200,12 +200,12 @@ private function enqueueWaitingPromises(): void */ private static function processWaitingTask(self $task): void { + // Unpack and clear references to allow garbage collection $onFulfilled = $task->waitingOnFulfilled; $onRejected = $task->waitingOnRejected; $state = $task->waitingState; $result = $task->waitingResult; - // Clear references to allow GC $task->waitingOnFulfilled = null; $task->waitingOnRejected = null; $task->waitingState = null; @@ -213,7 +213,9 @@ private static function processWaitingTask(self $task): void try { if ($state === self::FULFILLED) { - $task->resolve($onFulfilled === null ? $result : $onFulfilled($result)); + $task->resolve($onFulfilled !== null + ? $onFulfilled($result) + : $result); } elseif ($state === self::REJECTED) { if ($onRejected === null) { $task->reject($result); From 1a93c0017208f66fe7c4b0f5df762d1a8cd0def0 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 15:07:34 +0100 Subject: [PATCH 13/50] extract + better comment --- src/Executor/Promise/Adapter/SyncPromise.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index a9176d952..d1faf3328 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -73,9 +73,9 @@ public static function runQueue(): void while (! $queue->isEmpty()) { $task = $queue->dequeue(); - if ($task->executor !== null) { - $executor = $task->executor; - // Clear reference before execution to allow GC to free memory during long-running queues + $executor = $task->executor; + if ($executor !== null) { + // Allow garbage collection to free memory during long-running queues $task->executor = null; try { $task->resolve($executor()); From 7a9a686fb2e1605d09bdf188df9df5668e2a1344 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 16:42:48 +0100 Subject: [PATCH 14/50] extract queue --- src/Executor/Promise/Adapter/SyncPromise.php | 86 ++++++++----------- .../Promise/Adapter/SyncPromiseAdapter.php | 6 +- .../Promise/Adapter/SyncPromiseQueue.php | 47 ++++++++++ .../Promise/SyncPromiseAdapterTest.php | 3 +- tests/Executor/Promise/SyncPromiseTest.php | 17 ++-- 5 files changed, 99 insertions(+), 60 deletions(-) create mode 100644 src/Executor/Promise/Adapter/SyncPromiseQueue.php diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index d1faf3328..21c3d5549 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -67,29 +67,6 @@ class SyncPromise */ private $waitingResult; - public static function runQueue(): void - { - $queue = self::getQueue(); - while (! $queue->isEmpty()) { - $task = $queue->dequeue(); - - $executor = $task->executor; - if ($executor !== null) { - // Allow garbage collection to free memory during long-running queues - $task->executor = null; - try { - $task->resolve($executor()); - } catch (\Throwable $e) { - $task->reject($e); // @phpstan-ignore missingType.checkedException - } - } elseif ($task->waitingState !== null) { - self::processWaitingTask($task); // @phpstan-ignore missingType.checkedException - } - - unset($task); - } - } - /** @param Executor|null $executor */ public function __construct(?callable $executor = null) { @@ -99,10 +76,32 @@ public function __construct(?callable $executor = null) $this->executor = $executor; - $queue = self::getQueue(); + $queue = SyncPromiseQueue::getInstance(); $queue->enqueue($this); } + /** + * Execute the queued task for this promise. + * + * Handles both root promises (with executor) and child promises (waiting on parent). + */ + public function runQueuedTask(): void + { + $executor = $this->executor; + + if ($executor !== null) { + // Allow garbage collection to free memory during long-running queues + $this->executor = null; + try { + $this->resolve($executor()); + } catch (\Throwable $e) { + $this->reject($e); // @phpstan-ignore missingType.checkedException + } + } elseif ($this->waitingState !== null) { + $this->processWaitingTask(); // @phpstan-ignore missingType.checkedException + } + } + /** * @param mixed $value * @@ -179,10 +178,9 @@ private function enqueueWaitingPromises(): void throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); } - // Capture state and result as values rather than $this reference to reduce memory per queued item $state = $this->state; $result = $this->result; - $queue = self::getQueue(); + $queue = SyncPromiseQueue::getInstance(); foreach ($this->waiting as $promise) { $promise->waitingState = $state; @@ -194,48 +192,40 @@ private function enqueueWaitingPromises(): void } /** - * Process a waiting promise task from the queue. + * Process this promise as a waiting task (child promise awaiting parent resolution). * * @throws \Exception */ - private static function processWaitingTask(self $task): void + private function processWaitingTask(): void { // Unpack and clear references to allow garbage collection - $onFulfilled = $task->waitingOnFulfilled; - $onRejected = $task->waitingOnRejected; - $state = $task->waitingState; - $result = $task->waitingResult; + $onFulfilled = $this->waitingOnFulfilled; + $onRejected = $this->waitingOnRejected; + $state = $this->waitingState; + $result = $this->waitingResult; - $task->waitingOnFulfilled = null; - $task->waitingOnRejected = null; - $task->waitingState = null; - $task->waitingResult = null; + $this->waitingOnFulfilled = null; + $this->waitingOnRejected = null; + $this->waitingState = null; + $this->waitingResult = null; try { if ($state === self::FULFILLED) { - $task->resolve($onFulfilled !== null + $this->resolve($onFulfilled !== null ? $onFulfilled($result) : $result); } elseif ($state === self::REJECTED) { if ($onRejected === null) { - $task->reject($result); + $this->reject($result); } else { - $task->resolve($onRejected($result)); + $this->resolve($onRejected($result)); } } } catch (\Throwable $e) { - $task->reject($e); + $this->reject($e); } } - /** @return \SplQueue */ - public static function getQueue(): \SplQueue - { - static $queue; - - return $queue ??= new \SplQueue(); - } - /** * @param (callable(mixed): mixed)|null $onFulfilled * @param (callable(\Throwable): mixed)|null $onRejected diff --git a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php index 19ea4c3de..f4252029c 100644 --- a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php +++ b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php @@ -135,16 +135,16 @@ static function ($value) use (&$result, $index, &$count, &$resolveAllWhenFinishe public function wait(Promise $promise) { $this->beforeWait($promise); - $taskQueue = SyncPromise::getQueue(); + $queue = SyncPromiseQueue::getInstance(); $syncPromise = $promise->adoptedPromise; assert($syncPromise instanceof SyncPromise); while ( $syncPromise->state === SyncPromise::PENDING - && ! $taskQueue->isEmpty() + && ! $queue->isEmpty() ) { - SyncPromise::runQueue(); + $queue->run(); $this->onWait($promise); } diff --git a/src/Executor/Promise/Adapter/SyncPromiseQueue.php b/src/Executor/Promise/Adapter/SyncPromiseQueue.php new file mode 100644 index 000000000..d0dcf4394 --- /dev/null +++ b/src/Executor/Promise/Adapter/SyncPromiseQueue.php @@ -0,0 +1,47 @@ + */ + protected \SplQueue $queue; + + public function __construct() + { + $this->queue = new \SplQueue(); + } + + public static function getInstance(): self + { + return self::$instance ??= new self(); + } + + public function enqueue(SyncPromise $promise): void + { + $this->queue->enqueue($promise); + } + + public function isEmpty(): bool + { + return $this->queue->isEmpty(); + } + + public function count(): int + { + return $this->queue->count(); + } + + /** Process all queued promises until the queue is empty. */ + public function run(): void + { + while (! $this->queue->isEmpty()) { + $task = $this->queue->dequeue(); + $task->runQueuedTask(); + unset($task); + } + } +} diff --git a/tests/Executor/Promise/SyncPromiseAdapterTest.php b/tests/Executor/Promise/SyncPromiseAdapterTest.php index 51c825a98..cdb486dea 100644 --- a/tests/Executor/Promise/SyncPromiseAdapterTest.php +++ b/tests/Executor/Promise/SyncPromiseAdapterTest.php @@ -6,6 +6,7 @@ use GraphQL\Error\InvariantViolation; use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter; +use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; use GraphQL\Executor\Promise\Promise; use PHPUnit\Framework\TestCase; @@ -90,7 +91,7 @@ static function (\Throwable $reason) use (&$actualNextReason, &$onRejectedCalled self::assertFalse($onFulfilledCalled); self::assertFalse($onRejectedCalled); - SyncPromise::runQueue(); + SyncPromiseQueue::getInstance()->run(); if ($expectedNextState !== SyncPromise::PENDING) { if ($expectedNextReason === null) { diff --git a/tests/Executor/Promise/SyncPromiseTest.php b/tests/Executor/Promise/SyncPromiseTest.php index 9cf39109f..a4a1d8760 100644 --- a/tests/Executor/Promise/SyncPromiseTest.php +++ b/tests/Executor/Promise/SyncPromiseTest.php @@ -4,6 +4,7 @@ use GraphQL\Error\InvariantViolation; use GraphQL\Executor\Promise\Adapter\SyncPromise; +use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; use GraphQL\Tests\TestCaseBase; final class SyncPromiseTest extends TestCaseBase @@ -125,7 +126,7 @@ static function () use (&$onRejectedCalled): void { self::assertNotSame($nextPromise, $nextPromise2); } - SyncPromise::runQueue(); + SyncPromiseQueue::getInstance()->run(); self::assertValidPromise($nextPromise2, $expectedNextValue, $expectedNextReason, $expectedNextState); self::assertValidPromise($nextPromise3, $expectedNextValue, $expectedNextReason, $expectedNextState); @@ -161,7 +162,7 @@ static function (\Throwable $reason) use (&$actualNextReason, &$onRejectedCalled self::assertFalse($onFulfilledCalled); self::assertFalse($onRejectedCalled); - SyncPromise::runQueue(); + SyncPromiseQueue::getInstance()->run(); if ($expectedNextReason === null) { self::assertTrue($onFulfilledCalled); // @phpstan-ignore-line value is mutable @@ -296,7 +297,7 @@ static function () use (&$onFulfilledCalled): void { self::assertNotSame($nextPromise, $nextPromise2); } - SyncPromise::runQueue(); + SyncPromiseQueue::getInstance()->run(); self::assertValidPromise($nextPromise2, $expectedNextValue, $expectedNextReason, $expectedNextState); self::assertValidPromise($nextPromise3, $expectedNextValue, $expectedNextReason, $expectedNextState); @@ -390,12 +391,12 @@ public function testPendingPromiseThen(): void $nextPromise3 = $promise->then($onFulfilled, $onRejected); $nextPromise4 = $promise->then($onFulfilled, $onRejected); - self::assertCount(0, SyncPromise::getQueue()); + self::assertSame(0, SyncPromiseQueue::getInstance()->count()); self::assertSame(0, $onFulfilledCount); self::assertSame(0, $onRejectedCount); $promise->resolve(1); - self::assertCount(4, SyncPromise::getQueue()); + self::assertSame(4, SyncPromiseQueue::getInstance()->count()); self::assertSame(0, $onFulfilledCount); // @phpstan-ignore-line side-effects self::assertSame(0, $onRejectedCount); // @phpstan-ignore-line side-effects self::assertSame(SyncPromise::PENDING, $nextPromise->state); @@ -403,8 +404,8 @@ public function testPendingPromiseThen(): void self::assertSame(SyncPromise::PENDING, $nextPromise3->state); self::assertSame(SyncPromise::PENDING, $nextPromise4->state); - SyncPromise::runQueue(); - self::assertCount(0, SyncPromise::getQueue()); + SyncPromiseQueue::getInstance()->run(); + self::assertSame(0, SyncPromiseQueue::getInstance()->count()); self::assertSame(3, $onFulfilledCount); // @phpstan-ignore-line side-effects self::assertSame(0, $onRejectedCount); // @phpstan-ignore-line side-effects self::assertValidPromise($nextPromise, 1, null, SyncPromise::FULFILLED); @@ -415,7 +416,7 @@ public function testPendingPromiseThen(): void public function testRunEmptyQueue(): void { - SyncPromise::runQueue(); + SyncPromiseQueue::getInstance()->run(); $this->assertDidNotCrash(); } } From df37bfbff191fdad38efe555bd224c40f1285ae6 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 17:09:06 +0100 Subject: [PATCH 15/50] clean up properties --- src/Executor/Promise/Adapter/SyncPromise.php | 52 ++++++-------------- 1 file changed, 16 insertions(+), 36 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 21c3d5549..3a6b985bf 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -23,49 +23,29 @@ class SyncPromise public const FULFILLED = 'fulfilled'; public const REJECTED = 'rejected'; + /** Current promise state: PENDING, FULFILLED, or REJECTED. */ public string $state = self::PENDING; - /** - * Stored executor for deferred execution. - * Storing on the promise instead of in a closure reduces memory usage. - * - * @var (callable(): mixed)|null - */ - private $executor; - - /** @var mixed */ + /** @var mixed Resolved value or rejection reason. */ public $result; - /** - * Promises created in `then` method of this promise and awaiting resolution of this promise. - * - * @var array - */ + /** @var array Promises waiting for this promise to resolve. */ protected array $waiting = []; - /** - * Callback to execute when parent promise fulfills (for waiting promises). - * - * @var (callable(mixed): mixed)|null - */ - private $waitingOnFulfilled; + /** @var (callable(): mixed)|null Executor for deferred execution (root promises only). */ + protected $executor; - /** - * Callback to execute when parent promise rejects (for waiting promises). - * - * @var (callable(\Throwable): mixed)|null - */ - private $waitingOnRejected; + /** @var (callable(mixed): mixed)|null Callback when parent fulfills (child promises only). */ + protected $waitingOnFulfilled; - /** State of the parent promise when this waiting promise was enqueued. */ - private ?string $waitingState = null; + /** @var (callable(\Throwable): mixed)|null Callback when parent rejects (child promises only). */ + protected $waitingOnRejected; - /** - * Result of the parent promise when this waiting promise was enqueued. - * - * @var mixed - */ - private $waitingResult; + /** Parent promise state when this child was enqueued. */ + protected ?string $waitingState = null; + + /** @var mixed Parent promise result when this child was enqueued. */ + protected $waitingResult; /** @param Executor|null $executor */ public function __construct(?callable $executor = null) @@ -172,7 +152,7 @@ public function reject(\Throwable $reason): self } /** @throws InvariantViolation */ - private function enqueueWaitingPromises(): void + protected function enqueueWaitingPromises(): void { if ($this->state === self::PENDING) { throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); @@ -196,7 +176,7 @@ private function enqueueWaitingPromises(): void * * @throws \Exception */ - private function processWaitingTask(): void + protected function processWaitingTask(): void { // Unpack and clear references to allow garbage collection $onFulfilled = $this->waitingOnFulfilled; From eeb60e9f75f1578a67e697013ead1efdd5af36f8 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 17:12:50 +0100 Subject: [PATCH 16/50] order methods --- src/Executor/Promise/Adapter/SyncPromise.php | 121 +++++++++---------- 1 file changed, 58 insertions(+), 63 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 3a6b985bf..3498cbdad 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -56,30 +56,7 @@ public function __construct(?callable $executor = null) $this->executor = $executor; - $queue = SyncPromiseQueue::getInstance(); - $queue->enqueue($this); - } - - /** - * Execute the queued task for this promise. - * - * Handles both root promises (with executor) and child promises (waiting on parent). - */ - public function runQueuedTask(): void - { - $executor = $this->executor; - - if ($executor !== null) { - // Allow garbage collection to free memory during long-running queues - $this->executor = null; - try { - $this->resolve($executor()); - } catch (\Throwable $e) { - $this->reject($e); // @phpstan-ignore missingType.checkedException - } - } elseif ($this->waitingState !== null) { - $this->processWaitingTask(); // @phpstan-ignore missingType.checkedException - } + SyncPromiseQueue::getInstance()->enqueue($this); } /** @@ -151,6 +128,63 @@ public function reject(\Throwable $reason): self return $this; } + /** + * @param (callable(mixed): mixed)|null $onFulfilled + * @param (callable(\Throwable): mixed)|null $onRejected + * + * @throws InvariantViolation + */ + public function then(?callable $onFulfilled = null, ?callable $onRejected = null): self + { + if ($this->state === self::REJECTED && $onRejected === null) { + return $this; + } + + if ($this->state === self::FULFILLED && $onFulfilled === null) { + return $this; + } + + $tmp = new self(); + $tmp->waitingOnFulfilled = $onFulfilled; + $tmp->waitingOnRejected = $onRejected; + $this->waiting[] = $tmp; + + if ($this->state !== self::PENDING) { + $this->enqueueWaitingPromises(); + } + + return $tmp; + } + + /** + * @param callable(\Throwable): mixed $onRejected + * + * @throws InvariantViolation + * + * @deprecated TODO remove in next major version, use ->then(null, $onRejected) instead + */ + public function catch(callable $onRejected): self + { + return $this->then(null, $onRejected); + } + + /** Handles both root promises (with executor) and child promises (waiting on parent). */ + public function runQueuedTask(): void + { + $executor = $this->executor; + + if ($executor !== null) { + $this->executor = null; + try { + $this->resolve($executor()); + } catch (\Throwable $e) { + $this->reject($e); // @phpstan-ignore missingType.checkedException + } + } elseif ($this->waitingState !== null) { + $this->processWaitingTask(); // @phpstan-ignore missingType.checkedException + } + } + /** @throws InvariantViolation */ protected function enqueueWaitingPromises(): void { @@ -178,7 +212,6 @@ protected function enqueueWaitingPromises(): void */ protected function processWaitingTask(): void { - // Unpack and clear references to allow garbage collection $onFulfilled = $this->waitingOnFulfilled; $onRejected = $this->waitingOnRejected; $state = $this->waitingState; @@ -205,42 +238,4 @@ protected function processWaitingTask(): void $this->reject($e); } } - - /** - * @param (callable(mixed): mixed)|null $onFulfilled - * @param (callable(\Throwable): mixed)|null $onRejected - * - * @throws InvariantViolation - */ - public function then(?callable $onFulfilled = null, ?callable $onRejected = null): self - { - if ($this->state === self::REJECTED && $onRejected === null) { - return $this; - } - - if ($this->state === self::FULFILLED && $onFulfilled === null) { - return $this; - } - - $tmp = new self(); - $tmp->waitingOnFulfilled = $onFulfilled; - $tmp->waitingOnRejected = $onRejected; - $this->waiting[] = $tmp; - - if ($this->state !== self::PENDING) { - $this->enqueueWaitingPromises(); - } - - return $tmp; - } - - /** - * @param callable(\Throwable): mixed $onRejected - * - * @throws InvariantViolation - */ - public function catch(callable $onRejected): self - { - return $this->then(null, $onRejected); - } } From b73c8dc525c5ed2b10d441fb3a22e37af94b230d Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 17:17:10 +0100 Subject: [PATCH 17/50] format --- src/Executor/Promise/Adapter/SyncPromise.php | 36 ++++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 3498cbdad..a7e92d4e6 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -26,25 +26,49 @@ class SyncPromise /** Current promise state: PENDING, FULFILLED, or REJECTED. */ public string $state = self::PENDING; - /** @var mixed Resolved value or rejection reason. */ + /** + * Resolved value or rejection reason. + * + * @var mixed + */ public $result; - /** @var array Promises waiting for this promise to resolve. */ + /** + * Promises waiting for this promise to resolve. + * + * @var array + */ protected array $waiting = []; - /** @var (callable(): mixed)|null Executor for deferred execution (root promises only). */ + /** + * Executor for deferred execution (root promises only). + * + * @var (callable(): mixed)|null + */ protected $executor; - /** @var (callable(mixed): mixed)|null Callback when parent fulfills (child promises only). */ + /** + * Callback when parent fulfills (child promises only). + * + * @var (callable(mixed): mixed)|null + */ protected $waitingOnFulfilled; - /** @var (callable(\Throwable): mixed)|null Callback when parent rejects (child promises only). */ + /** + * Callback when parent rejects (child promises only). + * + * @var (callable(\Throwable): mixed)|null + */ protected $waitingOnRejected; /** Parent promise state when this child was enqueued. */ protected ?string $waitingState = null; - /** @var mixed Parent promise result when this child was enqueued. */ + /** + * Parent promise result when this child was enqueued. + * + * @var mixed + */ protected $waitingResult; /** @param Executor|null $executor */ From b6b095d54948cb3435a5c46d5562d98f6439ec10 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 17:30:00 +0100 Subject: [PATCH 18/50] split classes --- src/Deferred.php | 14 +- .../Promise/Adapter/ChildSyncPromise.php | 65 ++++++++ .../Promise/Adapter/RootSyncPromise.php | 42 +++++ src/Executor/Promise/Adapter/SyncPromise.php | 152 +++--------------- .../Promise/Adapter/SyncPromiseAdapter.php | 8 +- .../Promise/SyncPromiseAdapterTest.php | 5 +- tests/Executor/Promise/SyncPromiseTest.php | 28 ++-- 7 files changed, 157 insertions(+), 157 deletions(-) create mode 100644 src/Executor/Promise/Adapter/ChildSyncPromise.php create mode 100644 src/Executor/Promise/Adapter/RootSyncPromise.php diff --git a/src/Deferred.php b/src/Deferred.php index 69837cc07..fccc5f3cd 100644 --- a/src/Deferred.php +++ b/src/Deferred.php @@ -2,22 +2,18 @@ namespace GraphQL; -use GraphQL\Executor\Promise\Adapter\SyncPromise; +use GraphQL\Executor\Promise\Adapter\RootSyncPromise; /** - * @phpstan-import-type Executor from SyncPromise + * User-facing promise class for deferred field resolution. + * + * @phpstan-type Executor callable(): mixed */ -class Deferred extends SyncPromise +class Deferred extends RootSyncPromise { /** @param Executor $executor */ public static function create(callable $executor): self { return new self($executor); } - - /** @param Executor $executor */ - public function __construct(callable $executor) - { - parent::__construct($executor); - } } diff --git a/src/Executor/Promise/Adapter/ChildSyncPromise.php b/src/Executor/Promise/Adapter/ChildSyncPromise.php new file mode 100644 index 000000000..0054a8166 --- /dev/null +++ b/src/Executor/Promise/Adapter/ChildSyncPromise.php @@ -0,0 +1,65 @@ +onFulfilled; + $onRejected = $this->onRejected; + $state = $this->parentState; + $result = $this->parentResult; + + $this->onFulfilled = null; + $this->onRejected = null; + $this->parentState = null; + $this->parentResult = null; + + try { + if ($state === self::FULFILLED) { + $this->resolve($onFulfilled !== null + ? $onFulfilled($result) + : $result); + } elseif ($state === self::REJECTED) { + if ($onRejected === null) { + $this->reject($result); + } else { + $this->resolve($onRejected($result)); + } + } + } catch (\Throwable $e) { + $this->reject($e); + } + } +} diff --git a/src/Executor/Promise/Adapter/RootSyncPromise.php b/src/Executor/Promise/Adapter/RootSyncPromise.php new file mode 100644 index 000000000..ebcbded74 --- /dev/null +++ b/src/Executor/Promise/Adapter/RootSyncPromise.php @@ -0,0 +1,42 @@ +executor = $executor; + + $queue = SyncPromiseQueue::getInstance(); + $queue->enqueue($this); + } + + /** @throws \Exception */ + public function runQueuedTask(): void + { + $executor = $this->executor; + $this->executor = null; + + assert($executor !== null); + + try { + $this->resolve($executor()); + } catch (\Throwable $e) { + $this->reject($e); + } + } +} diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index a7e92d4e6..cfcae3b94 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -5,19 +5,14 @@ use GraphQL\Error\InvariantViolation; /** - * Simplistic (yet full-featured) implementation of Promises A+ spec for regular PHP `sync` mode - * (using queue to defer promises execution). + * Base class for synchronous promises implementing Promises A+ spec. * - * Library users are not supposed to use SyncPromise class in their resolvers. - * Instead, they should use @see \GraphQL\Deferred which enforces `$executor` callback in the constructor. + * Library users should use @see \GraphQL\Deferred to create promises. * - * Root SyncPromise without explicit `$executor` will never resolve (actually throw while trying). - * The whole point of Deferred is to ensure it never happens and that any resolver creates - * at least one $executor to start the promise chain. - * - * @phpstan-type Executor callable(): mixed + * @see RootSyncPromise for promises created with an executor + * @see ChildSyncPromise for promises created via then() chaining */ -class SyncPromise +abstract class SyncPromise { public const PENDING = 'pending'; public const FULFILLED = 'fulfilled'; @@ -34,54 +29,14 @@ class SyncPromise public $result; /** - * Promises waiting for this promise to resolve. + * Child promises waiting for this promise to resolve. * - * @var array + * @var array */ protected array $waiting = []; - /** - * Executor for deferred execution (root promises only). - * - * @var (callable(): mixed)|null - */ - protected $executor; - - /** - * Callback when parent fulfills (child promises only). - * - * @var (callable(mixed): mixed)|null - */ - protected $waitingOnFulfilled; - - /** - * Callback when parent rejects (child promises only). - * - * @var (callable(\Throwable): mixed)|null - */ - protected $waitingOnRejected; - - /** Parent promise state when this child was enqueued. */ - protected ?string $waitingState = null; - - /** - * Parent promise result when this child was enqueued. - * - * @var mixed - */ - protected $waitingResult; - - /** @param Executor|null $executor */ - public function __construct(?callable $executor = null) - { - if ($executor === null) { - return; - } - - $this->executor = $executor; - - SyncPromiseQueue::getInstance()->enqueue($this); - } + /** Execute the queued task for this promise. */ + abstract public function runQueuedTask(): void; /** * @param mixed $value @@ -160,53 +115,28 @@ public function reject(\Throwable $reason): self */ public function then(?callable $onFulfilled = null, ?callable $onRejected = null): self { - if ($this->state === self::REJECTED && $onRejected === null) { + if ($this->state === self::REJECTED + && $onRejected === null + ) { return $this; } - if ($this->state === self::FULFILLED && $onFulfilled === null) { + if ($this->state === self::FULFILLED + && $onFulfilled === null + ) { return $this; } - $tmp = new self(); - $tmp->waitingOnFulfilled = $onFulfilled; - $tmp->waitingOnRejected = $onRejected; - $this->waiting[] = $tmp; + $child = new ChildSyncPromise(); + $child->onFulfilled = $onFulfilled; + $child->onRejected = $onRejected; + $this->waiting[] = $child; if ($this->state !== self::PENDING) { $this->enqueueWaitingPromises(); } - return $tmp; - } - - /** - * @param callable(\Throwable): mixed $onRejected - * - * @throws InvariantViolation - * - * @deprecated TODO remove in next major version, use ->then(null, $onRejected) instead - */ - public function catch(callable $onRejected): self - { - return $this->then(null, $onRejected); - } - - /** Handles both root promises (with executor) and child promises (waiting on parent). */ - public function runQueuedTask(): void - { - $executor = $this->executor; - - if ($executor !== null) { - $this->executor = null; - try { - $this->resolve($executor()); - } catch (\Throwable $e) { - $this->reject($e); // @phpstan-ignore missingType.checkedException - } - } elseif ($this->waitingState !== null) { - $this->processWaitingTask(); // @phpstan-ignore missingType.checkedException - } + return $child; } /** @throws InvariantViolation */ @@ -220,46 +150,12 @@ protected function enqueueWaitingPromises(): void $result = $this->result; $queue = SyncPromiseQueue::getInstance(); - foreach ($this->waiting as $promise) { - $promise->waitingState = $state; - $promise->waitingResult = $result; - $queue->enqueue($promise); + foreach ($this->waiting as $child) { + $child->parentState = $state; + $child->parentResult = $result; + $queue->enqueue($child); } $this->waiting = []; } - - /** - * Process this promise as a waiting task (child promise awaiting parent resolution). - * - * @throws \Exception - */ - protected function processWaitingTask(): void - { - $onFulfilled = $this->waitingOnFulfilled; - $onRejected = $this->waitingOnRejected; - $state = $this->waitingState; - $result = $this->waitingResult; - - $this->waitingOnFulfilled = null; - $this->waitingOnRejected = null; - $this->waitingState = null; - $this->waitingResult = null; - - try { - if ($state === self::FULFILLED) { - $this->resolve($onFulfilled !== null - ? $onFulfilled($result) - : $result); - } elseif ($state === self::REJECTED) { - if ($onRejected === null) { - $this->reject($result); - } else { - $this->resolve($onRejected($result)); - } - } - } catch (\Throwable $e) { - $this->reject($e); - } - } } diff --git a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php index f4252029c..1101501d9 100644 --- a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php +++ b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php @@ -47,7 +47,7 @@ public function then(Promise $promise, ?callable $onFulfilled = null, ?callable */ public function create(callable $resolver): Promise { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); try { $resolver( @@ -67,7 +67,7 @@ public function create(callable $resolver): Promise */ public function createFulfilled($value = null): Promise { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); return new Promise($promise->resolve($value), $this); } @@ -78,7 +78,7 @@ public function createFulfilled($value = null): Promise */ public function createRejected(\Throwable $reason): Promise { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); return new Promise($promise->reject($reason), $this); } @@ -89,7 +89,7 @@ public function createRejected(\Throwable $reason): Promise */ public function all(iterable $promisesOrValues): Promise { - $all = new SyncPromise(); + $all = new ChildSyncPromise(); $total = is_array($promisesOrValues) ? count($promisesOrValues) diff --git a/tests/Executor/Promise/SyncPromiseAdapterTest.php b/tests/Executor/Promise/SyncPromiseAdapterTest.php index cdb486dea..870b7fbdb 100644 --- a/tests/Executor/Promise/SyncPromiseAdapterTest.php +++ b/tests/Executor/Promise/SyncPromiseAdapterTest.php @@ -4,6 +4,7 @@ use GraphQL\Deferred; use GraphQL\Error\InvariantViolation; +use GraphQL\Executor\Promise\Adapter\ChildSyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter; use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; @@ -128,8 +129,8 @@ public function testCreatePromiseAll(): void $promise = $this->promises->all(['1']); self::assertValidPromise($promise, null, ['1'], SyncPromise::FULFILLED); - $promise1 = new SyncPromise(); - $promise2 = new SyncPromise(); + $promise1 = new ChildSyncPromise(); + $promise2 = new ChildSyncPromise(); $promise3 = $promise2->then(static fn ($value): string => $value . '-value3'); $data = [ diff --git a/tests/Executor/Promise/SyncPromiseTest.php b/tests/Executor/Promise/SyncPromiseTest.php index a4a1d8760..4218220ed 100644 --- a/tests/Executor/Promise/SyncPromiseTest.php +++ b/tests/Executor/Promise/SyncPromiseTest.php @@ -3,6 +3,7 @@ namespace GraphQL\Tests\Executor\Promise; use GraphQL\Error\InvariantViolation; +use GraphQL\Executor\Promise\Adapter\ChildSyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; use GraphQL\Tests\TestCaseBase; @@ -45,7 +46,7 @@ public function testFulfilledPromiseCannotChangeValue( ?string $expectedNextReason, ?string $expectedNextState ): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->resolve($resolvedValue); @@ -64,7 +65,7 @@ public function testFulfilledPromiseCannotBeRejected( ?string $expectedNextReason, ?string $expectedNextState ): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->resolve($resolvedValue); @@ -87,7 +88,7 @@ public function testFulfilledPromise( ?string $expectedNextReason, ?string $expectedNextState ): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->resolve($resolvedValue); @@ -208,7 +209,7 @@ public function testRejectedPromiseCannotChangeReason( ?string $expectedNextReason, string $expectedNextState ): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->reject($rejectedReason); @@ -227,7 +228,7 @@ public function testRejectedPromiseCannotBeResolved( ?string $expectedNextReason, string $expectedNextState ): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->reject($rejectedReason); @@ -246,7 +247,7 @@ public function testRejectedPromise( ?string $expectedNextReason, string $expectedNextState ): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->reject($rejectedReason); @@ -283,7 +284,6 @@ static function () use (&$onFulfilledCalled): void { self::assertNotSame($promise, $nextPromise); self::assertSame(SyncPromise::PENDING, $nextPromise->state); } else { - /** @phpstan-ignore argument.unresolvableType (false positive?) */ self::assertSame(SyncPromise::REJECTED, $nextPromise->state); } @@ -305,7 +305,7 @@ static function () use (&$onFulfilledCalled): void { public function testPendingPromise(): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); try { @@ -317,7 +317,7 @@ public function testPendingPromise(): void } // Try to resolve with other promise (must resolve when other promise resolves) - $otherPromise = new SyncPromise(); + $otherPromise = new ChildSyncPromise(); $promise->resolve($otherPromise); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -328,13 +328,13 @@ public function testPendingPromise(): void self::assertSame(SyncPromise::PENDING, $promise->state); self::assertValidPromise($promise, 'the value', null, SyncPromise::FULFILLED); - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); $promise->resolve('resolved!'); self::assertValidPromise($promise, 'resolved!', null, SyncPromise::FULFILLED); // Test rejections - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); try { @@ -348,7 +348,7 @@ public function testPendingPromise(): void $promise->reject(new \Exception('Rejected Reason')); self::assertValidPromise($promise, null, 'Rejected Reason', SyncPromise::REJECTED); - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); $promise2 = $promise->then( null, static fn (): string => 'value' @@ -356,7 +356,7 @@ public function testPendingPromise(): void $promise->reject(new \Exception('Rejected Again')); self::assertValidPromise($promise2, 'value', null, SyncPromise::FULFILLED); - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); $promise2 = $promise->then(); $promise->reject(new \Exception('Rejected Once Again')); self::assertValidPromise($promise2, null, 'Rejected Once Again', SyncPromise::REJECTED); @@ -364,7 +364,7 @@ public function testPendingPromise(): void public function testPendingPromiseThen(): void { - $promise = new SyncPromise(); + $promise = new ChildSyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $nextPromise = $promise->then(); From b542f169716fc3e48800492bea17123430d2a0be Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 17:40:05 +0100 Subject: [PATCH 19/50] simplify ChildSyncPromise, benchmark is the same --- .../Promise/Adapter/ChildSyncPromise.php | 26 ++++++------------- 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/src/Executor/Promise/Adapter/ChildSyncPromise.php b/src/Executor/Promise/Adapter/ChildSyncPromise.php index 0054a8166..4cb25232c 100644 --- a/src/Executor/Promise/Adapter/ChildSyncPromise.php +++ b/src/Executor/Promise/Adapter/ChildSyncPromise.php @@ -36,26 +36,16 @@ class ChildSyncPromise extends SyncPromise /** @throws \Exception */ public function runQueuedTask(): void { - $onFulfilled = $this->onFulfilled; - $onRejected = $this->onRejected; - $state = $this->parentState; - $result = $this->parentResult; - - $this->onFulfilled = null; - $this->onRejected = null; - $this->parentState = null; - $this->parentResult = null; - try { - if ($state === self::FULFILLED) { - $this->resolve($onFulfilled !== null - ? $onFulfilled($result) - : $result); - } elseif ($state === self::REJECTED) { - if ($onRejected === null) { - $this->reject($result); + if ($this->parentState === self::FULFILLED) { + $this->resolve($this->onFulfilled !== null + ? ($this->onFulfilled)($this->parentResult) + : $this->parentResult); + } elseif ($this->parentState === self::REJECTED) { + if ($this->onRejected === null) { + $this->reject($this->parentResult); } else { - $this->resolve($onRejected($result)); + $this->resolve(($this->onRejected)($this->parentResult)); } } } catch (\Throwable $e) { From eb4f736ba4181dc1352d591b9c48d72f783cbc03 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 15 Dec 2025 17:44:46 +0100 Subject: [PATCH 20/50] comment RootSyncPromise --- src/Executor/Promise/Adapter/RootSyncPromise.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Executor/Promise/Adapter/RootSyncPromise.php b/src/Executor/Promise/Adapter/RootSyncPromise.php index ebcbded74..9e89cb742 100644 --- a/src/Executor/Promise/Adapter/RootSyncPromise.php +++ b/src/Executor/Promise/Adapter/RootSyncPromise.php @@ -28,10 +28,13 @@ public function __construct(callable $executor) /** @throws \Exception */ public function runQueuedTask(): void { + // Clear reference to allow immediate garbage collection of the closure + // and any variables it captures. Without this, closures stay alive until + // the promise object is freed, causing significant memory accumulation + // when many deferred operations are queued. $executor = $this->executor; $this->executor = null; - - assert($executor !== null); + assert($executor !== null, 'see __construct'); try { $this->resolve($executor()); From 4cd7423bd301c6dfe89aa43dfe42a0f5357515d2 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 08:32:59 +0100 Subject: [PATCH 21/50] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 885bc7a33..7ca52990c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Changed + +- Optimize `Deferred` execution https://github.com/webonyx/graphql-php/pull/1805 + ## v15.28.0 ### Changed From 5ed15147cb8aef874f5b32a0c1461396c2a68cbe Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 08:49:09 +0100 Subject: [PATCH 22/50] add DeferredBench, track memory --- benchmarks/DeferredBench.php | 80 ++++++++++++++++++++++++++++++++++++ composer.json | 2 +- phpbench.json | 8 +++- 3 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 benchmarks/DeferredBench.php diff --git a/benchmarks/DeferredBench.php b/benchmarks/DeferredBench.php new file mode 100644 index 000000000..cdd9973c0 --- /dev/null +++ b/benchmarks/DeferredBench.php @@ -0,0 +1,80 @@ + 'value'); + SyncPromiseQueue::getInstance()->run(); + } + + public function benchNestedDeferred(): void + { + new Deferred(static fn () => new Deferred(static fn () => null)); + SyncPromiseQueue::getInstance()->run(); + } + + public function benchChain5(): void + { + $deferred = new Deferred(static fn () => 'value'); + $deferred->then(static fn ($v) => $v) + ->then(static fn ($v) => $v) + ->then(static fn ($v) => $v) + ->then(static fn ($v) => $v) + ->then(static fn ($v) => $v); + SyncPromiseQueue::getInstance()->run(); + } + + public function benchChain100(): void + { + $deferred = new Deferred(static fn () => 'value'); + $promise = $deferred; + for ($i = 0; $i < 100; ++$i) { + $promise = $promise->then(static fn ($v) => $v); + } + SyncPromiseQueue::getInstance()->run(); + } + + public function benchManyDeferreds(): void + { + $fn = static fn () => null; + for ($i = 0; $i < 1000; ++$i) { + new Deferred($fn); + } + SyncPromiseQueue::getInstance()->run(); + } + + public function benchManyNestedDeferreds(): void + { + for ($i = 0; $i < 5000; ++$i) { + new Deferred(static fn () => new Deferred(static fn () => null)); + } + SyncPromiseQueue::getInstance()->run(); + } + + public function bench1000Chains(): void + { + $promises = []; + for ($i = 0; $i < 1000; ++$i) { + $d = new Deferred(static fn () => $i); + $promises[] = $d->then(static fn ($v) => $v) + ->then(static fn ($v) => $v) + ->then(static fn ($v) => $v); + } + SyncPromiseQueue::getInstance()->run(); + } +} diff --git a/composer.json b/composer.json index 5b95733e7..ff0551a92 100644 --- a/composer.json +++ b/composer.json @@ -64,7 +64,7 @@ }, "scripts": { "baseline": "phpstan --generate-baseline", - "bench": "phpbench run", + "bench": "phpbench run --report=default", "check": [ "@fix", "@stan", diff --git a/phpbench.json b/phpbench.json index 28c601c87..b47419350 100644 --- a/phpbench.json +++ b/phpbench.json @@ -3,5 +3,11 @@ "runner.path": "benchmarks", "runner.file_pattern": "*Bench.php", "runner.retry_threshold": 5, - "runner.time_unit": "milliseconds" + "runner.time_unit": "milliseconds", + "runner.progress": "plain", + "report.generators": { + "default": { + "extends": "aggregate" + } + } } From 1427bfaeafb8bdfb41c52962207a05129b9d8e33 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 09:50:56 +0100 Subject: [PATCH 23/50] more revs for benchmarks --- benchmarks/HugeSchemaBench.php | 9 ++++++--- benchmarks/StarWarsBench.php | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/benchmarks/HugeSchemaBench.php b/benchmarks/HugeSchemaBench.php index fe6bb1821..b440efcb5 100644 --- a/benchmarks/HugeSchemaBench.php +++ b/benchmarks/HugeSchemaBench.php @@ -14,11 +14,11 @@ * * @OutputTimeUnit("milliseconds", precision=3) * - * @Warmup(1) + * @Warmup(2) * - * @Revs(5) + * @Revs(10) * - * @Iterations(1) + * @Iterations(3) */ class HugeSchemaBench { @@ -50,6 +50,9 @@ public function benchSchema(): void ->getTypeMap(); } + /** + * @Revs(1000) + */ public function benchSchemaLazy(): void { $this->createLazySchema(); diff --git a/benchmarks/StarWarsBench.php b/benchmarks/StarWarsBench.php index 394e554ad..4c80e84a9 100644 --- a/benchmarks/StarWarsBench.php +++ b/benchmarks/StarWarsBench.php @@ -13,9 +13,9 @@ * * @Warmup(2) * - * @Revs(10) + * @Revs(50) * - * @Iterations(2) + * @Iterations(5) */ class StarWarsBench { From dfd1f05f754c46367c63958d5295be93f94f737b Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 11:32:09 +0100 Subject: [PATCH 24/50] Optimize SyncPromise with hybrid closure approach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use lightweight static closures in queue that only capture promise references, while storing heavy payload (callbacks, executors) on promise objects. Clear heavy data after execution to enable garbage collection. This achieves both: - Fast execution (closure-based queue like master) - Memory efficiency (no parent reference chains) Consolidate queue logic into SyncPromiseQueue and remove the RootSyncPromise/ChildSyncPromise class hierarchy. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- benchmarks/DeferredBench.php | 14 +- src/Deferred.php | 4 +- .../Promise/Adapter/ChildSyncPromise.php | 55 ------ .../Promise/Adapter/RootSyncPromise.php | 45 ----- src/Executor/Promise/Adapter/SyncPromise.php | 173 ++++++++++++++---- .../Promise/Adapter/SyncPromiseAdapter.php | 13 +- .../Promise/Adapter/SyncPromiseQueue.php | 49 +++-- .../Promise/SyncPromiseAdapterTest.php | 7 +- tests/Executor/Promise/SyncPromiseTest.php | 43 +++-- 9 files changed, 199 insertions(+), 204 deletions(-) delete mode 100644 src/Executor/Promise/Adapter/ChildSyncPromise.php delete mode 100644 src/Executor/Promise/Adapter/RootSyncPromise.php diff --git a/benchmarks/DeferredBench.php b/benchmarks/DeferredBench.php index cdd9973c0..4b4f7a026 100644 --- a/benchmarks/DeferredBench.php +++ b/benchmarks/DeferredBench.php @@ -19,13 +19,13 @@ class DeferredBench public function benchSingleDeferred(): void { new Deferred(static fn () => 'value'); - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); } public function benchNestedDeferred(): void { new Deferred(static fn () => new Deferred(static fn () => null)); - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); } public function benchChain5(): void @@ -36,7 +36,7 @@ public function benchChain5(): void ->then(static fn ($v) => $v) ->then(static fn ($v) => $v) ->then(static fn ($v) => $v); - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); } public function benchChain100(): void @@ -46,7 +46,7 @@ public function benchChain100(): void for ($i = 0; $i < 100; ++$i) { $promise = $promise->then(static fn ($v) => $v); } - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); } public function benchManyDeferreds(): void @@ -55,7 +55,7 @@ public function benchManyDeferreds(): void for ($i = 0; $i < 1000; ++$i) { new Deferred($fn); } - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); } public function benchManyNestedDeferreds(): void @@ -63,7 +63,7 @@ public function benchManyNestedDeferreds(): void for ($i = 0; $i < 5000; ++$i) { new Deferred(static fn () => new Deferred(static fn () => null)); } - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); } public function bench1000Chains(): void @@ -75,6 +75,6 @@ public function bench1000Chains(): void ->then(static fn ($v) => $v) ->then(static fn ($v) => $v); } - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); } } diff --git a/src/Deferred.php b/src/Deferred.php index fccc5f3cd..c19afcada 100644 --- a/src/Deferred.php +++ b/src/Deferred.php @@ -2,14 +2,14 @@ namespace GraphQL; -use GraphQL\Executor\Promise\Adapter\RootSyncPromise; +use GraphQL\Executor\Promise\Adapter\SyncPromise; /** * User-facing promise class for deferred field resolution. * * @phpstan-type Executor callable(): mixed */ -class Deferred extends RootSyncPromise +class Deferred extends SyncPromise { /** @param Executor $executor */ public static function create(callable $executor): self diff --git a/src/Executor/Promise/Adapter/ChildSyncPromise.php b/src/Executor/Promise/Adapter/ChildSyncPromise.php deleted file mode 100644 index 4cb25232c..000000000 --- a/src/Executor/Promise/Adapter/ChildSyncPromise.php +++ /dev/null @@ -1,55 +0,0 @@ -parentState === self::FULFILLED) { - $this->resolve($this->onFulfilled !== null - ? ($this->onFulfilled)($this->parentResult) - : $this->parentResult); - } elseif ($this->parentState === self::REJECTED) { - if ($this->onRejected === null) { - $this->reject($this->parentResult); - } else { - $this->resolve(($this->onRejected)($this->parentResult)); - } - } - } catch (\Throwable $e) { - $this->reject($e); - } - } -} diff --git a/src/Executor/Promise/Adapter/RootSyncPromise.php b/src/Executor/Promise/Adapter/RootSyncPromise.php deleted file mode 100644 index 9e89cb742..000000000 --- a/src/Executor/Promise/Adapter/RootSyncPromise.php +++ /dev/null @@ -1,45 +0,0 @@ -executor = $executor; - - $queue = SyncPromiseQueue::getInstance(); - $queue->enqueue($this); - } - - /** @throws \Exception */ - public function runQueuedTask(): void - { - // Clear reference to allow immediate garbage collection of the closure - // and any variables it captures. Without this, closures stay alive until - // the promise object is freed, causing significant memory accumulation - // when many deferred operations are queued. - $executor = $this->executor; - $this->executor = null; - assert($executor !== null, 'see __construct'); - - try { - $this->resolve($executor()); - } catch (\Throwable $e) { - $this->reject($e); - } - } -} diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index cfcae3b94..bcc75c9f2 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -5,20 +5,27 @@ use GraphQL\Error\InvariantViolation; /** - * Base class for synchronous promises implementing Promises A+ spec. + * Synchronous promise implementation following Promises A+ spec. + * + * Uses a hybrid approach for optimal memory and performance: + * - Lightweight closures in queue (fast execution) + * - Heavy payload (callbacks) stored on promise objects and cleared after use * * Library users should use @see \GraphQL\Deferred to create promises. * - * @see RootSyncPromise for promises created with an executor - * @see ChildSyncPromise for promises created via then() chaining + * @phpstan-type Executor callable(): mixed */ -abstract class SyncPromise +class SyncPromise { public const PENDING = 'pending'; public const FULFILLED = 'fulfilled'; public const REJECTED = 'rejected'; - /** Current promise state: PENDING, FULFILLED, or REJECTED. */ + /** + * Current promise state: PENDING, FULFILLED, or REJECTED. + * + * @var 'pending'|'fulfilled'|'rejected' + */ public string $state = self::PENDING; /** @@ -29,14 +36,100 @@ abstract class SyncPromise public $result; /** - * Child promises waiting for this promise to resolve. + * Executor for deferred promises. + * + * @var (callable(): mixed)|null + */ + protected $executor; + + /** + * Callback handlers for child promises (set when parent resolves). * - * @var array + * @var array{ + * (callable(mixed): mixed)|null, + * (callable(\Throwable): mixed)|null, + * mixed + * }|null + */ + protected ?array $pendingCallback = null; + + /** + * Promises created in `then` method awaiting resolution. + * + * @var array< + * int, + * array{ + * self, + * (callable(mixed): mixed)|null, + * (callable(\Throwable): mixed)|null + * } + * > */ protected array $waiting = []; - /** Execute the queued task for this promise. */ - abstract public function runQueuedTask(): void; + /** @param Executor|null $executor */ + public function __construct(?callable $executor = null) + { + if ($executor === null) { + return; + } + + $this->executor = $executor; + $promise = $this; + // Lightweight closure - only captures promise reference + SyncPromiseQueue::enqueue(static function () use ($promise): void { + $promise->runExecutor(); + }); + } + + /** Execute the deferred callback and clear it for garbage collection. */ + protected function runExecutor(): void + { + $executor = $this->executor; + $this->executor = null; // Clear for garbage collection + + if ($executor === null) { + return; + } + + try { + $this->resolve($executor()); + } catch (\Throwable $e) { + $this->reject($e); + } + } + + /** Execute the pending callback and clear it for garbage collection. */ + protected function runPendingCallback(): void + { + $callback = $this->pendingCallback; + $this->pendingCallback = null; // Clear for garbage collection + + if ($callback === null) { + return; + } + + [$onFulfilled, $onRejected, $parentResult] = $callback; + + // Determine parent state from result type + $parentRejected = $parentResult instanceof \Throwable; + + try { + if (! $parentRejected) { + $this->resolve( + $onFulfilled === null ? $parentResult : $onFulfilled($parentResult) + ); + } else { + if ($onRejected === null) { + $this->reject($parentResult); + } else { + $this->resolve($onRejected($parentResult)); + } + } + } catch (\Throwable $e) { + $this->reject($e); + } + } /** * @param mixed $value @@ -56,7 +149,7 @@ public function resolve($value): self function ($resolvedValue): void { $this->resolve($resolvedValue); }, - function ($reason): void { + function (\Throwable $reason): void { $this->reject($reason); } ); @@ -107,6 +200,28 @@ public function reject(\Throwable $reason): self return $this; } + /** @throws InvariantViolation */ + private function enqueueWaitingPromises(): void + { + if ($this->state === self::PENDING) { + throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); + } + + $result = $this->result; + + foreach ($this->waiting as [$childPromise, $onFulfilled, $onRejected]) { + // Store callback data on child promise to allow garbage collection after execution + $childPromise->pendingCallback = [$onFulfilled, $onRejected, $result]; + + // Only capture child promise reference + SyncPromiseQueue::enqueue(static function () use ($childPromise): void { + $childPromise->runPendingCallback(); + }); + } + + $this->waiting = []; + } + /** * @param (callable(mixed): mixed)|null $onFulfilled * @param (callable(\Throwable): mixed)|null $onRejected @@ -115,22 +230,16 @@ public function reject(\Throwable $reason): self */ public function then(?callable $onFulfilled = null, ?callable $onRejected = null): self { - if ($this->state === self::REJECTED - && $onRejected === null - ) { + if ($this->state === self::REJECTED && $onRejected === null) { return $this; } - if ($this->state === self::FULFILLED - && $onFulfilled === null - ) { + if ($this->state === self::FULFILLED && $onFulfilled === null) { return $this; } - $child = new ChildSyncPromise(); - $child->onFulfilled = $onFulfilled; - $child->onRejected = $onRejected; - $this->waiting[] = $child; + $child = new self(); + $this->waiting[] = [$child, $onFulfilled, $onRejected]; if ($this->state !== self::PENDING) { $this->enqueueWaitingPromises(); @@ -139,23 +248,13 @@ public function then(?callable $onFulfilled = null, ?callable $onRejected = null return $child; } - /** @throws InvariantViolation */ - protected function enqueueWaitingPromises(): void + /** + * @param callable(\Throwable): mixed $onRejected + * + * @throws InvariantViolation + */ + public function catch(callable $onRejected): self { - if ($this->state === self::PENDING) { - throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); - } - - $state = $this->state; - $result = $this->result; - $queue = SyncPromiseQueue::getInstance(); - - foreach ($this->waiting as $child) { - $child->parentState = $state; - $child->parentResult = $result; - $queue->enqueue($child); - } - - $this->waiting = []; + return $this->then(null, $onRejected); } } diff --git a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php index 1101501d9..61e03fe9e 100644 --- a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php +++ b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php @@ -47,7 +47,7 @@ public function then(Promise $promise, ?callable $onFulfilled = null, ?callable */ public function create(callable $resolver): Promise { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); try { $resolver( @@ -67,7 +67,7 @@ public function create(callable $resolver): Promise */ public function createFulfilled($value = null): Promise { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); return new Promise($promise->resolve($value), $this); } @@ -78,7 +78,7 @@ public function createFulfilled($value = null): Promise */ public function createRejected(\Throwable $reason): Promise { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); return new Promise($promise->reject($reason), $this); } @@ -89,7 +89,7 @@ public function createRejected(\Throwable $reason): Promise */ public function all(iterable $promisesOrValues): Promise { - $all = new ChildSyncPromise(); + $all = new SyncPromise(); $total = is_array($promisesOrValues) ? count($promisesOrValues) @@ -135,16 +135,15 @@ static function ($value) use (&$result, $index, &$count, &$resolveAllWhenFinishe public function wait(Promise $promise) { $this->beforeWait($promise); - $queue = SyncPromiseQueue::getInstance(); $syncPromise = $promise->adoptedPromise; assert($syncPromise instanceof SyncPromise); while ( $syncPromise->state === SyncPromise::PENDING - && ! $queue->isEmpty() + && ! SyncPromiseQueue::isEmpty() ) { - $queue->run(); + SyncPromiseQueue::run(); $this->onWait($promise); } diff --git a/src/Executor/Promise/Adapter/SyncPromiseQueue.php b/src/Executor/Promise/Adapter/SyncPromiseQueue.php index d0dcf4394..2ef492344 100644 --- a/src/Executor/Promise/Adapter/SyncPromiseQueue.php +++ b/src/Executor/Promise/Adapter/SyncPromiseQueue.php @@ -2,46 +2,45 @@ namespace GraphQL\Executor\Promise\Adapter; -/** Queue for deferred execution of SyncPromise tasks. */ +/** + * Queue for deferred execution of SyncPromise tasks. + * + * Owns the shared queue and provides the run loop for processing promises. + */ class SyncPromiseQueue { - protected static ?self $instance = null; - - /** @var \SplQueue */ - protected \SplQueue $queue; - - public function __construct() + /** @return \SplQueue */ + private static function queue(): \SplQueue { - $this->queue = new \SplQueue(); - } + /** @var \SplQueue|null $queue */ + static $queue; - public static function getInstance(): self - { - return self::$instance ??= new self(); + return $queue ??= new \SplQueue(); } - public function enqueue(SyncPromise $promise): void + /** @param callable(): void $task */ + public static function enqueue(callable $task): void { - $this->queue->enqueue($promise); + self::queue()->enqueue($task); } - public function isEmpty(): bool + /** Process all queued promises until the queue is empty. */ + public static function run(): void { - return $this->queue->isEmpty(); + $queue = self::queue(); + while (! $queue->isEmpty()) { + $task = $queue->dequeue(); + $task(); + } } - public function count(): int + public static function isEmpty(): bool { - return $this->queue->count(); + return self::queue()->isEmpty(); } - /** Process all queued promises until the queue is empty. */ - public function run(): void + public static function count(): int { - while (! $this->queue->isEmpty()) { - $task = $this->queue->dequeue(); - $task->runQueuedTask(); - unset($task); - } + return self::queue()->count(); } } diff --git a/tests/Executor/Promise/SyncPromiseAdapterTest.php b/tests/Executor/Promise/SyncPromiseAdapterTest.php index 870b7fbdb..59afb577c 100644 --- a/tests/Executor/Promise/SyncPromiseAdapterTest.php +++ b/tests/Executor/Promise/SyncPromiseAdapterTest.php @@ -4,7 +4,6 @@ use GraphQL\Deferred; use GraphQL\Error\InvariantViolation; -use GraphQL\Executor\Promise\Adapter\ChildSyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter; use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; @@ -92,7 +91,7 @@ static function (\Throwable $reason) use (&$actualNextReason, &$onRejectedCalled self::assertFalse($onFulfilledCalled); self::assertFalse($onRejectedCalled); - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); if ($expectedNextState !== SyncPromise::PENDING) { if ($expectedNextReason === null) { @@ -129,8 +128,8 @@ public function testCreatePromiseAll(): void $promise = $this->promises->all(['1']); self::assertValidPromise($promise, null, ['1'], SyncPromise::FULFILLED); - $promise1 = new ChildSyncPromise(); - $promise2 = new ChildSyncPromise(); + $promise1 = new SyncPromise(); + $promise2 = new SyncPromise(); $promise3 = $promise2->then(static fn ($value): string => $value . '-value3'); $data = [ diff --git a/tests/Executor/Promise/SyncPromiseTest.php b/tests/Executor/Promise/SyncPromiseTest.php index 4218220ed..238e80c3c 100644 --- a/tests/Executor/Promise/SyncPromiseTest.php +++ b/tests/Executor/Promise/SyncPromiseTest.php @@ -3,7 +3,6 @@ namespace GraphQL\Tests\Executor\Promise; use GraphQL\Error\InvariantViolation; -use GraphQL\Executor\Promise\Adapter\ChildSyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; use GraphQL\Tests\TestCaseBase; @@ -46,7 +45,7 @@ public function testFulfilledPromiseCannotChangeValue( ?string $expectedNextReason, ?string $expectedNextState ): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->resolve($resolvedValue); @@ -65,7 +64,7 @@ public function testFulfilledPromiseCannotBeRejected( ?string $expectedNextReason, ?string $expectedNextState ): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->resolve($resolvedValue); @@ -88,7 +87,7 @@ public function testFulfilledPromise( ?string $expectedNextReason, ?string $expectedNextState ): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->resolve($resolvedValue); @@ -127,7 +126,7 @@ static function () use (&$onRejectedCalled): void { self::assertNotSame($nextPromise, $nextPromise2); } - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); self::assertValidPromise($nextPromise2, $expectedNextValue, $expectedNextReason, $expectedNextState); self::assertValidPromise($nextPromise3, $expectedNextValue, $expectedNextReason, $expectedNextState); @@ -163,7 +162,7 @@ static function (\Throwable $reason) use (&$actualNextReason, &$onRejectedCalled self::assertFalse($onFulfilledCalled); self::assertFalse($onRejectedCalled); - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); if ($expectedNextReason === null) { self::assertTrue($onFulfilledCalled); // @phpstan-ignore-line value is mutable @@ -209,7 +208,7 @@ public function testRejectedPromiseCannotChangeReason( ?string $expectedNextReason, string $expectedNextState ): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->reject($rejectedReason); @@ -228,7 +227,7 @@ public function testRejectedPromiseCannotBeResolved( ?string $expectedNextReason, string $expectedNextState ): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->reject($rejectedReason); @@ -247,7 +246,7 @@ public function testRejectedPromise( ?string $expectedNextReason, string $expectedNextState ): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $promise->reject($rejectedReason); @@ -297,7 +296,7 @@ static function () use (&$onFulfilledCalled): void { self::assertNotSame($nextPromise, $nextPromise2); } - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); self::assertValidPromise($nextPromise2, $expectedNextValue, $expectedNextReason, $expectedNextState); self::assertValidPromise($nextPromise3, $expectedNextValue, $expectedNextReason, $expectedNextState); @@ -305,7 +304,7 @@ static function () use (&$onFulfilledCalled): void { public function testPendingPromise(): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); try { @@ -317,7 +316,7 @@ public function testPendingPromise(): void } // Try to resolve with other promise (must resolve when other promise resolves) - $otherPromise = new ChildSyncPromise(); + $otherPromise = new SyncPromise(); $promise->resolve($otherPromise); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -328,13 +327,13 @@ public function testPendingPromise(): void self::assertSame(SyncPromise::PENDING, $promise->state); self::assertValidPromise($promise, 'the value', null, SyncPromise::FULFILLED); - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); $promise->resolve('resolved!'); self::assertValidPromise($promise, 'resolved!', null, SyncPromise::FULFILLED); // Test rejections - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); try { @@ -348,7 +347,7 @@ public function testPendingPromise(): void $promise->reject(new \Exception('Rejected Reason')); self::assertValidPromise($promise, null, 'Rejected Reason', SyncPromise::REJECTED); - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); $promise2 = $promise->then( null, static fn (): string => 'value' @@ -356,7 +355,7 @@ public function testPendingPromise(): void $promise->reject(new \Exception('Rejected Again')); self::assertValidPromise($promise2, 'value', null, SyncPromise::FULFILLED); - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); $promise2 = $promise->then(); $promise->reject(new \Exception('Rejected Once Again')); self::assertValidPromise($promise2, null, 'Rejected Once Again', SyncPromise::REJECTED); @@ -364,7 +363,7 @@ public function testPendingPromise(): void public function testPendingPromiseThen(): void { - $promise = new ChildSyncPromise(); + $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); $nextPromise = $promise->then(); @@ -391,12 +390,12 @@ public function testPendingPromiseThen(): void $nextPromise3 = $promise->then($onFulfilled, $onRejected); $nextPromise4 = $promise->then($onFulfilled, $onRejected); - self::assertSame(0, SyncPromiseQueue::getInstance()->count()); + self::assertSame(0, SyncPromiseQueue::count()); self::assertSame(0, $onFulfilledCount); self::assertSame(0, $onRejectedCount); $promise->resolve(1); - self::assertSame(4, SyncPromiseQueue::getInstance()->count()); + self::assertSame(4, SyncPromiseQueue::count()); self::assertSame(0, $onFulfilledCount); // @phpstan-ignore-line side-effects self::assertSame(0, $onRejectedCount); // @phpstan-ignore-line side-effects self::assertSame(SyncPromise::PENDING, $nextPromise->state); @@ -404,8 +403,8 @@ public function testPendingPromiseThen(): void self::assertSame(SyncPromise::PENDING, $nextPromise3->state); self::assertSame(SyncPromise::PENDING, $nextPromise4->state); - SyncPromiseQueue::getInstance()->run(); - self::assertSame(0, SyncPromiseQueue::getInstance()->count()); + SyncPromiseQueue::run(); + self::assertSame(0, SyncPromiseQueue::count()); self::assertSame(3, $onFulfilledCount); // @phpstan-ignore-line side-effects self::assertSame(0, $onRejectedCount); // @phpstan-ignore-line side-effects self::assertValidPromise($nextPromise, 1, null, SyncPromise::FULFILLED); @@ -416,7 +415,7 @@ public function testPendingPromiseThen(): void public function testRunEmptyQueue(): void { - SyncPromiseQueue::getInstance()->run(); + SyncPromiseQueue::run(); $this->assertDidNotCrash(); } } From ce0db1648c99919bcef1c76bcf29e8d0340595d2 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 12:03:00 +0100 Subject: [PATCH 25/50] format --- benchmarks/HugeSchemaBench.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/benchmarks/HugeSchemaBench.php b/benchmarks/HugeSchemaBench.php index b440efcb5..1f58835a5 100644 --- a/benchmarks/HugeSchemaBench.php +++ b/benchmarks/HugeSchemaBench.php @@ -50,9 +50,7 @@ public function benchSchema(): void ->getTypeMap(); } - /** - * @Revs(1000) - */ + /** @Revs(1000) */ public function benchSchemaLazy(): void { $this->createLazySchema(); From ad597a8608976fcd890e5f8c7c682227f6d466c8 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 12:08:56 +0100 Subject: [PATCH 26/50] fix test phpstan --- tests/Executor/Promise/SyncPromiseTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Executor/Promise/SyncPromiseTest.php b/tests/Executor/Promise/SyncPromiseTest.php index 238e80c3c..64b8bf81c 100644 --- a/tests/Executor/Promise/SyncPromiseTest.php +++ b/tests/Executor/Promise/SyncPromiseTest.php @@ -283,6 +283,7 @@ static function () use (&$onFulfilledCalled): void { self::assertNotSame($promise, $nextPromise); self::assertSame(SyncPromise::PENDING, $nextPromise->state); } else { + /** @phpstan-ignore argument.unresolvableType (false positive?) */ self::assertSame(SyncPromise::REJECTED, $nextPromise->state); } From 77c172595c8d3ee0505a46cd758ed229677b9b72 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 13:53:01 +0100 Subject: [PATCH 27/50] reorder --- .../Promise/Adapter/SyncPromiseQueue.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromiseQueue.php b/src/Executor/Promise/Adapter/SyncPromiseQueue.php index 2ef492344..b47d01f0a 100644 --- a/src/Executor/Promise/Adapter/SyncPromiseQueue.php +++ b/src/Executor/Promise/Adapter/SyncPromiseQueue.php @@ -9,15 +9,6 @@ */ class SyncPromiseQueue { - /** @return \SplQueue */ - private static function queue(): \SplQueue - { - /** @var \SplQueue|null $queue */ - static $queue; - - return $queue ??= new \SplQueue(); - } - /** @param callable(): void $task */ public static function enqueue(callable $task): void { @@ -43,4 +34,13 @@ public static function count(): int { return self::queue()->count(); } + + /** @return \SplQueue */ + private static function queue(): \SplQueue + { + /** @var \SplQueue|null $queue */ + static $queue; + + return $queue ??= new \SplQueue(); + } } From 31d24a1c9ec64edb38620e4795b953b7a3e87b61 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 13:57:15 +0100 Subject: [PATCH 28/50] format --- src/Executor/Promise/Adapter/SyncPromise.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index bcc75c9f2..5a2af627b 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -61,8 +61,8 @@ class SyncPromise * array{ * self, * (callable(mixed): mixed)|null, - * (callable(\Throwable): mixed)|null - * } + * (callable(\Throwable): mixed)|null, + * }, * > */ protected array $waiting = []; From d679079830c28a4e96f76c5bfb595d8ff002cf20 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 15:16:28 +0100 Subject: [PATCH 29/50] simplify --- src/Executor/Promise/Adapter/SyncPromiseAdapter.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php index 61e03fe9e..d0f90103d 100644 --- a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php +++ b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php @@ -139,10 +139,7 @@ public function wait(Promise $promise) $syncPromise = $promise->adoptedPromise; assert($syncPromise instanceof SyncPromise); - while ( - $syncPromise->state === SyncPromise::PENDING - && ! SyncPromiseQueue::isEmpty() - ) { + while ($syncPromise->state === SyncPromise::PENDING) { SyncPromiseQueue::run(); $this->onWait($promise); } From 9eee7116697f67cfc7fc982d6d7cdcf229d8e6dc Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 16:40:21 +0100 Subject: [PATCH 30/50] reduce variance --- benchmarks/DeferredBench.php | 6 +++--- benchmarks/StarWarsBench.php | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/benchmarks/DeferredBench.php b/benchmarks/DeferredBench.php index 4b4f7a026..c63ed6915 100644 --- a/benchmarks/DeferredBench.php +++ b/benchmarks/DeferredBench.php @@ -8,11 +8,11 @@ /** * @OutputTimeUnit("microseconds", precision=3) * - * @Warmup(2) + * @Warmup(5) * - * @Revs(100) + * @Revs(200) * - * @Iterations(5) + * @Iterations(10) */ class DeferredBench { diff --git a/benchmarks/StarWarsBench.php b/benchmarks/StarWarsBench.php index 4c80e84a9..b56057055 100644 --- a/benchmarks/StarWarsBench.php +++ b/benchmarks/StarWarsBench.php @@ -11,11 +11,11 @@ * * @OutputTimeUnit("milliseconds", precision=3) * - * @Warmup(2) + * @Warmup(5) * - * @Revs(50) + * @Revs(100) * - * @Iterations(5) + * @Iterations(10) */ class StarWarsBench { From ba40eea0cb55fdddaa0903879459d44d8b45b433 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 16:43:28 +0100 Subject: [PATCH 31/50] temporary compat layer --- benchmarks/DeferredBench.php | 16 ++++++++-------- src/Executor/Promise/Adapter/SyncPromise.php | 10 ++++++++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/benchmarks/DeferredBench.php b/benchmarks/DeferredBench.php index c63ed6915..f631c5e67 100644 --- a/benchmarks/DeferredBench.php +++ b/benchmarks/DeferredBench.php @@ -3,7 +3,7 @@ namespace GraphQL\Benchmarks; use GraphQL\Deferred; -use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; +use GraphQL\Executor\Promise\Adapter\SyncPromise; /** * @OutputTimeUnit("microseconds", precision=3) @@ -19,13 +19,13 @@ class DeferredBench public function benchSingleDeferred(): void { new Deferred(static fn () => 'value'); - SyncPromiseQueue::run(); + SyncPromise::runQueue(); } public function benchNestedDeferred(): void { new Deferred(static fn () => new Deferred(static fn () => null)); - SyncPromiseQueue::run(); + SyncPromise::runQueue(); } public function benchChain5(): void @@ -36,7 +36,7 @@ public function benchChain5(): void ->then(static fn ($v) => $v) ->then(static fn ($v) => $v) ->then(static fn ($v) => $v); - SyncPromiseQueue::run(); + SyncPromise::runQueue(); } public function benchChain100(): void @@ -46,7 +46,7 @@ public function benchChain100(): void for ($i = 0; $i < 100; ++$i) { $promise = $promise->then(static fn ($v) => $v); } - SyncPromiseQueue::run(); + SyncPromise::runQueue(); } public function benchManyDeferreds(): void @@ -55,7 +55,7 @@ public function benchManyDeferreds(): void for ($i = 0; $i < 1000; ++$i) { new Deferred($fn); } - SyncPromiseQueue::run(); + SyncPromise::runQueue(); } public function benchManyNestedDeferreds(): void @@ -63,7 +63,7 @@ public function benchManyNestedDeferreds(): void for ($i = 0; $i < 5000; ++$i) { new Deferred(static fn () => new Deferred(static fn () => null)); } - SyncPromiseQueue::run(); + SyncPromise::runQueue(); } public function bench1000Chains(): void @@ -75,6 +75,6 @@ public function bench1000Chains(): void ->then(static fn ($v) => $v) ->then(static fn ($v) => $v); } - SyncPromiseQueue::run(); + SyncPromise::runQueue(); } } diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 5a2af627b..5b5276fc1 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -67,6 +67,16 @@ class SyncPromise */ protected array $waiting = []; + /** + * Process all queued promises until the queue is empty. + * + * TODO remove before merging, not part of public API + */ + public static function runQueue(): void + { + SyncPromiseQueue::run(); + } + /** @param Executor|null $executor */ public function __construct(?callable $executor = null) { From 0b897dd64bfbcfd7678524a16fd0237949b5908c Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 16:45:23 +0100 Subject: [PATCH 32/50] reorder methods --- src/Executor/Promise/Adapter/SyncPromise.php | 109 +++++++++---------- 1 file changed, 54 insertions(+), 55 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 5b5276fc1..6af55a68b 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -109,38 +109,6 @@ protected function runExecutor(): void } } - /** Execute the pending callback and clear it for garbage collection. */ - protected function runPendingCallback(): void - { - $callback = $this->pendingCallback; - $this->pendingCallback = null; // Clear for garbage collection - - if ($callback === null) { - return; - } - - [$onFulfilled, $onRejected, $parentResult] = $callback; - - // Determine parent state from result type - $parentRejected = $parentResult instanceof \Throwable; - - try { - if (! $parentRejected) { - $this->resolve( - $onFulfilled === null ? $parentResult : $onFulfilled($parentResult) - ); - } else { - if ($onRejected === null) { - $this->reject($parentResult); - } else { - $this->resolve($onRejected($parentResult)); - } - } - } catch (\Throwable $e) { - $this->reject($e); - } - } - /** * @param mixed $value * @@ -209,29 +177,6 @@ public function reject(\Throwable $reason): self return $this; } - - /** @throws InvariantViolation */ - private function enqueueWaitingPromises(): void - { - if ($this->state === self::PENDING) { - throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); - } - - $result = $this->result; - - foreach ($this->waiting as [$childPromise, $onFulfilled, $onRejected]) { - // Store callback data on child promise to allow garbage collection after execution - $childPromise->pendingCallback = [$onFulfilled, $onRejected, $result]; - - // Only capture child promise reference - SyncPromiseQueue::enqueue(static function () use ($childPromise): void { - $childPromise->runPendingCallback(); - }); - } - - $this->waiting = []; - } - /** * @param (callable(mixed): mixed)|null $onFulfilled * @param (callable(\Throwable): mixed)|null $onRejected @@ -267,4 +212,58 @@ public function catch(callable $onRejected): self { return $this->then(null, $onRejected); } + + /** @throws InvariantViolation */ + private function enqueueWaitingPromises(): void + { + if ($this->state === self::PENDING) { + throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); + } + + $result = $this->result; + + foreach ($this->waiting as [$childPromise, $onFulfilled, $onRejected]) { + // Store callback data on child promise to allow garbage collection after execution + $childPromise->pendingCallback = [$onFulfilled, $onRejected, $result]; + + // Only capture child promise reference + SyncPromiseQueue::enqueue(static function () use ($childPromise): void { + $childPromise->runPendingCallback(); + }); + } + + $this->waiting = []; + } + + /** Execute the pending callback and clear it for garbage collection. */ + protected function runPendingCallback(): void + { + $callback = $this->pendingCallback; + $this->pendingCallback = null; // Clear for garbage collection + + if ($callback === null) { + return; + } + + [$onFulfilled, $onRejected, $parentResult] = $callback; + + // Determine parent state from result type + $parentRejected = $parentResult instanceof \Throwable; + + try { + if (! $parentRejected) { + $this->resolve( + $onFulfilled === null ? $parentResult : $onFulfilled($parentResult) + ); + } else { + if ($onRejected === null) { + $this->reject($parentResult); + } else { + $this->resolve($onRejected($parentResult)); + } + } + } catch (\Throwable $e) { + $this->reject($e); + } + } } From e971258538ac9080ca39f669c79834cf54b3a30d Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 16:45:46 +0100 Subject: [PATCH 33/50] cs-fixer --- src/Executor/Promise/Adapter/SyncPromise.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 6af55a68b..45d8b5438 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -177,6 +177,7 @@ public function reject(\Throwable $reason): self return $this; } + /** * @param (callable(mixed): mixed)|null $onFulfilled * @param (callable(\Throwable): mixed)|null $onRejected From c35557ced2593f6d084a381b61d360b3ab246791 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 17:04:26 +0100 Subject: [PATCH 34/50] remove unnecessary null-check --- src/Executor/Promise/Adapter/SyncPromise.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 45d8b5438..3ca924079 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -98,10 +98,6 @@ protected function runExecutor(): void $executor = $this->executor; $this->executor = null; // Clear for garbage collection - if ($executor === null) { - return; - } - try { $this->resolve($executor()); } catch (\Throwable $e) { @@ -242,10 +238,6 @@ protected function runPendingCallback(): void $callback = $this->pendingCallback; $this->pendingCallback = null; // Clear for garbage collection - if ($callback === null) { - return; - } - [$onFulfilled, $onRejected, $parentResult] = $callback; // Determine parent state from result type From 956d9331355a6eb430544e4470710f888bc72052 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 17:30:33 +0100 Subject: [PATCH 35/50] always bench without assertions --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index ff0551a92..f63d62807 100644 --- a/composer.json +++ b/composer.json @@ -64,7 +64,7 @@ }, "scripts": { "baseline": "phpstan --generate-baseline", - "bench": "phpbench run --report=default", + "bench": "php -d zend.assertions=-1 vendor/bin/phpbench run --report=default", "check": [ "@fix", "@stan", @@ -78,6 +78,6 @@ "php-cs-fixer": "make php-cs-fixer", "rector": "rector process", "stan": "phpstan --verbose", - "test": "php -d zend.exception_ignore_args=Off -d zend.assertions=On -d assert.active=On -d assert.exception=On vendor/bin/phpunit" + "test": "php -d zend.exception_ignore_args=0 -d zend.assertions=1 -d assert.active=1 -d assert.exception=1 vendor/bin/phpunit" } } From 9b56e21592c8078f413c430a52fe5a7995b17884 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 17:58:49 +0100 Subject: [PATCH 36/50] simplify --- src/Executor/Promise/Adapter/SyncPromise.php | 81 +++++++------------- 1 file changed, 29 insertions(+), 52 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 3ca924079..39d52eb36 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -42,17 +42,6 @@ class SyncPromise */ protected $executor; - /** - * Callback handlers for child promises (set when parent resolves). - * - * @var array{ - * (callable(mixed): mixed)|null, - * (callable(\Throwable): mixed)|null, - * mixed - * }|null - */ - protected ?array $pendingCallback = null; - /** * Promises created in `then` method awaiting resolution. * @@ -200,16 +189,6 @@ public function then(?callable $onFulfilled = null, ?callable $onRejected = null return $child; } - /** - * @param callable(\Throwable): mixed $onRejected - * - * @throws InvariantViolation - */ - public function catch(callable $onRejected): self - { - return $this->then(null, $onRejected); - } - /** @throws InvariantViolation */ private function enqueueWaitingPromises(): void { @@ -219,44 +198,42 @@ private function enqueueWaitingPromises(): void $result = $this->result; - foreach ($this->waiting as [$childPromise, $onFulfilled, $onRejected]) { - // Store callback data on child promise to allow garbage collection after execution - $childPromise->pendingCallback = [$onFulfilled, $onRejected, $result]; + foreach ($this->waiting as $entry) { + SyncPromiseQueue::enqueue(static function () use ($entry, $result): void { + [$child, $onFulfilled, $onRejected] = $entry; - // Only capture child promise reference - SyncPromiseQueue::enqueue(static function () use ($childPromise): void { - $childPromise->runPendingCallback(); + try { + if ($result instanceof \Throwable) { + if ($onRejected === null) { + $child->reject($result); + } else { + $child->resolve($onRejected($result)); + } + } else { + $child->resolve( + $onFulfilled === null + ? $result + : $onFulfilled($result) + ); + } + } catch (\Throwable $e) { + $child->reject($e); + } }); } $this->waiting = []; } - /** Execute the pending callback and clear it for garbage collection. */ - protected function runPendingCallback(): void + /** + * @param callable(\Throwable): mixed $onRejected + * + * @throws InvariantViolation + * + * @deprecated TODO remove in next major version, use ->then(null, $onRejected) instead + */ + public function catch(callable $onRejected): self { - $callback = $this->pendingCallback; - $this->pendingCallback = null; // Clear for garbage collection - - [$onFulfilled, $onRejected, $parentResult] = $callback; - - // Determine parent state from result type - $parentRejected = $parentResult instanceof \Throwable; - - try { - if (! $parentRejected) { - $this->resolve( - $onFulfilled === null ? $parentResult : $onFulfilled($parentResult) - ); - } else { - if ($onRejected === null) { - $this->reject($parentResult); - } else { - $this->resolve($onRejected($parentResult)); - } - } - } catch (\Throwable $e) { - $this->reject($e); - } + return $this->then(null, $onRejected); } } From 0530d67076ef44a8638333cf5a150486735ffa72 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Tue, 16 Dec 2025 18:05:52 +0100 Subject: [PATCH 37/50] unalias $this --- src/Executor/Promise/Adapter/SyncPromise.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 39d52eb36..25618ab17 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -74,10 +74,8 @@ public function __construct(?callable $executor = null) } $this->executor = $executor; - $promise = $this; - // Lightweight closure - only captures promise reference - SyncPromiseQueue::enqueue(static function () use ($promise): void { - $promise->runExecutor(); + SyncPromiseQueue::enqueue(function (): void { + $this->runExecutor(); }); } From d9718dc66f69ec741ffb31ab8f026cb011aaee3e Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 08:28:59 +0100 Subject: [PATCH 38/50] inline --- src/Executor/Promise/Adapter/SyncPromise.php | 22 +++++++------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 25618ab17..bd365985d 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -75,23 +75,17 @@ public function __construct(?callable $executor = null) $this->executor = $executor; SyncPromiseQueue::enqueue(function (): void { - $this->runExecutor(); + $executor = $this->executor; + $this->executor = null; // Clear for garbage collection + + try { + $this->resolve($executor()); + } catch (\Throwable $e) { + $this->reject($e); + } }); } - /** Execute the deferred callback and clear it for garbage collection. */ - protected function runExecutor(): void - { - $executor = $this->executor; - $this->executor = null; // Clear for garbage collection - - try { - $this->resolve($executor()); - } catch (\Throwable $e) { - $this->reject($e); - } - } - /** * @param mixed $value * From 9b70533479b365f17bc6d15e48eb2cae9422b44e Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 08:42:52 +0100 Subject: [PATCH 39/50] document Deferred --- generate-class-reference.php | 1 + src/Deferred.php | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/generate-class-reference.php b/generate-class-reference.php index dff2bace2..8c51d1ebc 100644 --- a/generate-class-reference.php +++ b/generate-class-reference.php @@ -23,6 +23,7 @@ GraphQL\Executor\ScopedContext::class => [], GraphQL\Executor\ExecutionResult::class => [], GraphQL\Executor\Promise\PromiseAdapter::class => [], + GraphQL\Deferred::class => [], GraphQL\Validator\DocumentValidator::class => [], GraphQL\Error\Error::class => ['constants' => true], GraphQL\Error\Warning::class => ['constants' => true], diff --git a/src/Deferred.php b/src/Deferred.php index c19afcada..5642ee008 100644 --- a/src/Deferred.php +++ b/src/Deferred.php @@ -16,4 +16,16 @@ public static function create(callable $executor): self { return new self($executor); } + + /** + * Create a new Deferred promise and enqueue its execution. + * + * @api + * + * @param Executor $executor + */ + public function __construct(callable $executor) + { + parent::__construct($executor); + } } From 6c76d05df159cc5207c92ee0b7d2b6c37ad7b5e4 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 09:41:03 +0100 Subject: [PATCH 40/50] Move $executor to Deferred --- src/Deferred.php | 37 ++++++++++++++++---- src/Executor/Promise/Adapter/SyncPromise.php | 27 -------------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/src/Deferred.php b/src/Deferred.php index 5642ee008..76f04f5d4 100644 --- a/src/Deferred.php +++ b/src/Deferred.php @@ -3,6 +3,7 @@ namespace GraphQL; use GraphQL\Executor\Promise\Adapter\SyncPromise; +use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; /** * User-facing promise class for deferred field resolution. @@ -11,11 +12,12 @@ */ class Deferred extends SyncPromise { - /** @param Executor $executor */ - public static function create(callable $executor): self - { - return new self($executor); - } + /** + * Executor for deferred promises. + * + * @var callable(): mixed + */ + protected $executor; /** * Create a new Deferred promise and enqueue its execution. @@ -26,6 +28,29 @@ public static function create(callable $executor): self */ public function __construct(callable $executor) { - parent::__construct($executor); + $this->executor = $executor; + + SyncPromiseQueue::enqueue(function (): void { + $executor = $this->executor; + unset($this->executor); + + try { + $this->resolve($executor()); + } catch (\Throwable $e) { + $this->reject($e); + } + }); + } + + /** + * Alias for __construct. + * + * @param Executor $executor + * + * @deprecated TODO remove in next major version, use new Deferred() instead + */ + public static function create(callable $executor): self + { + return new self($executor); } } diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index bd365985d..9315a8377 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -35,13 +35,6 @@ class SyncPromise */ public $result; - /** - * Executor for deferred promises. - * - * @var (callable(): mixed)|null - */ - protected $executor; - /** * Promises created in `then` method awaiting resolution. * @@ -66,26 +59,6 @@ public static function runQueue(): void SyncPromiseQueue::run(); } - /** @param Executor|null $executor */ - public function __construct(?callable $executor = null) - { - if ($executor === null) { - return; - } - - $this->executor = $executor; - SyncPromiseQueue::enqueue(function (): void { - $executor = $this->executor; - $this->executor = null; // Clear for garbage collection - - try { - $this->resolve($executor()); - } catch (\Throwable $e) { - $this->reject($e); - } - }); - } - /** * @param mixed $value * From 2b48656472bab24223a775b949f45095efb483b3 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 09:49:04 +0100 Subject: [PATCH 41/50] clean up message --- src/Executor/Promise/Promise.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Executor/Promise/Promise.php b/src/Executor/Promise/Promise.php index c50886af2..28db6fbaa 100644 --- a/src/Executor/Promise/Promise.php +++ b/src/Executor/Promise/Promise.php @@ -25,7 +25,8 @@ class Promise public function __construct($adoptedPromise, PromiseAdapter $adapter) { if ($adoptedPromise instanceof self) { - throw new InvariantViolation('Expecting promise from adapted system, got ' . self::class); + $selfClass = self::class; + throw new InvariantViolation("Expected promise from adapted system, got {$selfClass}."); } $this->adoptedPromise = $adoptedPromise; From c2714307dce78d68626a354065ada9d7deeaee36 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 10:09:32 +0100 Subject: [PATCH 42/50] docs --- docs/class-reference.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/class-reference.md b/docs/class-reference.md index ca04e7f7c..3da1d4bbd 100644 --- a/docs/class-reference.md +++ b/docs/class-reference.md @@ -1778,6 +1778,25 @@ function createRejected(Throwable $reason): GraphQL\Executor\Promise\Promise function all(iterable $promisesOrValues): GraphQL\Executor\Promise\Promise ``` +## GraphQL\Deferred + +User-facing promise class for deferred field resolution. + +@phpstan-type Executor callable(): mixed + +### GraphQL\Deferred Methods + +```php +/** + * Create a new Deferred promise and enqueue its execution. + * + * @api + * + * @param Executor $executor + */ +function __construct(callable $executor) +``` + ## GraphQL\Validator\DocumentValidator Implements the "Validation" section of the spec. From eff1d5c03dab29cee89e058dc46313886fb81f27 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 10:09:52 +0100 Subject: [PATCH 43/50] improve adoptedPromise usage naming --- .../Promise/Adapter/AmpPromiseAdapter.php | 7 ++-- .../Promise/Adapter/ReactPromiseAdapter.php | 22 +++++------ .../Promise/Adapter/SyncPromiseAdapter.php | 39 ++++++++++--------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/Executor/Promise/Adapter/AmpPromiseAdapter.php b/src/Executor/Promise/Adapter/AmpPromiseAdapter.php index 735a37bdc..25e9fc84b 100644 --- a/src/Executor/Promise/Adapter/AmpPromiseAdapter.php +++ b/src/Executor/Promise/Adapter/AmpPromiseAdapter.php @@ -41,10 +41,9 @@ public function then(Promise $promise, ?callable $onFulfilled = null, ?callable } }; - $adoptedPromise = $promise->adoptedPromise; - assert($adoptedPromise instanceof AmpPromise); - - $adoptedPromise->onResolve($onResolve); + $ampPromise = $promise->adoptedPromise; + assert($ampPromise instanceof AmpPromise); + $ampPromise->onResolve($onResolve); return new Promise($deferred->promise(), $this); } diff --git a/src/Executor/Promise/Adapter/ReactPromiseAdapter.php b/src/Executor/Promise/Adapter/ReactPromiseAdapter.php index e38027001..7eb153c13 100644 --- a/src/Executor/Promise/Adapter/ReactPromiseAdapter.php +++ b/src/Executor/Promise/Adapter/ReactPromiseAdapter.php @@ -28,34 +28,34 @@ public function convertThenable($thenable): Promise /** @throws InvariantViolation */ public function then(Promise $promise, ?callable $onFulfilled = null, ?callable $onRejected = null): Promise { - $adoptedPromise = $promise->adoptedPromise; - assert($adoptedPromise instanceof ReactPromiseInterface); + $reactPromise = $promise->adoptedPromise; + assert($reactPromise instanceof ReactPromiseInterface); - return new Promise($adoptedPromise->then($onFulfilled, $onRejected), $this); + return new Promise($reactPromise->then($onFulfilled, $onRejected), $this); } /** @throws InvariantViolation */ public function create(callable $resolver): Promise { - $promise = new ReactPromise($resolver); + $reactPromise = new ReactPromise($resolver); - return new Promise($promise, $this); + return new Promise($reactPromise, $this); } /** @throws InvariantViolation */ public function createFulfilled($value = null): Promise { - $promise = resolve($value); + $reactPromise = resolve($value); - return new Promise($promise, $this); + return new Promise($reactPromise, $this); } /** @throws InvariantViolation */ public function createRejected(\Throwable $reason): Promise { - $promise = reject($reason); + $reactPromise = reject($reason); - return new Promise($promise, $this); + return new Promise($reactPromise, $this); } /** @throws InvariantViolation */ @@ -70,11 +70,11 @@ public function all(iterable $promisesOrValues): Promise $promisesOrValuesArray = is_array($promisesOrValues) ? $promisesOrValues : iterator_to_array($promisesOrValues); - $promise = all($promisesOrValuesArray)->then(static fn ($values): array => array_map( + $reactPromise = all($promisesOrValuesArray)->then(static fn (array $values): array => array_map( static fn ($key) => $values[$key], array_keys($promisesOrValuesArray), )); - return new Promise($promise, $this); + return new Promise($reactPromise, $this); } } diff --git a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php index d0f90103d..d59151c8c 100644 --- a/src/Executor/Promise/Adapter/SyncPromiseAdapter.php +++ b/src/Executor/Promise/Adapter/SyncPromiseAdapter.php @@ -23,10 +23,10 @@ public function isThenable($value): bool public function convertThenable($thenable): Promise { if (! $thenable instanceof SyncPromise) { - // End-users should always use Deferred (and don't use SyncPromise directly) - $deferred = Deferred::class; + // End-users should always use Deferred, not SyncPromise directly + $deferredClass = Deferred::class; $safeThenable = Utils::printSafe($thenable); - throw new InvariantViolation("Expected instance of {$deferred}, got {$safeThenable}"); + throw new InvariantViolation("Expected instance of {$deferredClass}, got {$safeThenable}."); } return new Promise($thenable, $this); @@ -35,10 +35,10 @@ public function convertThenable($thenable): Promise /** @throws InvariantViolation */ public function then(Promise $promise, ?callable $onFulfilled = null, ?callable $onRejected = null): Promise { - $adoptedPromise = $promise->adoptedPromise; - assert($adoptedPromise instanceof SyncPromise); + $syncPromise = $promise->adoptedPromise; + assert($syncPromise instanceof SyncPromise); - return new Promise($adoptedPromise->then($onFulfilled, $onRejected), $this); + return new Promise($syncPromise->then($onFulfilled, $onRejected), $this); } /** @@ -47,18 +47,18 @@ public function then(Promise $promise, ?callable $onFulfilled = null, ?callable */ public function create(callable $resolver): Promise { - $promise = new SyncPromise(); + $syncPromise = new SyncPromise(); try { $resolver( - [$promise, 'resolve'], - [$promise, 'reject'] + [$syncPromise, 'resolve'], + [$syncPromise, 'reject'] ); } catch (\Throwable $e) { - $promise->reject($e); + $syncPromise->reject($e); } - return new Promise($promise, $this); + return new Promise($syncPromise, $this); } /** @@ -67,9 +67,9 @@ public function create(callable $resolver): Promise */ public function createFulfilled($value = null): Promise { - $promise = new SyncPromise(); + $syncPromise = new SyncPromise(); - return new Promise($promise->resolve($value), $this); + return new Promise($syncPromise->resolve($value), $this); } /** @@ -78,9 +78,9 @@ public function createFulfilled($value = null): Promise */ public function createRejected(\Throwable $reason): Promise { - $promise = new SyncPromise(); + $syncPromise = new SyncPromise(); - return new Promise($promise->reject($reason), $this); + return new Promise($syncPromise->reject($reason), $this); } /** @@ -114,10 +114,11 @@ static function ($value) use (&$result, $index, &$count, &$resolveAllWhenFinishe }, [$all, 'reject'] ); - } else { - $result[$index] = $promiseOrValue; - ++$count; + continue; } + + $result[$index] = $promiseOrValue; + ++$count; } $resolveAllWhenFinished(); @@ -152,7 +153,7 @@ public function wait(Promise $promise) throw $syncPromise->result; } - throw new InvariantViolation('Could not resolve promise'); + throw new InvariantViolation('Could not resolve promise.'); } /** Execute just before starting to run promise completion. */ From b941c37d52edfe1d38c4fbde19590b4302cd0b62 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 10:19:15 +0100 Subject: [PATCH 44/50] int state --- src/Executor/Promise/Adapter/SyncPromise.php | 33 ++++---- .../Promise/SyncPromiseAdapterTest.php | 2 +- tests/Executor/Promise/SyncPromiseTest.php | 82 ++++++++----------- 3 files changed, 55 insertions(+), 62 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 9315a8377..baa3800b0 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -17,16 +17,16 @@ */ class SyncPromise { - public const PENDING = 'pending'; - public const FULFILLED = 'fulfilled'; - public const REJECTED = 'rejected'; + public const PENDING = 0; + public const FULFILLED = 1; + public const REJECTED = 2; /** - * Current promise state: PENDING, FULFILLED, or REJECTED. + * Current promise state. * - * @var 'pending'|'fulfilled'|'rejected' + * @var 0|1|2 */ - public string $state = self::PENDING; + public int $state = self::PENDING; /** * Resolved value or rejection reason. @@ -69,7 +69,7 @@ public function resolve($value): self switch ($this->state) { case self::PENDING: if ($value === $this) { - throw new \Exception('Cannot resolve promise with self'); + throw new \Exception('Cannot resolve promise with self.'); } if (is_object($value) && method_exists($value, 'then')) { @@ -91,12 +91,12 @@ function (\Throwable $reason): void { break; case self::FULFILLED: if ($this->result !== $value) { - throw new \Exception('Cannot change value of fulfilled promise'); + throw new \Exception('Cannot change value of fulfilled promise.'); } break; case self::REJECTED: - throw new \Exception('Cannot resolve rejected promise'); + throw new \Exception('Cannot resolve rejected promise.'); } return $this; @@ -117,12 +117,12 @@ public function reject(\Throwable $reason): self break; case self::REJECTED: if ($reason !== $this->result) { - throw new \Exception('Cannot change rejection reason'); + throw new \Exception('Cannot change rejection reason.'); } break; case self::FULFILLED: - throw new \Exception('Cannot reject fulfilled promise'); + throw new \Exception('Cannot reject fulfilled promise.'); } return $this; @@ -136,15 +136,20 @@ public function reject(\Throwable $reason): self */ public function then(?callable $onFulfilled = null, ?callable $onRejected = null): self { - if ($this->state === self::REJECTED && $onRejected === null) { + if ($this->state === self::REJECTED + && $onRejected === null + ) { return $this; } - if ($this->state === self::FULFILLED && $onFulfilled === null) { + if ($this->state === self::FULFILLED + && $onFulfilled === null + ) { return $this; } $child = new self(); + $this->waiting[] = [$child, $onFulfilled, $onRejected]; if ($this->state !== self::PENDING) { @@ -158,7 +163,7 @@ public function then(?callable $onFulfilled = null, ?callable $onRejected = null private function enqueueWaitingPromises(): void { if ($this->state === self::PENDING) { - throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending'); + throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending.'); } $result = $this->result; diff --git a/tests/Executor/Promise/SyncPromiseAdapterTest.php b/tests/Executor/Promise/SyncPromiseAdapterTest.php index 59afb577c..4c5465f9d 100644 --- a/tests/Executor/Promise/SyncPromiseAdapterTest.php +++ b/tests/Executor/Promise/SyncPromiseAdapterTest.php @@ -68,7 +68,7 @@ public function testCreatePromise(): void } /** @param mixed $expectedNextValue */ - private static function assertValidPromise(Promise $promise, ?string $expectedNextReason, $expectedNextValue, string $expectedNextState): void + private static function assertValidPromise(Promise $promise, ?string $expectedNextReason, $expectedNextValue, int $expectedNextState): void { self::assertInstanceOf(SyncPromise::class, $promise->adoptedPromise); diff --git a/tests/Executor/Promise/SyncPromiseTest.php b/tests/Executor/Promise/SyncPromiseTest.php index 64b8bf81c..7189ad4c4 100644 --- a/tests/Executor/Promise/SyncPromiseTest.php +++ b/tests/Executor/Promise/SyncPromiseTest.php @@ -15,7 +15,7 @@ final class SyncPromiseTest extends TestCaseBase * ?callable, * ?string, * ?string, - * string, + * int, * }> */ public static function fulfilledPromiseResolveData(): iterable @@ -24,27 +24,22 @@ public static function fulfilledPromiseResolveData(): iterable $onFulfilledReturnsSameValue = static fn ($value) => $value; - $onFulfilledReturnsOtherValue = static fn ($value): string => 'other-' . $value; + $onFulfilledReturnsOtherValue = static fn ($value): string => "other-{$value}"; $onFulfilledThrows = static function (): void { - throw new \Exception('onFulfilled throws this!'); + throw new \Exception('onFulfilled throws this.'); }; yield ['test-value', null, 'test-value', null, SyncPromise::FULFILLED]; yield [uniqid(), $onFulfilledReturnsNull, null, null, SyncPromise::FULFILLED]; yield ['test-value', $onFulfilledReturnsSameValue, 'test-value', null, SyncPromise::FULFILLED]; yield ['test-value-2', $onFulfilledReturnsOtherValue, 'other-test-value-2', null, SyncPromise::FULFILLED]; - yield ['test-value-3', $onFulfilledThrows, null, 'onFulfilled throws this!', SyncPromise::REJECTED]; + yield ['test-value-3', $onFulfilledThrows, null, 'onFulfilled throws this.', SyncPromise::REJECTED]; } /** @dataProvider fulfilledPromiseResolveData */ - public function testFulfilledPromiseCannotChangeValue( - string $resolvedValue, - ?callable $onFulfilled, - ?string $expectedNextValue, - ?string $expectedNextReason, - ?string $expectedNextState - ): void { + public function testFulfilledPromiseCannotChangeValue(string $resolvedValue): void + { $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -52,18 +47,13 @@ public function testFulfilledPromiseCannotChangeValue( self::assertSame(SyncPromise::FULFILLED, $promise->state); $this->expectException(\Throwable::class); - $this->expectExceptionMessage('Cannot change value of fulfilled promise'); - $promise->resolve($resolvedValue . '-other-value'); + $this->expectExceptionMessage('Cannot change value of fulfilled promise.'); + $promise->resolve("{$resolvedValue}-other-value"); } /** @dataProvider fulfilledPromiseResolveData */ - public function testFulfilledPromiseCannotBeRejected( - string $resolvedValue, - ?callable $onFulfilled, - ?string $expectedNextValue, - ?string $expectedNextReason, - ?string $expectedNextState - ): void { + public function testFulfilledPromiseCannotBeRejected(string $resolvedValue): void + { $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -71,7 +61,7 @@ public function testFulfilledPromiseCannotBeRejected( self::assertSame(SyncPromise::FULFILLED, $promise->state); $this->expectException(\Throwable::class); - $this->expectExceptionMessage('Cannot reject fulfilled promise'); + $this->expectExceptionMessage('Cannot reject fulfilled promise.'); $promise->reject(new \Exception('anything')); } @@ -85,7 +75,7 @@ public function testFulfilledPromise( ?callable $onFulfilled, $expectedNextValue, ?string $expectedNextReason, - ?string $expectedNextState + ?int $expectedNextState ): void { $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -141,7 +131,7 @@ private static function assertValidPromise( SyncPromise $promise, $expectedNextValue, ?string $expectedNextReason, - ?string $expectedNextState + ?int $expectedNextState ): void { $actualNextValue = null; $actualNextReason = null; @@ -177,7 +167,15 @@ static function (\Throwable $reason) use (&$actualNextReason, &$onRejectedCalled self::assertEquals($expectedNextState, $promise->state); } - /** @return iterable */ + /** + * @return iterable + */ public static function rejectedPromiseData(): iterable { $onRejectedReturnsNull = static fn () => null; @@ -201,13 +199,8 @@ public static function rejectedPromiseData(): iterable } /** @dataProvider rejectedPromiseData */ - public function testRejectedPromiseCannotChangeReason( - \Throwable $rejectedReason, - ?callable $onRejected, - ?string $expectedNextValue, - ?string $expectedNextReason, - string $expectedNextState - ): void { + public function testRejectedPromiseCannotChangeReason(\Throwable $rejectedReason): void + { $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -215,18 +208,13 @@ public function testRejectedPromiseCannotChangeReason( self::assertSame(SyncPromise::REJECTED, $promise->state); $this->expectException(\Throwable::class); - $this->expectExceptionMessage('Cannot change rejection reason'); + $this->expectExceptionMessage('Cannot change rejection reason.'); $promise->reject(new \Exception('other-reason')); } /** @dataProvider rejectedPromiseData */ - public function testRejectedPromiseCannotBeResolved( - \Throwable $rejectedReason, - ?callable $onRejected, - ?string $expectedNextValue, - ?string $expectedNextReason, - string $expectedNextState - ): void { + public function testRejectedPromiseCannotBeResolved(\Throwable $rejectedReason): void + { $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -234,7 +222,7 @@ public function testRejectedPromiseCannotBeResolved( self::assertSame(SyncPromise::REJECTED, $promise->state); $this->expectException(\Throwable::class); - $this->expectExceptionMessage('Cannot resolve rejected promise'); + $this->expectExceptionMessage('Cannot resolve rejected promise.'); $promise->resolve('anything'); } @@ -244,7 +232,7 @@ public function testRejectedPromise( ?callable $onRejected, ?string $expectedNextValue, ?string $expectedNextReason, - string $expectedNextState + int $expectedNextState ): void { $promise = new SyncPromise(); self::assertSame(SyncPromise::PENDING, $promise->state); @@ -256,14 +244,14 @@ public function testRejectedPromise( $promise->reject(new \Exception('other-reason')); self::fail('Expected exception not thrown'); } catch (\Throwable $e) { - self::assertSame('Cannot change rejection reason', $e->getMessage()); + self::assertSame('Cannot change rejection reason.', $e->getMessage()); } try { $promise->resolve('anything'); self::fail('Expected exception not thrown'); } catch (\Throwable $e) { - self::assertSame('Cannot resolve rejected promise', $e->getMessage()); + self::assertSame('Cannot resolve rejected promise.', $e->getMessage()); } $nextPromise = $promise->then( @@ -312,7 +300,7 @@ public function testPendingPromise(): void $promise->resolve($promise); self::fail('Expected exception not thrown'); } catch (\Throwable $e) { - self::assertSame('Cannot resolve promise with self', $e->getMessage()); + self::assertSame('Cannot resolve promise with self.', $e->getMessage()); self::assertSame(SyncPromise::PENDING, $promise->state); } @@ -372,15 +360,15 @@ public function testPendingPromiseThen(): void self::assertSame(SyncPromise::PENDING, $promise->state); self::assertSame(SyncPromise::PENDING, $nextPromise->state); - // Make sure that it queues derivative promises until resolution: + // Make sure that it queues derivative promises until resolution $onFulfilledCount = 0; - $onRejectedCount = 0; - $onFulfilled = static function ($value) use (&$onFulfilledCount): int { + $onFulfilled = static function () use (&$onFulfilledCount): int { ++$onFulfilledCount; return $onFulfilledCount; }; + $onRejectedCount = 0; $onRejected = static function ($reason) use (&$onRejectedCount): void { ++$onRejectedCount; From 3e77e0b608ed3b09c704d147a2f386ef34c383cd Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Wed, 17 Dec 2025 15:07:33 +0100 Subject: [PATCH 45/50] single batch --- src/Executor/Promise/Adapter/SyncPromise.php | 19 +++++++++++-------- tests/Executor/Promise/SyncPromiseTest.php | 2 +- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index baa3800b0..0fdff21f2 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -166,12 +166,17 @@ private function enqueueWaitingPromises(): void throw new InvariantViolation('Cannot enqueue derived promises when parent is still pending.'); } - $result = $this->result; + $waiting = $this->waiting; + if ($waiting === []) { + return; + } + + $this->waiting = []; - foreach ($this->waiting as $entry) { - SyncPromiseQueue::enqueue(static function () use ($entry, $result): void { - [$child, $onFulfilled, $onRejected] = $entry; + $result = $this->result; + SyncPromiseQueue::enqueue(static function () use ($waiting, $result): void { + foreach ($waiting as [$child, $onFulfilled, $onRejected]) { try { if ($result instanceof \Throwable) { if ($onRejected === null) { @@ -189,10 +194,8 @@ private function enqueueWaitingPromises(): void } catch (\Throwable $e) { $child->reject($e); } - }); - } - - $this->waiting = []; + } + }); } /** diff --git a/tests/Executor/Promise/SyncPromiseTest.php b/tests/Executor/Promise/SyncPromiseTest.php index 7189ad4c4..48872b1c1 100644 --- a/tests/Executor/Promise/SyncPromiseTest.php +++ b/tests/Executor/Promise/SyncPromiseTest.php @@ -384,7 +384,7 @@ public function testPendingPromiseThen(): void self::assertSame(0, $onRejectedCount); $promise->resolve(1); - self::assertSame(4, SyncPromiseQueue::count()); + self::assertSame(1, SyncPromiseQueue::count()); self::assertSame(0, $onFulfilledCount); // @phpstan-ignore-line side-effects self::assertSame(0, $onRejectedCount); // @phpstan-ignore-line side-effects self::assertSame(SyncPromise::PENDING, $nextPromise->state); From f2e5221505a26d6d701cbfb93215e92d668000fe Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 18 Dec 2025 06:57:10 +0100 Subject: [PATCH 46/50] phpstan-ignore --- src/Deferred.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Deferred.php b/src/Deferred.php index 76f04f5d4..51746747e 100644 --- a/src/Deferred.php +++ b/src/Deferred.php @@ -32,7 +32,7 @@ public function __construct(callable $executor) SyncPromiseQueue::enqueue(function (): void { $executor = $this->executor; - unset($this->executor); + unset($this->executor); // @phpstan-ignore-line no subclasses are expected and not other uses of this property exist try { $this->resolve($executor()); From 6aa7d57159014dd9bd99a3d2cc48721b7f5a1e32 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 18 Dec 2025 07:13:49 +0100 Subject: [PATCH 47/50] nullable --- src/Deferred.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Deferred.php b/src/Deferred.php index 51746747e..22d410c28 100644 --- a/src/Deferred.php +++ b/src/Deferred.php @@ -15,7 +15,7 @@ class Deferred extends SyncPromise /** * Executor for deferred promises. * - * @var callable(): mixed + * @var (callable(): mixed)|null */ protected $executor; @@ -32,7 +32,8 @@ public function __construct(callable $executor) SyncPromiseQueue::enqueue(function (): void { $executor = $this->executor; - unset($this->executor); // @phpstan-ignore-line no subclasses are expected and not other uses of this property exist + assert($executor !== null, 'Always set in constructor, this callback runs only once.'); + $this->executor = null; try { $this->resolve($executor()); From 09518ec0b5b28d4382ac9dcb9ceb84c4ea6360b3 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 18 Dec 2025 08:31:31 +0100 Subject: [PATCH 48/50] remove workaround --- src/Executor/Promise/Adapter/SyncPromise.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 0fdff21f2..2c4a597da 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -49,16 +49,6 @@ class SyncPromise */ protected array $waiting = []; - /** - * Process all queued promises until the queue is empty. - * - * TODO remove before merging, not part of public API - */ - public static function runQueue(): void - { - SyncPromiseQueue::run(); - } - /** * @param mixed $value * From 24db74bf8ac5a0b0fa4e943b5d39747f739b1073 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 18 Dec 2025 08:35:09 +0100 Subject: [PATCH 49/50] clean up --- benchmarks/DeferredBench.php | 16 ++++++++-------- src/Executor/Promise/Adapter/SyncPromise.php | 12 ------------ 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/benchmarks/DeferredBench.php b/benchmarks/DeferredBench.php index f631c5e67..c63ed6915 100644 --- a/benchmarks/DeferredBench.php +++ b/benchmarks/DeferredBench.php @@ -3,7 +3,7 @@ namespace GraphQL\Benchmarks; use GraphQL\Deferred; -use GraphQL\Executor\Promise\Adapter\SyncPromise; +use GraphQL\Executor\Promise\Adapter\SyncPromiseQueue; /** * @OutputTimeUnit("microseconds", precision=3) @@ -19,13 +19,13 @@ class DeferredBench public function benchSingleDeferred(): void { new Deferred(static fn () => 'value'); - SyncPromise::runQueue(); + SyncPromiseQueue::run(); } public function benchNestedDeferred(): void { new Deferred(static fn () => new Deferred(static fn () => null)); - SyncPromise::runQueue(); + SyncPromiseQueue::run(); } public function benchChain5(): void @@ -36,7 +36,7 @@ public function benchChain5(): void ->then(static fn ($v) => $v) ->then(static fn ($v) => $v) ->then(static fn ($v) => $v); - SyncPromise::runQueue(); + SyncPromiseQueue::run(); } public function benchChain100(): void @@ -46,7 +46,7 @@ public function benchChain100(): void for ($i = 0; $i < 100; ++$i) { $promise = $promise->then(static fn ($v) => $v); } - SyncPromise::runQueue(); + SyncPromiseQueue::run(); } public function benchManyDeferreds(): void @@ -55,7 +55,7 @@ public function benchManyDeferreds(): void for ($i = 0; $i < 1000; ++$i) { new Deferred($fn); } - SyncPromise::runQueue(); + SyncPromiseQueue::run(); } public function benchManyNestedDeferreds(): void @@ -63,7 +63,7 @@ public function benchManyNestedDeferreds(): void for ($i = 0; $i < 5000; ++$i) { new Deferred(static fn () => new Deferred(static fn () => null)); } - SyncPromise::runQueue(); + SyncPromiseQueue::run(); } public function bench1000Chains(): void @@ -75,6 +75,6 @@ public function bench1000Chains(): void ->then(static fn ($v) => $v) ->then(static fn ($v) => $v); } - SyncPromise::runQueue(); + SyncPromiseQueue::run(); } } diff --git a/src/Executor/Promise/Adapter/SyncPromise.php b/src/Executor/Promise/Adapter/SyncPromise.php index 2c4a597da..87cfb1b8e 100644 --- a/src/Executor/Promise/Adapter/SyncPromise.php +++ b/src/Executor/Promise/Adapter/SyncPromise.php @@ -187,16 +187,4 @@ private function enqueueWaitingPromises(): void } }); } - - /** - * @param callable(\Throwable): mixed $onRejected - * - * @throws InvariantViolation - * - * @deprecated TODO remove in next major version, use ->then(null, $onRejected) instead - */ - public function catch(callable $onRejected): self - { - return $this->then(null, $onRejected); - } } From 1e733627d6c75cb1cc9b8ca87bea4dc614c079ff Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 18 Dec 2025 17:26:26 +0100 Subject: [PATCH 50/50] prepare release --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ca52990c..0cdc31d7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,16 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +## v15.29.0 + ### Changed - Optimize `Deferred` execution https://github.com/webonyx/graphql-php/pull/1805 +### Deprecated + +- Deprecate `GraphQL\Deferred::create()` in favor of constructor https://github.com/webonyx/graphql-php/pull/1805 + ## v15.28.0 ### Changed