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

port - [Exec] Use CCF in self-describing mode to encode events (replaces JSON-CDC) #4437

Merged
merged 17 commits into from
Jun 7, 2023

Conversation

janezpodhostnik
Copy link
Contributor

@janezpodhostnik janezpodhostnik commented Jun 7, 2023

this contains #4439 that should be merged first

this is a port of #4417.

I checked everything file by file and they are all identical besides go.mod-s and go.sum-s

fxamacker and others added 14 commits June 7, 2023 17:34
This uses CCF in fully self-describing mode, so events will
encode to about 1/2 the size of JSON-CDC encoding.

Using CCF in partially self-describing mode can encode events
to 1/14 the size of JSON-CDC but that requires other changes
outside of CCF codec.
Since JSON-CDC encodes less type information than CCF, decoding
JSON-CDC and then encoding it in CCF breaks because required
type information is missing.

Fix this by creating Cadence events and then encoding it in CCF,
without involving JSON-CDC.
Currently, JSON-CDC decoded event fields are not explicitly sorted
and field values are obtained by index in some tests.  This isn't
compatible with CCF which explicitly sorts fields for determinism.

Fix fvm tests by obtaining field values by field identifier to
avoid dependency on field order.
Currently, JSON-CDC decoded event fields are not explicitly sorted
and field values are obtained by index in some tests.  This isn't
compatible with CCF which explicitly sorts fields for determinism.

Fix fvm tests by obtaining field values by field identifier to
avoid dependency on field order.
Currently, JSON-CDC decoded event fields are not explicitly sorted
and field values are obtained by index in some tests.  This isn't
compatible with CCF which explicitly sorts fields for determinism.

Fix service events by obtaining field values by field identifier to
avoid dependency on field order.
This update gets Cadence PRs 2528 and 2529 to unblock tests.
@janezpodhostnik janezpodhostnik self-assigned this Jun 7, 2023
@janezpodhostnik janezpodhostnik changed the base branch from master to v0.31 June 7, 2023 18:41
@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2023

FVM Benchstat comparison

This branch with compared with the base branch onflow:v0.31 commit d988ba2

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/get_account_and_get_available_balance-2330ms ± 3%349ms ± 7%+5.63%(p=0.005 n=6+7)
RuntimeTransaction/borrow_array_from_storage-2165ms ± 5%171ms ± 3%+4.00%(p=0.035 n=7+6)
RuntimeNFTBatchTransfer-2144ms ± 3%148ms ± 6%~(p=0.318 n=7+7)
RuntimeTransaction/reference_tx-238.8ms ± 1%42.2ms ±24%~(p=0.343 n=5+7)
RuntimeTransaction/convert_int_to_string-240.5ms ± 8%43.0ms ± 6%~(p=0.132 n=6+6)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-245.8ms ±22%44.0ms ± 7%~(p=0.902 n=7+7)
RuntimeTransaction/get_signer_address-240.9ms ±11%41.5ms ±13%~(p=0.805 n=7+7)
RuntimeTransaction/get_public_account-242.8ms ± 3%46.1ms ±10%~(p=0.234 n=6+7)
RuntimeTransaction/get_account_and_get_balance-2345ms ± 3%345ms ± 9%~(p=1.000 n=6+7)
RuntimeTransaction/get_account_and_get_storage_used-246.0ms ± 4%45.9ms ± 8%~(p=0.805 n=7+7)
RuntimeTransaction/get_account_and_get_storage_capacity-2302ms ± 8%292ms ± 2%~(p=0.165 n=7+7)
RuntimeTransaction/get_signer_vault-249.0ms ± 9%50.1ms ± 4%~(p=0.535 n=7+7)
RuntimeTransaction/get_signer_receiver-261.3ms ± 7%64.1ms ± 4%~(p=0.053 n=7+7)
RuntimeTransaction/transfer_tokens-2265ms ± 5%260ms ± 7%~(p=0.259 n=7+7)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-247.3ms ± 2%48.7ms ± 4%~(p=0.073 n=7+7)
RuntimeTransaction/load_and_save_long_string_on_signers_address-297.4ms ± 4%99.7ms ± 4%~(p=0.097 n=7+7)
RuntimeTransaction/create_new_account-21.03s ± 4%1.03s ± 5%~(p=0.805 n=7+7)
RuntimeTransaction/call_empty_contract_function-243.7ms ± 2%44.2ms ± 8%~(p=0.902 n=7+7)
RuntimeTransaction/emit_event-260.6ms ± 6%63.4ms ± 6%~(p=0.073 n=7+7)
RuntimeTransaction/copy_array_from_storage-2172ms ± 6%174ms ± 3%~(p=0.534 n=7+6)
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-25.75s ± 4%5.82s ± 1%~(p=0.343 n=7+5)
 
alloc/opdelta
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeNFTBatchTransfer-254.9MB ± 4%56.1MB ± 7%~(p=0.165 n=7+7)
RuntimeTransaction/reference_tx-234.7MB ± 2%34.9MB ± 4%~(p=0.731 n=6+7)
RuntimeTransaction/convert_int_to_string-234.8MB ± 4%35.9MB ± 3%~(p=0.073 n=7+7)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-235.7MB ± 4%36.1MB ± 2%~(p=0.456 n=7+7)
RuntimeTransaction/get_signer_address-234.7MB ± 4%35.6MB ± 3%~(p=0.128 n=7+7)
RuntimeTransaction/get_public_account-237.1MB ± 4%37.2MB ± 3%~(p=1.000 n=7+7)
RuntimeTransaction/get_account_and_get_balance-2122MB ± 3%122MB ± 2%~(p=0.902 n=7+7)
RuntimeTransaction/get_account_and_get_available_balance-2108MB ± 3%110MB ± 1%~(p=0.383 n=7+7)
RuntimeTransaction/get_account_and_get_storage_used-237.5MB ± 1%37.1MB ± 6%~(p=0.805 n=7+7)
RuntimeTransaction/get_account_and_get_storage_capacity-2103MB ± 3%104MB ± 1%~(p=0.805 n=7+7)
RuntimeTransaction/get_signer_vault-237.7MB ± 5%37.9MB ± 5%~(p=0.805 n=7+7)
RuntimeTransaction/get_signer_receiver-240.6MB ± 6%41.8MB ± 5%~(p=0.259 n=7+7)
RuntimeTransaction/transfer_tokens-285.9MB ± 5%83.1MB ± 4%~(p=0.073 n=7+7)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-236.4MB ± 4%36.6MB ± 4%~(p=0.710 n=7+7)
RuntimeTransaction/load_and_save_long_string_on_signers_address-253.3MB ± 5%54.1MB ± 3%~(p=0.456 n=7+7)
RuntimeTransaction/create_new_account-2185MB ± 3%182MB ± 1%~(p=0.128 n=7+7)
RuntimeTransaction/call_empty_contract_function-236.2MB ± 4%36.3MB ± 2%~(p=0.731 n=7+6)
RuntimeTransaction/emit_event-241.0MB ± 7%41.7MB ± 7%~(p=0.456 n=7+7)
RuntimeTransaction/borrow_array_from_storage-268.0MB ± 4%69.7MB ± 3%~(p=0.234 n=7+6)
RuntimeTransaction/copy_array_from_storage-281.4MB ± 1%81.3MB ± 3%~(p=0.902 n=7+7)
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-21.24GB ± 1%1.23GB ± 1%−0.88%(p=0.038 n=7+7)
 
allocs/opdelta
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/emit_event-2140k ± 0%143k ± 0%+2.77%(p=0.001 n=7+7)
RuntimeNFTBatchTransfer-2273k ± 1%272k ± 1%~(p=0.053 n=7+7)
RuntimeTransaction/get_account_and_get_storage_capacity-21.15M ± 0%1.15M ± 0%−0.01%(p=0.001 n=7+6)
RuntimeTransaction/get_account_and_get_available_balance-21.28M ± 0%1.28M ± 0%−0.01%(p=0.001 n=7+7)
RuntimeTransaction/get_account_and_get_balance-21.33M ± 0%1.33M ± 0%−0.01%(p=0.001 n=7+7)
RuntimeTransaction/borrow_array_from_storage-2364k ± 0%364k ± 0%−0.04%(p=0.001 n=7+7)
RuntimeTransaction/copy_array_from_storage-2297k ± 0%296k ± 0%−0.05%(p=0.001 n=7+7)
RuntimeTransaction/get_signer_receiver-2216k ± 0%215k ± 0%−0.06%(p=0.001 n=7+7)
RuntimeTransaction/load_and_save_long_string_on_signers_address-2218k ± 0%218k ± 0%−0.06%(p=0.001 n=7+7)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-2135k ± 0%135k ± 0%−0.10%(p=0.001 n=7+7)
RuntimeTransaction/get_signer_vault-2129k ± 0%129k ± 0%−0.10%(p=0.001 n=7+7)
RuntimeTransaction/get_account_and_get_storage_used-2124k ± 0%124k ± 0%−0.12%(p=0.001 n=7+7)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-2108k ± 0%108k ± 0%−0.12%(p=0.001 n=7+6)
RuntimeTransaction/get_public_account-2113k ± 0%113k ± 0%−0.12%(p=0.001 n=7+7)
RuntimeTransaction/convert_int_to_string-296.7k ± 0%96.6k ± 0%−0.13%(p=0.001 n=7+7)
RuntimeTransaction/get_signer_address-288.7k ± 0%88.5k ± 0%−0.14%(p=0.001 n=7+7)
RuntimeTransaction/call_empty_contract_function-2102k ± 0%102k ± 0%−0.14%(p=0.001 n=7+7)
RuntimeTransaction/reference_tx-284.5k ± 0%84.4k ± 0%−0.15%(p=0.001 n=7+7)
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-217.8M ± 0%17.7M ± 0%−0.19%(p=0.001 n=7+7)
pkg:github.com/onflow/flow-go/fvm goos:linux goarch:amd64
RuntimeTransaction/create_new_account-22.36M ± 0%2.35M ± 0%−0.24%(p=0.001 n=7+7)
RuntimeTransaction/transfer_tokens-2866k ± 0%858k ± 0%−0.93%(p=0.001 n=7+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-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/get_signer_address-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/get_public_account-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_balance-246.4k ± 0%46.4k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/get_signer_vault-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/get_signer_receiver-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/transfer_tokens-238.1k ± 0%38.1k ± 0%~(all equal)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-238.3k ± 0%38.3k ± 0%~(all equal)
RuntimeTransaction/load_and_save_long_string_on_signers_address-242.1k ± 0%42.1k ± 0%~(all equal)
RuntimeTransaction/create_new_account-284.3k ± 0%84.3k ± 0%~(all equal)
RuntimeTransaction/call_empty_contract_function-238.3k ± 0%38.3k ± 0%~(all equal)
RuntimeTransaction/emit_event-238.3k ± 0%38.3k ± 0%~(all equal)
RuntimeTransaction/borrow_array_from_storage-243.4k ± 0%43.4k ± 0%~(p=0.070 n=7+7)
RuntimeTransaction/copy_array_from_storage-243.4k ± 0%43.4k ± 0%~(p=0.070 n=7+7)
 
us/txdelta
pkg:github.com/onflow/flow-go/engine/execution/computation goos:linux goarch:amd64
ComputeBlock/16/cols/128/txes-22.81k ± 4%2.84k ± 1%~(p=0.343 n=7+5)
 

@janezpodhostnik janezpodhostnik marked this pull request as ready for review June 7, 2023 19:46
@codecov-commenter
Copy link

codecov-commenter commented Jun 7, 2023

Codecov Report

Merging #4437 (e672c33) into v0.31 (d988ba2) will increase coverage by 0.47%.
The diff coverage is 79.12%.

@@            Coverage Diff             @@
##            v0.31    #4437      +/-   ##
==========================================
+ Coverage   53.67%   54.15%   +0.47%     
==========================================
  Files         887      887              
  Lines       82528    83653    +1125     
==========================================
+ Hits        44296    45301    +1005     
- Misses      34748    34834      +86     
- Partials     3484     3518      +34     
Flag Coverage Δ
unittests 54.15% <79.12%> (+0.47%) ⬆️

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

Impacted Files Coverage Δ
model/convert/service_event.go 47.59% <45.34%> (+2.99%) ⬆️
utils/unittest/service_events_fixtures.go 84.14% <98.00%> (+84.14%) ⬆️
engine/execution/rpc/engine.go 51.60% <100.00%> (+0.52%) ⬆️
fvm/environment/event_encoder.go 100.00% <100.00%> (ø)

... and 14 files with indirect coverage changes

Copy link
Member

@fxamacker fxamacker left a comment

Choose a reason for hiding this comment

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

Thanks for porting this! LGTM!

@janezpodhostnik janezpodhostnik merged commit 8d32e5e into v0.31 Jun 7, 2023
@janezpodhostnik janezpodhostnik deleted the janez/port-4417 branch June 7, 2023 20:35
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