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.

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

Remove caching of builtin runtimes #4319

Closed

Conversation

effectfully
Copy link
Contributor

Don't look here yet.

@effectfully
Copy link
Contributor Author

/benchmark

This reverts commit c016cff.
@iohk-devops
Copy link

Comparing benchmark results of '4710dff2e' (base) and 'c016cff89' (PR)

Script 4710dff c016cff Change
auction_1-1 315.8 μs 491.1 μs +55.5%
auction_1-2 1.062 ms 1.302 ms +22.6%
auction_1-3 1.066 ms 1.313 ms +23.2%
auction_1-4 415.6 μs 653.0 μs +57.1%
auction_2-1 318.5 μs 495.1 μs +55.4%
auction_2-2 1.068 ms 1.301 ms +21.8%
auction_2-3 1.350 ms 1.637 ms +21.3%
auction_2-4 1.069 ms 1.312 ms +22.7%
auction_2-5 416.6 μs 655.3 μs +57.3%
crowdfunding-success-1 376.3 μs 587.0 μs +56.0%
crowdfunding-success-2 376.5 μs 586.9 μs +55.9%
crowdfunding-success-3 377.2 μs 587.1 μs +55.6%
currency-1 414.2 μs 566.0 μs +36.6%
escrow-redeem_1-1 619.1 μs 849.1 μs +37.2%
escrow-redeem_1-2 618.2 μs 847.2 μs +37.0%
escrow-redeem_2-1 654.8 μs 966.9 μs +47.7%
escrow-redeem_2-2 656.9 μs 966.7 μs +47.2%
escrow-redeem_2-3 654.6 μs 965.9 μs +47.6%
escrow-refund-1 281.4 μs 438.1 μs +55.7%
future-increase-margin-1 413.7 μs 564.2 μs +36.4%
future-increase-margin-2 934.5 μs 1.203 ms +28.7%
future-increase-margin-3 930.6 μs 1.202 ms +29.2%
future-increase-margin-4 862.8 μs 1.081 ms +25.3%
future-increase-margin-5 1.296 ms 1.551 ms +19.7%
future-pay-out-1 412.2 μs 563.7 μs +36.8%
future-pay-out-2 934.8 μs 1.204 ms +28.8%
future-pay-out-3 935.0 μs 1.204 ms +28.8%
future-pay-out-4 1.294 ms 1.554 ms +20.1%
future-settle-early-1 414.8 μs 565.5 μs +36.3%
future-settle-early-2 934.8 μs 1.207 ms +29.1%
future-settle-early-3 933.4 μs 1.203 ms +28.9%
future-settle-early-4 1.009 ms 1.239 ms +22.8%
game-sm-success_1-1 691.3 μs 923.8 μs +33.6%
game-sm-success_1-2 354.5 μs 553.1 μs +56.0%
game-sm-success_1-3 1.069 ms 1.326 ms +24.0%
game-sm-success_1-4 414.7 μs 642.5 μs +54.9%
game-sm-success_2-1 691.3 μs 926.1 μs +34.0%
game-sm-success_2-2 355.7 μs 554.9 μs +56.0%
game-sm-success_2-3 1.072 ms 1.329 ms +24.0%
game-sm-success_2-4 415.6 μs 643.2 μs +54.8%
game-sm-success_2-5 1.071 ms 1.327 ms +23.9%
game-sm-success_2-6 414.6 μs 642.7 μs +55.0%
multisig-sm-1 702.9 μs 913.1 μs +29.9%
multisig-sm-2 687.9 μs 903.3 μs +31.3%
multisig-sm-3 693.9 μs 913.5 μs +31.6%
multisig-sm-4 701.9 μs 920.3 μs +31.1%
multisig-sm-5 953.2 μs 1.180 ms +23.8%
multisig-sm-6 704.1 μs 916.8 μs +30.2%
multisig-sm-7 691.2 μs 906.8 μs +31.2%
multisig-sm-8 701.2 μs 912.6 μs +30.1%
multisig-sm-9 706.5 μs 919.8 μs +30.2%
multisig-sm-10 952.7 μs 1.176 ms +23.4%
ping-pong-1 575.2 μs 777.9 μs +35.2%
ping-pong-2 574.0 μs 778.3 μs +35.6%
ping-pong_2-1 352.9 μs 525.8 μs +49.0%
prism-1 294.2 μs 460.5 μs +56.5%
prism-2 749.9 μs 999.9 μs +33.3%
prism-3 632.5 μs 871.8 μs +37.8%
pubkey-1 252.3 μs 393.1 μs +55.8%
stablecoin_1-1 1.476 ms 1.852 ms +25.5%
stablecoin_1-2 346.7 μs 538.9 μs +55.4%
stablecoin_1-3 1.681 ms 2.115 ms +25.8%
stablecoin_1-4 367.4 μs 577.0 μs +57.0%
stablecoin_1-5 2.110 ms 2.705 ms +28.2%
stablecoin_1-6 456.9 μs 704.9 μs +54.3%
stablecoin_2-1 1.477 ms 1.853 ms +25.5%
stablecoin_2-2 347.6 μs 538.5 μs +54.9%
stablecoin_2-3 1.687 ms 2.108 ms +25.0%
stablecoin_2-4 368.6 μs 575.9 μs +56.2%
token-account-1 321.6 μs 457.0 μs +42.1%
token-account-2 570.0 μs 776.4 μs +36.2%
uniswap-1 674.6 μs 841.0 μs +24.7%
uniswap-2 384.2 μs 562.4 μs +46.4%
uniswap-3 2.721 ms 3.361 ms +23.5%
uniswap-4 606.4 μs 914.0 μs +50.7%
uniswap-5 1.918 ms 2.470 ms +28.8%
uniswap-6 577.4 μs 869.0 μs +50.5%
vesting-1 594.4 μs 782.9 μs +31.7%

@effectfully
Copy link
Contributor Author

Nice :)
But worry not, that is to be expected. Let's try to revert that.

@effectfully
Copy link
Contributor Author

/benchmark

@iohk-devops
Copy link

Comparing benchmark results of '4710dff2e' (base) and '90dded294' (PR)

Script 4710dff 90dded2 Change
auction_1-1 314.7 μs 382.9 μs +21.7%
auction_1-2 1.063 ms 1.148 ms +8.0%
auction_1-3 1.065 ms 1.151 ms +8.1%
auction_1-4 415.1 μs 505.5 μs +21.8%
auction_2-1 317.5 μs 383.8 μs +20.9%
auction_2-2 1.062 ms 1.152 ms +8.5%
auction_2-3 1.340 ms 1.443 ms +7.7%
auction_2-4 1.063 ms 1.151 ms +8.3%
auction_2-5 415.9 μs 504.8 μs +21.4%
crowdfunding-success-1 375.0 μs 450.0 μs +20.0%
crowdfunding-success-2 374.5 μs 451.7 μs +20.6%
crowdfunding-success-3 374.4 μs 452.7 μs +20.9%
currency-1 412.5 μs 476.5 μs +15.5%
escrow-redeem_1-1 615.8 μs 711.4 μs +15.5%
escrow-redeem_1-2 614.7 μs 709.5 μs +15.4%
escrow-redeem_2-1 653.5 μs 820.7 μs +25.6%
escrow-redeem_2-2 651.6 μs 817.9 μs +25.5%
escrow-redeem_2-3 652.0 μs 820.6 μs +25.9%
escrow-refund-1 280.3 μs 341.8 μs +21.9%
future-increase-margin-1 410.6 μs 475.2 μs +15.7%
future-increase-margin-2 927.8 μs 1.024 ms +10.4%
future-increase-margin-3 930.8 μs 1.026 ms +10.2%
future-increase-margin-4 868.8 μs 950.9 μs +9.4%
future-increase-margin-5 1.292 ms 1.385 ms +7.2%
future-pay-out-1 411.8 μs 471.6 μs +14.5%
future-pay-out-2 927.7 μs 1.027 ms +10.7%
future-pay-out-3 927.3 μs 1.027 ms +10.8%
future-pay-out-4 1.291 ms 1.387 ms +7.4%
future-settle-early-1 413.3 μs 473.6 μs +14.6%
future-settle-early-2 934.4 μs 1.027 ms +9.9%
future-settle-early-3 933.8 μs 1.031 ms +10.4%
future-settle-early-4 1.006 ms 1.104 ms +9.7%
game-sm-success_1-1 690.1 μs 786.6 μs +14.0%
game-sm-success_1-2 353.3 μs 429.3 μs +21.5%
game-sm-success_1-3 1.067 ms 1.166 ms +9.3%
game-sm-success_1-4 413.8 μs 500.6 μs +21.0%
game-sm-success_2-1 690.1 μs 782.5 μs +13.4%
game-sm-success_2-2 353.1 μs 427.4 μs +21.0%
game-sm-success_2-3 1.065 ms 1.164 ms +9.3%
game-sm-success_2-4 413.1 μs 499.5 μs +20.9%
game-sm-success_2-5 1.066 ms 1.162 ms +9.0%
game-sm-success_2-6 413.1 μs 499.9 μs +21.0%
multisig-sm-1 700.8 μs 787.8 μs +12.4%
multisig-sm-2 689.4 μs 773.8 μs +12.2%
multisig-sm-3 692.5 μs 783.0 μs +13.1%
multisig-sm-4 700.8 μs 789.1 μs +12.6%
multisig-sm-5 946.2 μs 1.042 ms +10.1%
multisig-sm-6 702.1 μs 791.4 μs +12.7%
multisig-sm-7 688.9 μs 776.3 μs +12.7%
multisig-sm-8 696.8 μs 782.8 μs +12.3%
multisig-sm-9 706.0 μs 789.7 μs +11.9%
multisig-sm-10 951.9 μs 1.040 ms +9.3%
ping-pong-1 572.9 μs 652.2 μs +13.8%
ping-pong-2 573.3 μs 652.3 μs +13.8%
ping-pong_2-1 352.3 μs 414.1 μs +17.5%
prism-1 291.8 μs 329.0 μs +12.7%
prism-2 749.0 μs 852.0 μs +13.8%
prism-3 630.0 μs 729.9 μs +15.9%
pubkey-1 250.7 μs 309.9 μs +23.6%
stablecoin_1-1 1.473 ms 1.594 ms +8.2%
stablecoin_1-2 345.2 μs 419.8 μs +21.6%
stablecoin_1-3 1.674 ms 1.801 ms +7.6%
stablecoin_1-4 365.5 μs 445.7 μs +21.9%
stablecoin_1-5 2.105 ms 2.258 ms +7.3%
stablecoin_1-6 455.6 μs 548.5 μs +20.4%
stablecoin_2-1 1.472 ms 1.591 ms +8.1%
stablecoin_2-2 346.5 μs 419.3 μs +21.0%
stablecoin_2-3 1.677 ms 1.803 ms +7.5%
stablecoin_2-4 367.0 μs 446.3 μs +21.6%
token-account-1 319.0 μs 336.6 μs +5.5%
token-account-2 567.1 μs 652.5 μs +15.1%
uniswap-1 670.5 μs 750.4 μs +11.9%
uniswap-2 383.4 μs 450.6 μs +17.5%
uniswap-3 2.705 ms 2.870 ms +6.1%
uniswap-4 602.2 μs 716.4 μs +19.0%
uniswap-5 1.911 ms 2.069 ms +8.3%
uniswap-6 575.8 μs 683.0 μs +18.6%
vesting-1 592.2 μs 665.3 μs +12.3%

@effectfully
Copy link
Contributor Author

/benchmark

@iohk-devops
Copy link

Comparing benchmark results of '4710dff2e' (base) and '250ae7ba4' (PR)

Script 4710dff 250ae7b Change
auction_1-1 317.2 μs 385.1 μs +21.4%
auction_1-2 1.062 ms 1.151 ms +8.4%
auction_1-3 1.063 ms 1.149 ms +8.1%
auction_1-4 415.5 μs 503.4 μs +21.2%
auction_2-1 317.4 μs 382.7 μs +20.6%
auction_2-2 1.061 ms 1.152 ms +8.6%
auction_2-3 1.340 ms 1.445 ms +7.8%
auction_2-4 1.059 ms 1.154 ms +9.0%
auction_2-5 415.5 μs 506.1 μs +21.8%
crowdfunding-success-1 375.9 μs 451.1 μs +20.0%
crowdfunding-success-2 376.8 μs 450.2 μs +19.5%
crowdfunding-success-3 376.6 μs 451.0 μs +19.8%
currency-1 414.8 μs 473.1 μs +14.1%
escrow-redeem_1-1 618.0 μs 704.6 μs +14.0%
escrow-redeem_1-2 617.9 μs 705.3 μs +14.1%
escrow-redeem_2-1 656.3 μs 814.3 μs +24.1%
escrow-redeem_2-2 656.7 μs 811.9 μs +23.6%
escrow-redeem_2-3 656.6 μs 815.9 μs +24.3%
escrow-refund-1 282.1 μs 339.8 μs +20.5%
future-increase-margin-1 412.3 μs 472.6 μs +14.6%
future-increase-margin-2 927.9 μs 1.025 ms +10.5%
future-increase-margin-3 927.8 μs 1.024 ms +10.4%
future-increase-margin-4 860.4 μs 948.4 μs +10.2%
future-increase-margin-5 1.286 ms 1.392 ms +8.2%
future-pay-out-1 411.4 μs 473.9 μs +15.2%
future-pay-out-2 933.4 μs 1.027 ms +10.0%
future-pay-out-3 931.6 μs 1.025 ms +10.0%
future-pay-out-4 1.290 ms 1.386 ms +7.4%
future-settle-early-1 412.5 μs 472.6 μs +14.6%
future-settle-early-2 929.3 μs 1.022 ms +10.0%
future-settle-early-3 930.3 μs 1.022 ms +9.9%
future-settle-early-4 1.008 ms 1.096 ms +8.7%
game-sm-success_1-1 691.8 μs 779.4 μs +12.7%
game-sm-success_1-2 356.1 μs 427.2 μs +20.0%
game-sm-success_1-3 1.070 ms 1.162 ms +8.6%
game-sm-success_1-4 416.8 μs 497.4 μs +19.3%
game-sm-success_2-1 693.4 μs 779.0 μs +12.3%
game-sm-success_2-2 355.5 μs 426.3 μs +19.9%
game-sm-success_2-3 1.066 ms 1.159 ms +8.7%
game-sm-success_2-4 415.2 μs 499.0 μs +20.2%
game-sm-success_2-5 1.066 ms 1.163 ms +9.1%
game-sm-success_2-6 415.0 μs 497.6 μs +19.9%
multisig-sm-1 699.6 μs 787.0 μs +12.5%
multisig-sm-2 685.7 μs 773.2 μs +12.8%
multisig-sm-3 694.0 μs 779.4 μs +12.3%
multisig-sm-4 701.8 μs 786.5 μs +12.1%
multisig-sm-5 949.1 μs 1.036 ms +9.2%
multisig-sm-6 699.8 μs 787.5 μs +12.5%
multisig-sm-7 685.9 μs 772.1 μs +12.6%
multisig-sm-8 697.4 μs 781.7 μs +12.1%
multisig-sm-9 706.1 μs 788.2 μs +11.6%
multisig-sm-10 953.1 μs 1.036 ms +8.7%
ping-pong-1 574.5 μs 649.0 μs +13.0%
ping-pong-2 573.4 μs 647.3 μs +12.9%
ping-pong_2-1 353.3 μs 411.3 μs +16.4%
prism-1 294.7 μs 326.9 μs +10.9%
prism-2 751.4 μs 847.7 μs +12.8%
prism-3 632.9 μs 727.6 μs +15.0%
pubkey-1 252.4 μs 310.9 μs +23.2%
stablecoin_1-1 1.470 ms 1.595 ms +8.5%
stablecoin_1-2 347.3 μs 419.6 μs +20.8%
stablecoin_1-3 1.675 ms 1.806 ms +7.8%
stablecoin_1-4 367.1 μs 446.3 μs +21.6%
stablecoin_1-5 2.103 ms 2.266 ms +7.8%
stablecoin_1-6 456.5 μs 548.5 μs +20.2%
stablecoin_2-1 1.467 ms 1.590 ms +8.4%
stablecoin_2-2 347.2 μs 418.7 μs +20.6%
stablecoin_2-3 1.679 ms 1.795 ms +6.9%
stablecoin_2-4 368.7 μs 446.1 μs +21.0%
token-account-1 320.2 μs 337.0 μs +5.2%
token-account-2 568.9 μs 646.9 μs +13.7%
uniswap-1 672.6 μs 746.3 μs +11.0%
uniswap-2 384.6 μs 450.5 μs +17.1%
uniswap-3 2.716 ms 2.852 ms +5.0%
uniswap-4 605.6 μs 716.4 μs +18.3%
uniswap-5 1.919 ms 2.070 ms +7.9%
uniswap-6 578.2 μs 684.7 μs +18.4%
vesting-1 591.3 μs 666.4 μs +12.7%

@effectfully
Copy link
Contributor Author

Well, that is surprising to me. The code now is supposed to push all the thunks for meanings of builtins outside if the main loop, so that they're only created once, but either I've failed at that (I should try using some trace or something, it's hard to figure out what's going on from reading Core) or the cost of converting from BuiltinMeaning to BuiltinRuntime on the fly is just that high, which seems unlikely.

@effectfully
Copy link
Contributor Author

Ok, I'm pretty clueless as to what's going on here. The thing is, we might be having the same problem at startup currently without knowing it. Whatever is slow here is quite likely also slow during the initial traversal where we compute the BuiltinRuntime for each builtin in master. This certainly needs to be investigated, but I'm gonna postpone that for now.

@effectfully effectfully closed this Jan 9, 2022
@michaelpj
Copy link
Contributor

The thing is, we might be having the same problem at startup currently without knowing it.

That's true. However, if that's the case we can certainly address it on the ledger side by ensuring that that is only done once and is then cached.

@effectfully effectfully deleted the effectfully/evaluation/no-lookup-builtin-2 branch May 5, 2022 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants