Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Builtins] Stop doing the worker-wrapper transformation for the denotations #4532

Merged
merged 1 commit into from
Apr 27, 2022
Merged

[Builtins] Stop doing the worker-wrapper transformation for the denotations #4532

merged 1 commit into from
Apr 27, 2022

Conversation

effectfully
Copy link
Contributor

@effectfully effectfully commented Apr 12, 2022 •
edited
Loading

It seems like the worker-wrapper transformation may do more harm than good for builtins.

@effectfully effectfully force-pushed the effectfully/builtins/stop-worker-wrapper branch from 1b7a7ad to eb6e443 Compare April 12, 2022 18:13
@effectfully
Copy link
Contributor Author

/benchmark plutus-benchmark:validation

@iohk-devops
Copy link

Comparing benchmark results of 'plutus-benchmark:validation' on 'b30008f5d' (base) and 'eb6e44396' (PR)

Results table
Script b30008f eb6e443 Change
auction_1-1 206.5 μs 206.2 μs -0.1%
auction_1-2 824.1 μs 825.8 μs +0.2%
auction_1-3 813.5 μs 816.1 μs +0.3%
auction_1-4 261.3 μs 262.1 μs +0.3%
auction_2-1 207.3 μs 206.9 μs -0.2%
auction_2-2 824.2 μs 824.5 μs +0.0%
auction_2-3 1.039 ms 1.044 ms +0.5%
auction_2-4 809.0 μs 810.3 μs +0.2%
auction_2-5 261.8 μs 261.6 μs -0.1%
crowdfunding-success-1 240.8 μs 241.2 μs +0.2%
crowdfunding-success-2 240.6 μs 240.9 μs +0.1%
crowdfunding-success-3 241.1 μs 241.8 μs +0.3%
currency-1 302.3 μs 302.5 μs +0.1%
escrow-redeem_1-1 430.0 μs 430.1 μs +0.0%
escrow-redeem_1-2 430.2 μs 430.1 μs -0.0%
escrow-redeem_2-1 505.2 μs 506.0 μs +0.2%
escrow-redeem_2-2 504.8 μs 505.8 μs +0.2%
escrow-redeem_2-3 504.4 μs 504.8 μs +0.1%
escrow-refund-1 182.1 μs 181.9 μs -0.1%
future-increase-margin-1 303.0 μs 301.8 μs -0.4%
future-increase-margin-2 683.1 μs 683.0 μs -0.0%
future-increase-margin-3 682.7 μs 683.6 μs +0.1%
future-increase-margin-4 640.7 μs 641.4 μs +0.1%
future-increase-margin-5 1.022 ms 1.021 ms -0.1%
future-pay-out-1 303.3 μs 304.3 μs +0.3%
future-pay-out-2 683.2 μs 685.2 μs +0.3%
future-pay-out-3 682.1 μs 683.9 μs +0.3%
future-pay-out-4 1.013 ms 1.016 ms +0.3%
future-settle-early-1 302.5 μs 302.2 μs -0.1%
future-settle-early-2 680.4 μs 681.7 μs +0.2%
future-settle-early-3 680.5 μs 681.2 μs +0.1%
future-settle-early-4 770.7 μs 771.3 μs +0.1%
game-sm-success_1-1 491.0 μs 490.1 μs -0.2%
game-sm-success_1-2 224.0 μs 224.6 μs +0.3%
game-sm-success_1-3 811.8 μs 810.5 μs -0.2%
game-sm-success_1-4 260.8 μs 260.4 μs -0.2%
game-sm-success_2-1 490.9 μs 490.7 μs -0.0%
game-sm-success_2-2 225.4 μs 225.7 μs +0.1%
game-sm-success_2-3 813.4 μs 814.1 μs +0.1%
game-sm-success_2-4 261.6 μs 261.4 μs -0.1%
game-sm-success_2-5 810.2 μs 812.0 μs +0.2%
game-sm-success_2-6 260.6 μs 261.3 μs +0.3%
multisig-sm-1 503.2 μs 504.7 μs +0.3%
multisig-sm-2 490.3 μs 491.8 μs +0.3%
multisig-sm-3 497.1 μs 497.7 μs +0.1%
multisig-sm-4 501.3 μs 503.9 μs +0.5%
multisig-sm-5 720.7 μs 724.7 μs +0.6%
multisig-sm-6 505.2 μs 505.7 μs +0.1%
multisig-sm-7 491.1 μs 492.3 μs +0.2%
multisig-sm-8 497.9 μs 496.4 μs -0.3%
multisig-sm-9 502.3 μs 503.0 μs +0.1%
multisig-sm-10 720.0 μs 723.8 μs +0.5%
ping-pong-1 411.8 μs 413.8 μs +0.5%
ping-pong-2 411.9 μs 414.1 μs +0.5%
ping-pong_2-1 242.2 μs 243.8 μs +0.7%
prism-1 186.9 μs 187.2 μs +0.2%
prism-2 528.0 μs 528.3 μs +0.1%
prism-3 443.2 μs 444.9 μs +0.4%
pubkey-1 157.6 μs 157.4 μs -0.1%
stablecoin_1-1 1.123 ms 1.123 ms 0.0%
stablecoin_1-2 220.0 μs 220.5 μs +0.2%
stablecoin_1-3 1.279 ms 1.282 ms +0.2%
stablecoin_1-4 233.4 μs 233.4 μs 0.0%
stablecoin_1-5 1.601 ms 1.601 ms 0.0%
stablecoin_1-6 287.8 μs 288.6 μs +0.3%
stablecoin_2-1 1.119 ms 1.125 ms +0.5%
stablecoin_2-2 219.3 μs 220.0 μs +0.3%
stablecoin_2-3 1.279 ms 1.280 ms +0.1%
stablecoin_2-4 233.4 μs 233.7 μs +0.1%
token-account-1 225.9 μs 225.1 μs -0.4%
token-account-2 399.6 μs 399.5 μs -0.0%
uniswap-1 509.1 μs 512.8 μs +0.7%
uniswap-2 265.2 μs 267.0 μs +0.7%
uniswap-3 2.057 ms 2.069 ms +0.6%
uniswap-4 385.6 μs 386.4 μs +0.2%
uniswap-5 1.414 ms 1.416 ms +0.1%
uniswap-6 369.7 μs 370.3 μs +0.2%
vesting-1 436.3 μs 435.7 μs -0.1%

@effectfully
Copy link
Contributor Author

^ noise. @michaelpj I'm still inclined to do this. Fewer indirections and "what's that for?" moments when inspecting Core. Don't merge though, I'll need to add a comment if we're doing this.

@effectfully effectfully requested a review from michaelpj April 12, 2022 23:34
michaelpj
michaelpj approved these changes Apr 13, 2022
View reviewed changes
Copy link
Contributor

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

Fine, you can just add a short note saying it makes things easier to read!

@effectfully effectfully mentioned this pull request Apr 14, 2022
Open
@effectfully effectfully force-pushed the effectfully/builtins/stop-worker-wrapper branch from eb6e443 to 1c0634c Compare April 14, 2022 11:55
@effectfully
Copy link
Contributor Author

/benchmark plutus-benchmark:validation

@iohk-devops
Copy link

Comparing benchmark results of 'plutus-benchmark:validation' on '74e9b54e0' (base) and '1c0634c99' (PR)

Results table
Script 74e9b54 1c0634c Change
auction_1-1 202.3 μs 202.2 μs -0.0%
auction_1-2 809.6 μs 813.0 μs +0.4%
auction_1-3 802.3 μs 804.9 μs +0.3%
auction_1-4 259.3 μs 258.1 μs -0.5%
auction_2-1 204.7 μs 204.3 μs -0.2%
auction_2-2 816.6 μs 812.6 μs -0.5%
auction_2-3 1.027 ms 1.027 ms 0.0%
auction_2-4 802.1 μs 798.9 μs -0.4%
auction_2-5 257.4 μs 257.6 μs +0.1%
crowdfunding-success-1 238.0 μs 237.6 μs -0.2%
crowdfunding-success-2 237.4 μs 237.3 μs -0.0%
crowdfunding-success-3 238.7 μs 237.2 μs -0.6%
currency-1 298.9 μs 299.3 μs +0.1%
escrow-redeem_1-1 425.9 μs 425.6 μs -0.1%
escrow-redeem_1-2 425.3 μs 427.1 μs +0.4%
escrow-redeem_2-1 501.3 μs 502.6 μs +0.3%
escrow-redeem_2-2 501.2 μs 502.8 μs +0.3%
escrow-redeem_2-3 502.8 μs 501.8 μs -0.2%
escrow-refund-1 180.9 μs 181.2 μs +0.2%
future-increase-margin-1 300.3 μs 300.0 μs -0.1%
future-increase-margin-2 677.9 μs 676.9 μs -0.1%
future-increase-margin-3 676.8 μs 677.3 μs +0.1%
future-increase-margin-4 634.3 μs 632.6 μs -0.3%
future-increase-margin-5 1.003 ms 1.002 ms -0.1%
future-pay-out-1 299.9 μs 299.0 μs -0.3%
future-pay-out-2 676.4 μs 676.9 μs +0.1%
future-pay-out-3 678.5 μs 674.4 μs -0.6%
future-pay-out-4 1.000 ms 998.4 μs -0.2%
future-settle-early-1 299.7 μs 299.0 μs -0.2%
future-settle-early-2 675.6 μs 672.7 μs -0.4%
future-settle-early-3 675.5 μs 673.8 μs -0.3%
future-settle-early-4 762.9 μs 764.2 μs +0.2%
game-sm-success_1-1 484.2 μs 484.6 μs +0.1%
game-sm-success_1-2 222.0 μs 221.5 μs -0.2%
game-sm-success_1-3 801.3 μs 799.9 μs -0.2%
game-sm-success_1-4 258.2 μs 257.6 μs -0.2%
game-sm-success_2-1 483.0 μs 482.9 μs -0.0%
game-sm-success_2-2 221.4 μs 221.3 μs -0.0%
game-sm-success_2-3 800.2 μs 801.8 μs +0.2%
game-sm-success_2-4 259.2 μs 258.4 μs -0.3%
game-sm-success_2-5 804.5 μs 803.0 μs -0.2%
game-sm-success_2-6 258.4 μs 258.3 μs -0.0%
multisig-sm-1 501.5 μs 500.3 μs -0.2%
multisig-sm-2 488.6 μs 489.2 μs +0.1%
multisig-sm-3 495.3 μs 494.7 μs -0.1%
multisig-sm-4 499.4 μs 497.5 μs -0.4%
multisig-sm-5 716.2 μs 713.6 μs -0.4%
multisig-sm-6 499.1 μs 496.6 μs -0.5%
multisig-sm-7 488.1 μs 485.3 μs -0.6%
multisig-sm-8 492.8 μs 493.4 μs +0.1%
multisig-sm-9 497.6 μs 498.3 μs +0.1%
multisig-sm-10 712.0 μs 714.2 μs +0.3%
ping-pong-1 409.2 μs 408.3 μs -0.2%
ping-pong-2 408.4 μs 408.1 μs -0.1%
ping-pong_2-1 239.8 μs 239.4 μs -0.2%
prism-1 184.4 μs 185.6 μs +0.7%
prism-2 519.5 μs 521.0 μs +0.3%
prism-3 439.1 μs 438.2 μs -0.2%
pubkey-1 155.3 μs 155.8 μs +0.3%
stablecoin_1-1 1.117 ms 1.112 ms -0.4%
stablecoin_1-2 218.1 μs 217.7 μs -0.2%
stablecoin_1-3 1.275 ms 1.265 ms -0.8%
stablecoin_1-4 230.9 μs 230.1 μs -0.3%
stablecoin_1-5 1.595 ms 1.586 ms -0.6%
stablecoin_1-6 285.5 μs 284.4 μs -0.4%
stablecoin_2-1 1.116 ms 1.115 ms -0.1%
stablecoin_2-2 217.1 μs 216.8 μs -0.1%
stablecoin_2-3 1.275 ms 1.271 ms -0.3%
stablecoin_2-4 230.0 μs 230.5 μs +0.2%
token-account-1 223.7 μs 223.7 μs 0.0%
token-account-2 397.6 μs 396.2 μs -0.4%
uniswap-1 507.9 μs 506.4 μs -0.3%
uniswap-2 262.1 μs 262.7 μs +0.2%
uniswap-3 2.030 ms 2.035 ms +0.2%
uniswap-4 379.5 μs 380.2 μs +0.2%
uniswap-5 1.400 ms 1.401 ms +0.1%
uniswap-6 363.9 μs 365.3 μs +0.4%
vesting-1 428.8 μs 429.8 μs +0.2%

@effectfully effectfully force-pushed the effectfully/builtins/stop-worker-wrapper branch from 1c0634c to 86aeb2f Compare April 27, 2022 17:36
@effectfully
Copy link
Contributor Author

Comments (well, comment) addressed, merging.

@effectfully effectfully merged commit 19f0ff1 into master Apr 27, 2022
@effectfully effectfully deleted the effectfully/builtins/stop-worker-wrapper branch April 27, 2022 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelpj michaelpj michaelpj approved these changes

Assignees
No one assigned
Labels
Builtins Do not merge Evaluation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@effectfully @iohk-devops @michaelpj