-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Cost of contract calls snapshot tests #7440
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
Conversation
CodSpeed Performance ReportMerging #7440 will not alter performanceComparing Summary
|
e59a081 to
ebd23f5
Compare
ironcev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very useful extensions of our snapshot testing infrastructure! ❤️ Both the blocks and also getting the historical gas values in HTML. Excellent documentation as well! We definitely need it considering how feature-rich snapshot testing becomes.
fca605d to
d614ca1
Compare
## Description This is an optimization of contract calls on top of #7440. ```csv const_of_contract_call::cost_of_in_array_0,1976,1285 const_of_contract_call::cost_of_in_array_1,3164,1779 const_of_contract_call::cost_of_in_array_16,5096,4753 const_of_contract_call::cost_of_in_array_32,8421,8079 const_of_contract_call::cost_of_in_array_64,60382,14545 const_of_contract_call::cost_of_in_array_8,4724,4382 const_of_contract_call::cost_of_in_b256,2491,2147 const_of_contract_call::cost_of_in_bool,2627,2279 const_of_contract_call::cost_of_in_str_0,3043,2697 const_of_contract_call::cost_of_in_str_1,3345,2998 const_of_contract_call::cost_of_in_str_16,3374,3030 const_of_contract_call::cost_of_in_str_32,3572,3227 const_of_contract_call::cost_of_in_str_8,3768,3421 const_of_contract_call::cost_of_in_tuple_0,3822,3479 const_of_contract_call::cost_of_in_tuple_1,4392,4050 const_of_contract_call::cost_of_in_tuple_2,4761,4418 const_of_contract_call::cost_of_in_tuple_3,5104,4761 const_of_contract_call::cost_of_in_tuple_4,5468,5126 const_of_contract_call::cost_of_in_u16,4493,4141 const_of_contract_call::cost_of_in_u256,4777,4433 const_of_contract_call::cost_of_in_u32,4926,4582 const_of_contract_call::cost_of_in_u64,4934,4589 const_of_contract_call::cost_of_in_u8,4874,4526 const_of_contract_call::in_enum_u64,3186,2843 const_of_contract_call::in_enum_u64_u64,3061,2718 const_of_contract_call::in_enum_u64_u64_u64,3173,2830 const_of_contract_call::in_struct_u64,3635,3293 const_of_contract_call::in_struct_u64_u64,3897,3554 const_of_contract_call::in_struct_u64_u64_u64,4156,3813 const_of_contract_call::isolated_cost_of_in_array_0,956,956 const_of_contract_call::isolated_cost_of_in_array_1,1515,1185 const_of_contract_call::isolated_cost_of_in_array_16,3955,3266 const_of_contract_call::isolated_cost_of_in_array_32,6544,5471 const_of_contract_call::isolated_cost_of_in_array_64,9914,9914 const_of_contract_call::isolated_cost_of_in_array_8,3225,2728 const_of_contract_call::isolated_cost_of_in_b256,1462,1120 const_of_contract_call::isolated_cost_of_in_bool,1358,1047 const_of_contract_call::isolated_cost_of_in_str_0,1451,1140 const_of_contract_call::isolated_cost_of_in_str_1,1547,1237 const_of_contract_call::isolated_cost_of_in_str_16,1552,1241 const_of_contract_call::isolated_cost_of_in_str_32,1553,1242 const_of_contract_call::isolated_cost_of_in_str_8,1551,1241 const_of_contract_call::isolated_cost_of_in_tuple_0,820,820 const_of_contract_call::isolated_cost_of_in_tuple_1,1412,1090 const_of_contract_call::isolated_cost_of_in_tuple_2,1615,1304 const_of_contract_call::isolated_cost_of_in_tuple_3,1766,1455 const_of_contract_call::isolated_cost_of_in_tuple_4,1916,1605 const_of_contract_call::isolated_cost_of_in_u16,1428,1117 const_of_contract_call::isolated_cost_of_in_u256,1431,1120 const_of_contract_call::isolated_cost_of_in_u32,1554,1243 const_of_contract_call::isolated_cost_of_in_u64,1336,1025 const_of_contract_call::isolated_cost_of_in_u8,1352,1041 ``` ## Checklist - [x] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
## Description This is an optimisation of contract calls on top of #7440. The old version would pass a string with the method name and encode it, which is `<LEN><BYTES`. Now the compiler generates these bytes directly, avoiding encoding the method name in the first place. For that this PR creats the concept of `Literal::Binary` that does not exist in the language, yet. Would be something like `b""` in Rust. ## Improvements | Test | Before | After | Percentage | |------|--------|-------|------------| | should_pass/empty_fields_in_storage_struct (test.toml)::test_read_write_bytes | 19298 | 17533 | 9.15% | | should_pass/empty_fields_in_storage_struct (test.toml)::test_read_write_map | 14971 | 13986 | 6.58% | | should_pass/empty_fields_in_storage_struct (test.toml)::test_read_write_vec | 31061 | 29288 | 5.71% | | should_pass/language/associated_const_abi (test.toml)::test | 8789 | 7817 | 11.06% | | should_pass/language/associated_const_abi_multiple (test.toml)::test | 2421 | 2097 | 13.38% | | should_pass/language/associated_const_in_decls_of_other_constants (test.toml)::test | 870 | 709 | 18.51% | | should_pass/language/contract_caller_dynamic_address (test.toml) | 540 | 379 | 29.81% | | should_pass/language/contract_ret_intrinsic (test.toml)::test | 2133 | 1809 | 15.19% | | should_pass/language/pusha_popa_multiple_defreg (test.toml)::incorrect_pusha_popa | 819 | 624 | 23.81% | | should_pass/language/raw_identifiers (test.error_type.toml)::test | 1236 | 1075 | 13.03% | | should_pass/language/raw_identifiers (test.toml)::test | 1236 | 1075 | 13.03% | | should_pass/language/references/mutability_of_references_memcpy_bug (test.toml)::test | 1364 | 1203 | 11.80% | | should_pass/language/slice/slice_contract (test.toml)::test_success | 1338 | 1145 | 14.42% | | should_pass/language/string_slice/string_slice_contract (test.toml)::test_success | 1515 | 1322 | 12.74% | | should_pass/stdlib/storage_vec_insert (test.toml)::test_test_function | 3992 | 3831 | 4.03% | | should_pass/storage_element_key_modification (test.toml)::test_storage_key_address | 1300 | 1138 | 12.46% | | should_pass/storage_element_key_modification (test.toml)::test_storage_key_modification | 919 | 757 | 17.63% | | should_pass/storage_slot_key_calculation (test.toml)::test | 3140 | 2979 | 5.13% | | should_pass/superabi_contract_calls (test.toml)::tests | 2136 | 1812 | 15.17% | | should_pass/superabi_supertrait_same_methods (test.toml)::tests | 1097 | 936 | 14.68% | | should_pass/test_abis/abi_impl_methods_callable (test.toml)::tests | 933 | 772 | 17.26% | | should_pass/test_abis/contract_abi-auto_impl (test.toml)::tests | 933 | 772 | 17.26% | | should_pass/unit_tests/aggr_indexing (test.toml)::test1 | 6007 | 5443 | 9.39% | | should_pass/unit_tests/contract-multi-contract-calls (test.toml)::test_contract_2_call | 966 | 804 | 16.77% | | should_pass/unit_tests/contract-multi-contract-calls (test.toml)::test_contract_call | 969 | 807 | 16.72% | | should_pass/unit_tests/contract-multi-contract-calls (test.toml)::test_contract_multi_call | 1916 | 1592 | 16.91% | | should_pass/unit_tests/contract_multi_test (test.toml)::test_fail | 970 | 808 | 16.70% | | should_pass/unit_tests/contract_multi_test (test.toml)::test_success | 968 | 806 | 16.74% | | should_pass/unit_tests/script-contract-calls (test.toml)::test_contract_call | 931 | 736 | 20.95% | | should_pass/unit_tests/workspace_test (test.toml)::test_fail | 970 | 808 | 16.70% | | should_pass/unit_tests/workspace_test (test.toml)::test_success | 969 | 807 | 16.72% | ## Failed Test Only now I realized that the test for array of size zero `[u64; 0]` is not working. As this is not a useful case, I will solve it in another PR. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] If my change requires substantial documentation changes, I have [requested support from the DevRel team](https://github.com/FuelLabs/devrel-requests/issues/new/choose) - [x] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
Description
This PR is a prequel for #7419.
It will set the baseline, which future PRs need to improve.
Apart from that it also:
1 - Fix a problem with
SubtsTypeforTyConstantDecl;2 - Introduce a new way to write snapshot tests. Now the test source code can have "blocks"
These blocks can be manipulated from inside the
snapshot.toml. In this case, thetomlis:Which repeats the inner
forc testfor each block, removing all others. That allows me to check the cost of the contract method selection algorithm for each contract method.For example,
cost_of_in_array_64with all other contract methods costs60382, but alone it only costs58218, which means that around 2000 was "wasted" in the contract method selection.Checklist
Breaking*orNew Featurelabels where relevant.