Skip to content

Add Future completion subscriptions - #469

Open
simPod wants to merge 3 commits into
amphp:3.xfrom
simPod:feat/future-subscribe
Open

Add Future completion subscriptions#469
simPod wants to merge 3 commits into
amphp:3.xfrom
simPod:feat/future-subscribe

Conversation

@simPod

@simPod simPod commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Context

Promise adapters may need to observe a Future completion without transforming it into another Future.

Decision

Add Future::subscribe() and Future::unsubscribe() as public facades for the existing one-shot completion callback mechanism.

Consequences

Adapters can observe values and errors without allocating derived Future chains. Callback subscriptions can be removed before a Future completes.

Implementation Example

GraphQL PHP PR #8 uses Future::subscribe() to replace completion observation through map()->catch()->ignore().

The chained approach creates two derived FutureState and Future pairs per observation. Direct subscription registers one callback on the original Future.

Benchmark

The GraphQL PHP implementation was profiled with 500 parent objects and six Future-backed fields per parent. Both warmed profiles executed the same query and 9 SQL requests. Assertions were enabled and AMP_DEBUG was disabled.

Observation strategy Wall time CPU time Peak memory
map()->catch()->ignore() 5.61 s 2.44 s 180 MB
Direct subscription 5.21 s 1.81 s 145 MB

Direct subscription reduced CPU time by approximately 26% and peak memory by approximately 19% in this profile.

View the Blackfire comparison.

The direct variant used the same underlying completion subscription mechanism exposed by this PR. These measurements demonstrate the effect in the GraphQL PHP adapter and are not a general performance guarantee.

@kelunik

kelunik commented Aug 18, 2026

Copy link
Copy Markdown
Member

Hey Simon, thank you for providing some numbers. However, the difference seems to be suspiciously high given the intended change. There's no change to fiber interactions with this PR, all it's saving is two FiberState allocations per adapter, no? Are you running with AMP_DEBUG and assertions enabled?

@simPod

simPod commented Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

You鈥檙e right. The previous benchmark conflated this change with an earlier GraphQL PHP optimization that removed Fiber-per-continuation handling. I misformulated what I did in AMP as I'm working on kinda larger fibers related change across multiple libs.

I replaced it with an isolated comparison of GraphQL PHP鈥檚 current map()->catch()->ignore() observation against direct subscription. This Amp PR does not change Fiber behavior.

The new benchmark uses 500 parent objects with six Future-backed fields per parent. Both warmed profiles execute the same query and 9 SQL requests. Assertions were enabled and AMP_DEBUG was disabled.

Mind that the benchmark is not of AMP but rather what it could do in consumer code - why I'd like to make the change and expose new public API.

Chained observation: 2.44 s CPU, 180 MB peak memory
Direct subscription: 1.81 s CPU, 145 MB peak memory
Comparison: app.blackfire.io/profiles/compare/f4fce594-cbb7-492d-b2ca-fb0e01b8d2e2/8d217f7a-3ff8-46db-a616-96e61b6d7120/graph

The saving is two derived FutureState and Future pairs per observation, plus the additional chained callback. The performance effect is realized in the GraphQL PHP adapter; this PR exposes the underlying subscription mechanism as a supported public API.

I also clarified that subscription callbacks are always queued on the event loop and added tests for callback exceptions and unsubscribe-after-completion behavior.

MR desc updated.

@kelunik

kelunik commented Aug 19, 2026

Copy link
Copy Markdown
Member

Could you make the Blackfire results public? I don't have access right now.

@simPod

simPod commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants