Skip to content

Fuse unionWith and checkBinRel in PlutusLedgerApi.V1.Data.Value#7799

Merged
Unisay merged 6 commits into
masterfrom
yura/issue-2243-fused-unionwith-impl
Jun 18, 2026
Merged

Fuse unionWith and checkBinRel in PlutusLedgerApi.V1.Data.Value#7799
Unisay merged 6 commits into
masterfrom
yura/issue-2243-fused-unionwith-impl

Conversation

@Unisay

@Unisay Unisay commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

PlutusLedgerApi.V1.Data.Value.unionWith now runs one outer Map.union and one outer Map.map that collapses the These shape inline per currency-symbol entry. The previous implementation built a Map CurrencySymbol (Map TokenName (These Integer Integer)) intermediate via a unionVal helper and then re-walked it to apply f: three outer passes for a single conceptual merge. checkBinRel is refactored along the same shape with Map.union + Map.all; the inner per-currency merge now sits inside the short-circuiting Map.all walk, so a failing pair skips the inner merges of every later currency (geq / leq / gt / lt inherit the early exit). unionVal and checkPred are dropped (both were module-internal, not exported). Same semantics, no public API change.

The Map.union lookup-and-merge core (O(L×R)) is untouched, so this is a constant-factor change: one of the two outer Map.map passes over the post-union map goes away, and a currency present on only one side has its token map mapped once instead of wrapped and then re-mapped.

Closes plutus-private#2243.

@github-actions

github-actions Bot commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Execution Budget Golden Diff

b1db04c (master) vs 4a69249

output

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/geq1.golden.eval

Metric Old New Δ%
CPU 333_462_100 331_010_215 -0.74%
Memory 963_345 952_905 -1.08%
Flat Size 942 938 -0.42%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/geq2.golden.eval

Metric Old New Δ%
CPU 350_706_346 340_731_162 -2.84%
Memory 1_027_715 978_731 -4.77%
Flat Size 993 989 -0.40%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/geq3.golden.eval

Metric Old New Δ%
CPU 363_986_374 351_760_500 -3.36%
Memory 1_070_929 1_010_925 -5.60%
Flat Size 993 989 -0.40%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/geq4.golden.eval

Metric Old New Δ%
CPU 327_778_382 177_245_560 -45.93%
Memory 924_936 523_041 -43.45%
Flat Size 949 945 -0.42%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/geq5.golden.eval

Metric Old New Δ%
CPU 345_381_135 342_977_250 -0.70%
Memory 995_200 985_060 -1.02%
Flat Size 949 945 -0.42%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/gt1.golden.eval

Metric Old New Δ%
CPU 384_062_505 381_466_620 -0.68%
Memory 1_141_260 1_129_920 -0.99%
Flat Size 1_316 1_291 -1.90%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/gt2.golden.eval

Metric Old New Δ%
CPU 351_074_346 340_955_162 -2.88%
Memory 1_030_015 980_131 -4.84%
Flat Size 1_367 1_342 -1.83%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/gt3.golden.eval

Metric Old New Δ%
CPU 415_308_479 403_162_605 -2.92%
Memory 1_252_809 1_193_305 -4.75%
Flat Size 1_367 1_342 -1.83%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/gt4.golden.eval

Metric Old New Δ%
CPU 328_146_382 177_469_560 -45.92%
Memory 927_236 524_441 -43.44%
Flat Size 1_323 1_298 -1.89%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.12/gt5.golden.eval

Metric Old New Δ%
CPU 369_587_763 367_039_878 -0.69%
Memory 1_084_024 1_072_984 -1.02%
Flat Size 1_323 1_298 -1.89%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/geq1.golden.eval

Metric Old New Δ%
CPU 333_462_100 331_010_215 -0.74%
Memory 963_345 952_905 -1.08%
Flat Size 942 938 -0.42%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/geq2.golden.eval

Metric Old New Δ%
CPU 350_706_346 340_731_162 -2.84%
Memory 1_027_715 978_731 -4.77%
Flat Size 993 989 -0.40%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/geq3.golden.eval

Metric Old New Δ%
CPU 363_986_374 351_760_500 -3.36%
Memory 1_070_929 1_010_925 -5.60%
Flat Size 993 989 -0.40%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/geq4.golden.eval

Metric Old New Δ%
CPU 327_778_382 177_245_560 -45.93%
Memory 924_936 523_041 -43.45%
Flat Size 949 945 -0.42%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/geq5.golden.eval

Metric Old New Δ%
CPU 345_381_135 342_977_250 -0.70%
Memory 995_200 985_060 -1.02%
Flat Size 949 945 -0.42%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/gt1.golden.eval

Metric Old New Δ%
CPU 384_062_505 381_466_620 -0.68%
Memory 1_141_260 1_129_920 -0.99%
Flat Size 1_316 1_291 -1.90%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/gt2.golden.eval

Metric Old New Δ%
CPU 351_074_346 340_955_162 -2.88%
Memory 1_030_015 980_131 -4.84%
Flat Size 1_367 1_342 -1.83%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/gt3.golden.eval

Metric Old New Δ%
CPU 415_308_479 403_162_605 -2.92%
Memory 1_252_809 1_193_305 -4.75%
Flat Size 1_367 1_342 -1.83%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/gt4.golden.eval

Metric Old New Δ%
CPU 328_146_382 177_469_560 -45.92%
Memory 927_236 524_441 -43.44%
Flat Size 1_323 1_298 -1.89%

plutus-tx-plugin/test-ledger-api/Spec/Data/Budget/9.6/gt5.golden.eval

Metric Old New Δ%
CPU 369_587_763 367_039_878 -0.69%
Memory 1_084_024 1_072_984 -1.02%
Flat Size 1_323 1_298 -1.89%

This comment will get updated when changes are made.

@Unisay Unisay self-assigned this May 28, 2026
@Unisay Unisay requested a review from Copilot May 28, 2026 12:43

This comment was marked as resolved.

Sign in to view

@Unisay Unisay marked this pull request as draft May 28, 2026 12:47
@Unisay Unisay mentioned this pull request May 28, 2026
Draft
@Unisay Unisay force-pushed the yura/issue-2243-fused-unionwith-impl branch 2 times, most recently from 5282416 to 178738a Compare June 1, 2026 14:13
@Unisay Unisay requested a review from Copilot June 1, 2026 15:04
@Unisay Unisay marked this pull request as ready for review June 1, 2026 15:04
@Unisay Unisay requested a review from a team June 1, 2026 15:05

This comment was marked as resolved.

Sign in to view

SeungheonOh
SeungheonOh approved these changes Jun 2, 2026
View reviewed changes

@SeungheonOh SeungheonOh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice optimization. maybe it could be faster if Map.all gets inlined as well but I'm not sure if that will be worth it

Unisay added 6 commits June 18, 2026 12:16
@Unisay
Fuse unionWith and checkBinRel in V1.Data.Value
cea3fe4 
Drop the internal unionVal and checkPred helpers; inline the merge logic
into unionWith and checkBinRel respectively. The previous chain built a
Map CurrencySymbol (Map TokenName (These Integer Integer)) intermediate
via unionVal, then re-walked it to apply f -- three outer passes for a
single conceptual merge.

The fused unionWith now runs Map.union once and a single outer Map.map,
collapsing the These shape inline per currency-symbol entry. The
Map TokenName (These Integer Integer) stage is gone; the outer Map.map
runs once instead of twice. checkBinRel is refactored along the same
shape with Map.union + Map.all, which gives geq / leq / gt / lt
short-circuit termination on the first failing pair.

Adds Spec.Data.Value.test_unionWith: a QuickCheck property that compiles
unionWith via TH, evaluates on the CEK machine, and compares against the
host-Haskell unionWith for the same inputs. Differential test against
the Plinth compiler: any divergence is a compilation bug, not a
semantics bug.

The Spec.Data.Budget gt / geq budget goldens are regenerated: short-circuit
checkBinRel reduces gt4 / geq4 by ~46% (the worst-case adverse input);
other shapes drop 0.7-3% from removing one outer pass over the These
intermediate. The remaining diff is the cost-model anchor; that
component is unrelated to this change.

Budget evidence (union matrix vs builtin, unsafeDataAsValue baseline)
lives on the companion experimental branch
yura/issue-2243-fused-unionwith-evidence, stacked on the sibling
valueOf-evidence branch.

For IntersectMBO/plutus-private#2243.
@Unisay
Refresh GHC 9.12 gt / geq budget goldens after checkBinRel rewrite
b7e3a5a 
The previous commit (642442a) regenerated the GHC 9.6 column only.
plutus-ledger-api-plugin-test is also buildable on GHC 9.12 per the
ghc-version-support common stanza, and Hydra runs it there; the
checkBinRel rewrite changes the compiled UPLC enough to move the
budgets in both columns. Same regen, run in nix develop .#ghc912.

For IntersectMBO/plutus-private#2243.
@Unisay
Make the unionWith property test differential against the builtin
dac80e8 
Compare the typed unionWith (+) against the builtin unionValue path on
CEK rather than against host-Haskell unionWith: a shared-source oracle
cannot catch a bug that lands the same way on both sides. Inputs are
restricted to the well-formed domain unsafeDataAsValue accepts, and
results are compared up to key order and zero-sum entries. Bindings use
plinthc instead of the compile splice.

Also rephrase Note [Single-pass unionWith] and the checkBinRel docstring
to describe the present structure without contrasting against history.

For IntersectMBO/plutus-private#2243.
@Unisay
Correct the unionWith fusion wording
f00a356 
The fusion is a constant-factor change, not the elimination of the These
intermediate it was described as. Map.union still produces a per-key These
and the inner These is still built transiently for shared keys; what the
fusion removes is one of the two outer Map.map passes over the post-union
map, plus the wrap-then-remap of single-side currencies. Adjust the
changelog and drop Note [Single-pass unionWith] accordingly; the merge
structure is plain from the code.

For IntersectMBO/plutus-private#2243.
@Unisay
Clarify checkBinRel short-circuit wording in changelog
a02cdbb
@Unisay
Trim changelog to the exported unionWith change
4a69249
@Unisay Unisay force-pushed the yura/issue-2243-fused-unionwith-impl branch from 178738a to 4a69249 Compare June 18, 2026 10:37
@Unisay

Unisay commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks. Leaving this out of the PR for now. Map.all is INLINEABLE, but PIR keeps it as one shared binding; forcing the inline at the four checkBinRel sites might shave CPU at the cost of flat size (which this PR currently reduces), so the net sign is unclear. It's also shared across the module, so changing its pragma isn't local. Better as a measured follow-up on the evidence branch. Can open a ticket if it's worth chasing.

@Unisay Unisay enabled auto-merge (squash) June 18, 2026 10:50
@Unisay Unisay merged commit c35657a into master Jun 18, 2026
9 of 10 checks passed
@Unisay Unisay deleted the yura/issue-2243-fused-unionwith-impl branch June 18, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@SeungheonOh SeungheonOh SeungheonOh approved these changes

Assignees

@Unisay Unisay

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Unisay @SeungheonOh