Skip to content

pure fusion in Dynamic#100

Merged
zyla merged 3 commits intomasterfrom
fusion
Aug 18, 2025
Merged

pure fusion in Dynamic#100
zyla merged 3 commits intomasterfrom
fusion

Conversation

@zyla
Copy link
Collaborator

@zyla zyla commented Oct 8, 2024

This PR modifies Dynamic by adding a special-case constructor for constants (DynPure). Constants are propagated, for example map on a constant also produces a constant.

This enables some optimizations:

  • reduces the number of nodes in the Incremental graph
  • reduces the number of subscriptions and unsubscriptions (subscribing to a DynPure is a no-op)

This can help especially for computations which are expressed as Dynamic but where the actual dynamism is limited.

@zyla zyla marked this pull request as ready for review August 16, 2025 07:44
@zyla zyla requested review from jborkowski and kozak August 16, 2025 07:50
pure = DynPure

dynToNode :: forall a. Dynamic a -> Node a
dynToNode (DynPure x) = unsafePerformEffect do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's interesting: Dynamic Apply instance doesn't need this, Dynamic Bind instance does. And mapAsync does. It seems this should be avoidable so we have even less nodes. Or it isn't?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting observation!

In principle we could specialize bind and mapAsync so that we don't need it. It's just that in these cases I think we don't gain much, and it would require substantial work.

Firsy, in both cases the Pure has no chance of propagating further.
mapAsync on a Pure still has to return a DynNode, to handle the loading state.
And for RHS of a bind case: if the LHS is still dynamic, we can't propagate anyway. And we already handle the case where the LHS is Pure.
So this would be a small local gain. Async is usually so heavy that the specialization wouldn't make sense. For bind I think we can still consider it.

Second, in the current shape rhe change doesn't need to touch the Incremental layer, which buys us a but of "obviously correct"ness.

Copy link
Contributor

@kozak kozak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG!

@zyla zyla merged commit 5e77336 into master Aug 18, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments