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

Clean up address usage (part 1 of many) #3912

Merged
merged 1 commit into from
Feb 16, 2023
Merged

Conversation

pattyshack
Copy link
Contributor

Prep work for changing address internal representation

The number of changes is getting a bit out of hand. I'll split up the change into smaller PRs, with the goal of consistently operating on flow.Address whenever possible.

Note that (except for tests) I'm trying to consistently name the address variables based on their types:

  • address is always a flow.Address
  • runtimeAddress is a common.Address
  • cadenceAddress is a cadence.Address

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2023

FVM Benchstat comparison

This branch with compared with the base branch onflow:master commit 48e38e6

The command (for i in {1..7}; do go test ./fvm ./engine/execution/computation --bench . --tags relic -shuffle=on --benchmem --run ^$; done) was used.

Collapsed results for better readability

old.txtnew.txt
time/opdelta
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/create_new_account-2823ms ± 1%832ms ± 1%+1.05%(p=0.011 n=7+7)
RuntimeNFTBatchTransfer-2117ms ± 2%117ms ± 3%~(p=0.818 n=6+6)
RuntimeTransaction/reference_tx-229.9ms ± 1%29.7ms ± 2%~(p=0.181 n=6+7)
RuntimeTransaction/convert_int_to_string-231.1ms ± 4%32.6ms ±16%~(p=0.731 n=6+7)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-231.9ms ± 1%32.8ms ± 3%~(p=0.051 n=6+7)
RuntimeTransaction/get_signer_address-230.9ms ±13%30.1ms ± 2%~(p=0.731 n=7+6)
RuntimeTransaction/get_public_account-234.4ms ±16%32.4ms ± 3%~(p=0.383 n=7+7)
RuntimeTransaction/get_account_and_get_balance-2266ms ± 2%266ms ± 1%~(p=0.589 n=6+6)
RuntimeTransaction/get_account_and_get_available_balance-2241ms ± 1%243ms ± 2%~(p=0.318 n=7+7)
RuntimeTransaction/get_account_and_get_storage_used-233.8ms ± 2%34.4ms ± 6%~(p=0.234 n=6+7)
RuntimeTransaction/get_account_and_get_storage_capacity-2217ms ± 1%219ms ± 2%~(p=0.295 n=6+7)
RuntimeTransaction/get_signer_vault-236.6ms ± 5%35.8ms ± 2%~(p=0.181 n=7+6)
RuntimeTransaction/get_signer_receiver-247.6ms ± 2%47.6ms ± 4%~(p=0.628 n=6+7)
RuntimeTransaction/transfer_tokens-2193ms ± 6%191ms ± 2%~(p=0.628 n=7+6)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-235.9ms ± 2%35.8ms ± 3%~(p=0.805 n=7+7)
RuntimeTransaction/load_and_save_long_string_on_signers_address-277.9ms ± 2%78.5ms ± 3%~(p=0.456 n=7+7)
RuntimeTransaction/call_empty_contract_function-232.2ms ± 2%32.4ms ± 4%~(p=1.000 n=7+7)
RuntimeTransaction/borrow_array_from_storage-2128ms ± 2%130ms ± 1%~(p=0.128 n=7+7)
RuntimeTransaction/copy_array_from_storage-2129ms ± 3%129ms ± 3%~(p=1.000 n=7+7)
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-24.31s ± 1%4.30s ± 1%~(p=0.456 n=7+7)
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/emit_event-247.8ms ± 3%46.6ms ± 1%−2.54%(p=0.004 n=6+5)
 
alloc/opdelta
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeNFTBatchTransfer-253.9MB ± 7%55.0MB ± 4%~(p=0.383 n=7+7)
RuntimeTransaction/reference_tx-234.9MB ± 3%35.0MB ± 2%~(p=0.710 n=7+7)
RuntimeTransaction/convert_int_to_string-235.3MB ± 2%34.9MB ± 4%~(p=0.318 n=7+7)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-235.1MB ± 2%35.2MB ± 2%~(p=0.620 n=7+7)
RuntimeTransaction/get_signer_address-235.2MB ± 3%35.1MB ± 2%~(p=0.902 n=7+7)
RuntimeTransaction/get_public_account-236.9MB ± 3%35.9MB ± 4%~(p=0.209 n=7+7)
RuntimeTransaction/get_account_and_get_balance-2122MB ± 2%122MB ± 2%~(p=0.945 n=6+7)
RuntimeTransaction/get_account_and_get_available_balance-2105MB ± 3%107MB ± 2%~(p=0.128 n=7+7)
RuntimeTransaction/get_account_and_get_storage_used-236.0MB ± 2%36.3MB ± 5%~(p=0.366 n=6+7)
RuntimeTransaction/get_account_and_get_storage_capacity-2102MB ± 3%102MB ± 3%~(p=0.805 n=7+7)
RuntimeTransaction/get_signer_vault-236.8MB ± 4%36.2MB ± 2%~(p=0.138 n=7+6)
RuntimeTransaction/get_signer_receiver-241.2MB ± 6%40.8MB ± 4%~(p=0.620 n=7+7)
RuntimeTransaction/transfer_tokens-283.6MB ± 3%84.0MB ± 5%~(p=0.535 n=7+7)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-235.9MB ± 1%35.5MB ± 3%~(p=0.209 n=7+7)
RuntimeTransaction/load_and_save_long_string_on_signers_address-252.5MB ± 1%52.5MB ± 2%~(p=1.000 n=7+7)
RuntimeTransaction/create_new_account-2184MB ± 3%186MB ± 4%~(p=0.165 n=7+7)
RuntimeTransaction/call_empty_contract_function-235.2MB ± 2%35.5MB ± 4%~(p=0.445 n=6+7)
RuntimeTransaction/borrow_array_from_storage-268.6MB ± 2%69.6MB ± 4%~(p=0.383 n=7+7)
RuntimeTransaction/copy_array_from_storage-282.0MB ± 4%81.0MB ± 2%~(p=0.259 n=7+7)
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-21.20GB ± 1%1.20GB ± 1%~(p=0.620 n=7+7)
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/emit_event-240.9MB ± 1%40.0MB ± 1%−2.40%(p=0.008 n=5+5)
 
allocs/opdelta
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeNFTBatchTransfer-2268k ± 0%268k ± 0%~(p=0.312 n=5+6)
RuntimeTransaction/reference_tx-282.3k ± 0%82.3k ± 0%~(p=0.279 n=7+6)
RuntimeTransaction/convert_int_to_string-294.4k ± 0%94.4k ± 0%~(p=0.169 n=7+6)
RuntimeTransaction/get_signer_address-286.4k ± 0%86.4k ± 0%~(p=0.650 n=7+7)
RuntimeTransaction/get_account_and_get_balance-21.30M ± 0%1.30M ± 0%~(p=0.915 n=7+6)
RuntimeTransaction/get_account_and_get_available_balance-21.23M ± 0%1.23M ± 0%~(p=0.259 n=7+7)
RuntimeTransaction/get_account_and_get_storage_used-2119k ± 0%119k ± 0%~(p=0.365 n=7+7)
RuntimeTransaction/get_account_and_get_storage_capacity-21.11M ± 0%1.11M ± 0%~(p=0.731 n=6+7)
RuntimeTransaction/get_signer_vault-2123k ± 0%123k ± 0%~(p=0.558 n=7+7)
RuntimeTransaction/get_signer_receiver-2197k ± 0%197k ± 0%~(p=0.094 n=6+7)
RuntimeTransaction/transfer_tokens-2842k ± 0%841k ± 0%~(p=0.053 n=7+7)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-2121k ± 0%121k ± 0%~(p=0.195 n=7+7)
RuntimeTransaction/load_and_save_long_string_on_signers_address-2192k ± 0%192k ± 0%~(p=0.168 n=7+6)
RuntimeTransaction/create_new_account-22.28M ± 0%2.28M ± 0%~(p=0.902 n=7+7)
RuntimeTransaction/call_empty_contract_function-297.7k ± 0%97.7k ± 0%~(p=0.270 n=7+7)
RuntimeTransaction/emit_event-2137k ± 0%137k ± 0%~(p=0.594 n=6+7)
RuntimeTransaction/borrow_array_from_storage-2336k ± 0%336k ± 0%~(p=0.209 n=7+7)
RuntimeTransaction/copy_array_from_storage-2292k ± 0%292k ± 0%~(p=0.245 n=7+7)
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-217.3M ± 0%17.3M ± 0%~(p=0.535 n=7+7)
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/convert_int_to_string_and_concatenate_it-2106k ± 0%106k ± 0%−0.01%(p=0.045 n=6+7)
RuntimeTransaction/get_public_account-2109k ± 0%109k ± 0%−0.01%(p=0.022 n=6+7)
 
computationdelta
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/reference_tx-2202 ± 0%202 ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-2502 ± 0%502 ± 0%~(all equal)
RuntimeTransaction/get_signer_address-2302 ± 0%302 ± 0%~(all equal)
RuntimeTransaction/get_public_account-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_balance-21.00k ± 0%1.00k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-23.10k ± 0%3.10k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-21.70k ± 0%1.70k ± 0%~(all equal)
RuntimeTransaction/get_signer_vault-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_signer_receiver-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/transfer_tokens-23.50k ± 0%3.50k ± 0%~(all equal)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/load_and_save_long_string_on_signers_address-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/create_new_account-2202 ± 0%202 ± 0%~(all equal)
RuntimeTransaction/call_empty_contract_function-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/emit_event-2602 ± 0%602 ± 0%~(all equal)
RuntimeTransaction/borrow_array_from_storage-22.60k ± 0%2.60k ± 0%~(all equal)
RuntimeTransaction/copy_array_from_storage-22.60k ± 0%2.60k ± 0%~(all equal)
 
interactionsdelta
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/reference_tx-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/get_signer_address-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/get_public_account-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_balance-224.3M ± 0%24.3M ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-25.64M ± 0%5.64M ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-25.64M ± 0%5.64M ± 0%~(all equal)
RuntimeTransaction/get_signer_vault-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/get_signer_receiver-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/transfer_tokens-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/load_and_save_long_string_on_signers_address-2334k ± 0%334k ± 0%~(all equal)
RuntimeTransaction/create_new_account-212.3M ± 0%12.3M ± 0%~(all equal)
RuntimeTransaction/call_empty_contract_function-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/emit_event-2331k ± 0%331k ± 0%~(all equal)
RuntimeTransaction/borrow_array_from_storage-2336k ± 0%336k ± 0%~(all equal)
RuntimeTransaction/copy_array_from_storage-2336k ± 0%336k ± 0%~(all equal)
 
us/txdelta
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-22.10k ± 1%2.10k ± 1%~(p=0.434 n=7+7)
 

@pattyshack pattyshack force-pushed the patrick/address-clean-up branch from c141bf0 to 0d286a4 Compare February 8, 2023 19:18
fvm/environment/transaction_info.go Outdated Show resolved Hide resolved
@pattyshack pattyshack force-pushed the patrick/address-clean-up branch from 0d286a4 to e2ee169 Compare February 14, 2023 18:05
@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2023

Codecov Report

Merging #3912 (e2ee169) into master (cff2bb9) will decrease coverage by 2.44%.
The diff coverage is 32.60%.

@@            Coverage Diff             @@
##           master    #3912      +/-   ##
==========================================
- Coverage   53.25%   50.82%   -2.44%     
==========================================
  Files         821      593     -228     
  Lines       76839    55359   -21480     
==========================================
- Hits        40923    28136   -12787     
+ Misses      32601    24872    -7729     
+ Partials     3315     2351     -964     
Flag Coverage Δ
unittests 50.82% <32.60%> (-2.44%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
fvm/derived/derived_block_data.go 30.00% <0.00%> (ø)
fvm/environment/account_creator.go 39.68% <0.00%> (ø)
fvm/environment/account_info.go 11.30% <0.00%> (-0.31%) ⬇️
fvm/environment/derived_data_invalidator.go 100.00% <ø> (ø)
fvm/environment/system_contracts.go 23.25% <ø> (ø)
fvm/environment/transaction_info.go 67.12% <ø> (+2.56%) ⬆️
fvm/environment/account_key_updater.go 19.30% <3.70%> (-0.39%) ⬇️
fvm/environment/contract_reader.go 43.54% <26.31%> (-3.61%) ⬇️
fvm/environment/account_freezer.go 35.59% <28.57%> (ø)
fvm/environment/contract_updater.go 66.11% <53.33%> (-1.33%) ⬇️
... and 237 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pattyshack pattyshack force-pushed the patrick/address-clean-up branch from e2ee169 to 93bcb44 Compare February 16, 2023 18:18
@pattyshack
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Feb 16, 2023
3912: Clean up address usage (part 1 of many) r=pattyshack a=pattyshack

Prep work for changing address internal representation

The number of changes is getting a bit out of hand.  I'll split up the change into smaller PRs, with the goal of consistently operating on flow.Address whenever possible.

Note that (except for tests) I'm trying to consistently name the address variables based on their types:
 - address is always a flow.Address
 - runtimeAddress is a common.Address
 - cadenceAddress is a cadence.Address

Co-authored-by: Patrick Lee <patrick.lee@dapperlabs.com>
@bors
Copy link
Contributor

bors bot commented Feb 16, 2023

Build failed:

@pattyshack
Copy link
Contributor Author

bors retry

bors bot added a commit that referenced this pull request Feb 16, 2023
3912: Clean up address usage (part 1 of many) r=pattyshack a=pattyshack

Prep work for changing address internal representation

The number of changes is getting a bit out of hand.  I'll split up the change into smaller PRs, with the goal of consistently operating on flow.Address whenever possible.

Note that (except for tests) I'm trying to consistently name the address variables based on their types:
 - address is always a flow.Address
 - runtimeAddress is a common.Address
 - cadenceAddress is a cadence.Address

Co-authored-by: Patrick Lee <patrick.lee@dapperlabs.com>
@bors
Copy link
Contributor

bors bot commented Feb 16, 2023

Build failed:

Prep work for changing address internal representation

The number of changes is getting a bit out of hand.  I'll split up the change
into smaller PRs, with the goal of consistently operating on flow.Address
whenever possible.

Note that (except for tests) I'm trying to consistently name the address
variables based on their types:
 - address is always a flow.Address
 - runtimeAddress is a common.Address
 - cadenceAddress is a cadence.Address
@pattyshack pattyshack force-pushed the patrick/address-clean-up branch from 93bcb44 to c53b3bc Compare February 16, 2023 19:44
@pattyshack
Copy link
Contributor Author

bors merge

@bors bors bot merged commit 9429bde into master Feb 16, 2023
@bors bors bot deleted the patrick/address-clean-up branch February 16, 2023 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants