Skip to content

Commit 6bca188

Browse files
xunilrjJoshuaBatty
andauthored
Improve method selector on contracts (#7458)
## Description This PR continues the optimisation of contract calls on top of #7455. Now the contract method selector is optimised. Ideally, the method selection would just be a `match` and we would let the compiler generate the best code. But we are not there yet. So, for now, we are going to first test the called method length. And based on its length we are going to check the whole string. Something like: ```sway if called_method.len() == 5 { if called_method == "abcde" { ... } if called_method == "fghij" { ... } } if called_method.len() == 10 { if called_method == "..." { .... } } ``` This alone reduces gas a lot. To understand why `match_expressions_all` gas usage is much worse see below. | Test | Before | After | Percentage | |------|--------|-------|------------| | should_pass/empty_fields_in_storage_struct (test.toml)::test_read_write_bytes | 17533 | 12080 | 31.10% | | should_pass/empty_fields_in_storage_struct (test.toml)::test_read_write_map | 13986 | 8502 | 39.21% | | should_pass/empty_fields_in_storage_struct (test.toml)::test_read_write_vec | 29288 | 12797 | 56.31% | | should_pass/language/associated_const_abi (test.toml)::test | 7817 | 4547 | 41.83% | | should_pass/language/associated_const_abi_multiple (test.toml)::test | 2097 | 1511 | 27.94% | | should_pass/language/associated_const_in_decls_of_other_constants (test.toml)::test | 709 | 537 | 24.26% | | should_pass/language/contract_ret_intrinsic (test.toml)::test | 1809 | 1465 | 19.02% | | should_pass/language/match_expressions_all (test.toml) | 764 | 3708 | -385.34% | | should_pass/language/pusha_popa_multiple_defreg (test.toml)::incorrect_pusha_popa | 624 | 451 | 27.72% | | should_pass/language/raw_identifiers (test.error_type.toml)::test | 1075 | 903 | 16.00% | | should_pass/language/raw_identifiers (test.toml)::test | 1075 | 903 | 16.00% | | should_pass/language/references/mutability_of_references_memcpy_bug (test.toml)::test | 1203 | 1031 | 14.30% | | should_pass/language/slice/slice_contract (test.toml)::test_success | 1145 | 920 | 19.65% | | should_pass/language/string_slice/string_slice_contract (test.toml)::test_success | 1322 | 1079 | 18.38% | | should_pass/stdlib/storage_vec_insert (test.toml)::test_test_function | 3831 | 3669 | 4.23% | | should_pass/storage_element_key_modification (test.toml)::test_storage_key_address | 1138 | 943 | 17.14% | | should_pass/storage_element_key_modification (test.toml)::test_storage_key_modification | 757 | 465 | 38.57% | | should_pass/storage_slot_key_calculation (test.toml)::test | 2979 | 2785 | 6.51% | | should_pass/superabi_contract_calls (test.toml)::tests | 1812 | 1310 | 27.70% | | should_pass/superabi_supertrait_external_call (test.toml) | 95 | 76 | 20.00% | | should_pass/superabi_supertrait_same_methods (test.toml)::tests | 936 | 634 | 32.26% | | should_pass/test_abis/abi_impl_methods_callable (test.toml)::tests | 772 | 599 | 22.41% | | should_pass/test_abis/contract_abi-auto_impl (test.toml)::tests | 772 | 599 | 22.41% | | should_pass/unit_tests/aggr_indexing (test.toml)::test1 | 5443 | 4282 | 21.33% | | should_pass/unit_tests/contract-multi-contract-calls (test.toml)::test_contract_2_call | 804 | 631 | 21.52% | | should_pass/unit_tests/contract-multi-contract-calls (test.toml)::test_contract_call | 807 | 634 | 21.44% | | should_pass/unit_tests/contract-multi-contract-calls (test.toml)::test_contract_multi_call | 1592 | 1246 | 21.73% | | should_pass/unit_tests/contract_multi_test (test.toml)::test_fail | 808 | 635 | 21.41% | | should_pass/unit_tests/contract_multi_test (test.toml)::test_success | 806 | 633 | 21.46% | | should_pass/unit_tests/script-contract-calls (test.toml)::test_contract_call | 736 | 563 | 23.51% | | should_pass/unit_tests/workspace_test (test.toml)::test_fail | 808 | 635 | 21.41% | | should_pass/unit_tests/workspace_test (test.toml)::test_success | 807 | 634 | 21.44% | # Radix Trie This PR also turns off the radix trie optimisation at `sway-core/src/semantic_analysis/ast_node/expression/match_expression/typed/typed_match_expression.rs`. I am not 100% sure of its correctness and heuristics. We need a better way to decide when not to use it. One of the examples where the generated code is worse is when all strings are very similar. For example: `get_a` , `get_b`. The generated trie will first test its length. Both have length 5. Not much was gained. Then it tests if for `get_`, which makes sense as it is the common substring. And the last step will test for `a` or `b`. The issue is that all branches and jumps of the last step are not worthy to test just one character. Is probably cheaper to just test the whole string for each option. In the end, we need a better heuristic to determine when to use this optimisation. ## 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). - [ ] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Joshua Batty <[email protected]>
1 parent e6f2ac8 commit 6bca188

File tree

25 files changed

+844
-752
lines changed

25 files changed

+844
-752
lines changed

forc-plugins/forc-client/tests/deploy.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ async fn test_simple_deploy() {
377377
node.kill().unwrap();
378378
let expected = vec![DeployedPackage::Contract(DeployedContract {
379379
id: ContractId::from_str(
380-
"b9443b8e389d42e551b80bb7e94adb4dd37c6985088401940035e36489c90af8",
380+
"d54d140236c92deb6b10fb5926d0362f86654111cd8e24bb9b416a84c5eba9a5",
381381
)
382382
.unwrap(),
383383
proxy: None,
@@ -421,7 +421,7 @@ async fn test_deploy_submit_only() {
421421
node.kill().unwrap();
422422
let expected = vec![DeployedPackage::Contract(DeployedContract {
423423
id: ContractId::from_str(
424-
"b9443b8e389d42e551b80bb7e94adb4dd37c6985088401940035e36489c90af8",
424+
"d54d140236c92deb6b10fb5926d0362f86654111cd8e24bb9b416a84c5eba9a5",
425425
)
426426
.unwrap(),
427427
proxy: None,
@@ -468,12 +468,12 @@ async fn test_deploy_fresh_proxy() {
468468
node.kill().unwrap();
469469
let impl_contract = DeployedPackage::Contract(DeployedContract {
470470
id: ContractId::from_str(
471-
"b9443b8e389d42e551b80bb7e94adb4dd37c6985088401940035e36489c90af8",
471+
"d54d140236c92deb6b10fb5926d0362f86654111cd8e24bb9b416a84c5eba9a5",
472472
)
473473
.unwrap(),
474474
proxy: Some(
475475
ContractId::from_str(
476-
"e597d0ef3dc3375402f32bcaa7fc66940f92c31a916e87e7eebb32fcd147d752",
476+
"3806de177541f8e9e970640c1455bc9740a2b0248ee959a634ffe4ab6df558e2",
477477
)
478478
.unwrap(),
479479
),

forc/tests/cli_integration.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,10 @@ fn test_forc_test_raw_logs() -> Result<(), rexpect::error::Error> {
5151
// Assert that the output is correct
5252
process.exp_string(" test test_log_4")?;
5353
process.exp_string("raw logs:")?;
54-
process.exp_string(r#"[{"LogData":{"data":"0000000000000004","digest":"8005f02d43fa06e7d0585fb64c961d57e318b27a145c857bcd3a6bdb413ff7fc","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12408,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
54+
process.exp_string(r#"[{"LogData":{"data":"0000000000000004","digest":"8005f02d43fa06e7d0585fb64c961d57e318b27a145c857bcd3a6bdb413ff7fc","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12044,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
5555
process.exp_string(" test test_log_2")?;
5656
process.exp_string("raw logs:")?;
57-
process.exp_string(r#"[{"LogData":{"data":"0000000000000002","digest":"cd04a4754498e06db5a13c5f371f1f04ff6d2470f24aa9bd886540e5dce77f70","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12408,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
57+
process.exp_string(r#"[{"LogData":{"data":"0000000000000002","digest":"cd04a4754498e06db5a13c5f371f1f04ff6d2470f24aa9bd886540e5dce77f70","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12044,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
5858

5959
process.process.exit()?;
6060
Ok(())
@@ -77,12 +77,12 @@ fn test_forc_test_both_logs() -> Result<(), rexpect::error::Error> {
7777
process.exp_string("decoded log values:")?;
7878
process.exp_string("4, log rb: 1515152261580153489")?;
7979
process.exp_string("raw logs:")?;
80-
process.exp_string(r#"[{"LogData":{"data":"0000000000000004","digest":"8005f02d43fa06e7d0585fb64c961d57e318b27a145c857bcd3a6bdb413ff7fc","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12408,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
80+
process.exp_string(r#"[{"LogData":{"data":"0000000000000004","digest":"8005f02d43fa06e7d0585fb64c961d57e318b27a145c857bcd3a6bdb413ff7fc","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12044,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
8181
process.exp_string(" test test_log_2")?;
8282
process.exp_string("decoded log values:")?;
8383
process.exp_string("2, log rb: 1515152261580153489")?;
8484
process.exp_string("raw logs:")?;
85-
process.exp_string(r#"[{"LogData":{"data":"0000000000000002","digest":"cd04a4754498e06db5a13c5f371f1f04ff6d2470f24aa9bd886540e5dce77f70","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12408,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
85+
process.exp_string(r#"[{"LogData":{"data":"0000000000000002","digest":"cd04a4754498e06db5a13c5f371f1f04ff6d2470f24aa9bd886540e5dce77f70","id":"0000000000000000000000000000000000000000000000000000000000000000","is":10368,"len":8,"pc":12044,"ptr":67107840,"ra":0,"rb":1515152261580153489}}]"#)?;
8686
process.process.exit()?;
8787
Ok(())
8888
}

sway-core/src/semantic_analysis/ast_node/declaration/auto_impl/abi_encoding.rs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::{
88
},
99
Engines, TypeInfo, TypeParameter,
1010
};
11+
use itertools::Itertools;
1112
use std::collections::BTreeMap;
1213
use sway_error::{
1314
error::CompileError,
@@ -312,15 +313,16 @@ where
312313
fallback_fn: Option<DeclId<TyFunctionDecl>>,
313314
handler: &Handler,
314315
) -> Result<TyAstNode, ErrorEmitted> {
315-
let mut code = String::new();
316-
317316
let mut reads = false;
318317
let mut writes = false;
319318

320319
// used to check for name collisions
321320
let mut contract_methods: BTreeMap<String, Vec<Span>> = <_>::default();
322321

322+
let mut arm_by_size = BTreeMap::<usize, String>::default();
323+
323324
// generate code
325+
let mut method_names = String::new();
324326
for r in contract_fns {
325327
let decl = engines.de().get(r);
326328

@@ -382,8 +384,20 @@ where
382384
};
383385

384386
let method_name = decl.name.as_str();
387+
let offset = if let Some(offset) = method_names.find(method_name) {
388+
offset
389+
} else {
390+
let offset = method_names.len();
391+
method_names.push_str(method_name);
392+
offset
393+
};
394+
395+
let method_name_len = method_name.len();
396+
let code = arm_by_size.entry(method_name.len()).or_default();
385397

386-
code.push_str(&format!("if _method_name == \"{method_name}\" {{\n"));
398+
code.push_str(&format!("
399+
let is_this_method = asm(r, ptr: _method_name_ptr, name: _method_names_ptr, len: {method_name_len}) {{ addi r name i{offset}; meq r ptr r len; r: bool }};
400+
if is_this_method {{\n"));
387401

388402
if args_types == "()" {
389403
code.push_str(&format!(
@@ -454,10 +468,19 @@ where
454468
(false, false) => "",
455469
};
456470

471+
let code = arm_by_size
472+
.iter()
473+
.map(|(len, code)| format!("if _method_len == {len} {{ {code} }}"))
474+
.join("");
457475
let code = format!(
458476
"{att} pub fn __entry() {{
477+
let _method_names = \"{method_names}\";
459478
let mut _buffer = BufferReader::from_second_parameter();
460-
let _method_name = decode_first_param::<str>();
479+
480+
let mut _first_param_buffer = BufferReader::from_first_parameter();
481+
let _method_len = _first_param_buffer.read::<u64>();
482+
let _method_name_ptr = _first_param_buffer.ptr();
483+
let _method_names_ptr = _method_names.as_ptr();
461484
{code}
462485
{fallback}
463486
}}"

0 commit comments

Comments
 (0)