perf(amp): avoid fibers in future adapter#1954
Open
simPod wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the AMPHP v3 AmpFutureAdapter to compose and aggregate Future continuations without creating a new Fiber per continuation/aggregate, aiming to reduce allocations and event-loop overhead for wide async GraphQL result graphs.
Changes:
- Reworked
AmpFutureAdapter::then()to settle a singleDeferredFutureviaFuture::map()/catch()observation rather thanasync+await. - Reimplemented
AmpFutureAdapter::all()to resolve/reject by observing each inputFuture, preserving array keys and avoiding aggregateawait. - Added/expanded PHPUnit coverage for nested-future flattening, fulfillment callback exceptions, key preservation, aggregate rejection, and “stay rejected” behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/Executor/Promise/Adapter/AmpFutureAdapter.php |
Removes per-continuation fibers by observing futures with map()/catch() and settling a shared DeferredFuture. |
tests/Executor/Promise/AmpFutureAdapterTest.php |
Adds tests covering nested future unwrapping, rejection/exception behavior, key preservation, and aggregate failure semantics. |
CHANGELOG.md |
Documents the behavioral/performance fix under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
spawnia
reviewed
Jul 24, 2026
simPod
force-pushed
the
perf/avoid-fiber-per-amp-continuation
branch
from
July 25, 2026 07:31
67ab737 to
79f8304
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
AmpFutureAdaptercreated an AMPHP Fiber for every promise continuation and aggregate. Wide GraphQL result graphs therefore retained many Fiber states and event-loop callbacks even when the underlying Futures only represented batched work.Decision
Compose and aggregate Futures through
map()andcatch()callbacks, settling oneDeferredFuturewhile retaining Promise fulfillment, rejection, and nested-Future flattening semantics.Consequences
Benchmark
On the same wide GraphQL result graph with 838 parent objects and three Future-backed fields:
Relative to the existing AMPHP adapter, this reduces CPU time by about 19% and peak memory by about 46%. The remaining overhead compared with the synchronous adapter is about 13-15% CPU and 22% memory.
Disclosure
This change was identified while migrating a production GraphQL application that is not yet fully migrated to asynchronous resolving. It is a step toward that migration, rather than a claim of end-to-end asynchronous production use.