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

[Networking] Upgrades pubsub version of libp2p #2515

Merged
merged 2 commits into from May 31, 2022

Conversation

yhassanzadeh13
Copy link
Contributor

@yhassanzadeh13 yhassanzadeh13 commented May 30, 2022

Upgrading pubsub version of libp2p to account for the recent change we implemented on the original pubsub repository: libp2p/go-libp2p-pubsub#483

@github-actions
Copy link
Contributor

github-actions bot commented May 30, 2022

FVM Benchstat comparison

This branch with compared with the base branch onflow:master commit 98dc45b

The command (for i in {1..N}; do go test ./fvm --bench . --tags relic -shuffle=on; done) was used.

Bench tests were run a total of 7 times on each branch.

Collapsed results for better readability

old.txtnew.txt
time/opdelta
RuntimeTransaction/reference_tx-229.9ms ± 0%29.6ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/convert_int_to_string-231.5ms ± 0%31.0ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-232.9ms ± 0%33.2ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_signer_address-230.5ms ± 0%30.3ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_public_account-233.0ms ± 0%33.0ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_account_and_get_balance-2512ms ± 0%514ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_account_and_get_available_balance-2418ms ± 0%409ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_account_and_get_storage_used-237.7ms ± 0%38.2ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_account_and_get_storage_capacity-2382ms ± 0%380ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_signer_vault-239.5ms ± 0%39.9ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/get_signer_receiver-250.7ms ± 0%52.1ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/transfer_tokens-2212ms ± 0%189ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-236.6ms ± 0%37.0ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/load_and_save_long_string_on_signers_address-278.7ms ± 0%79.6ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/create_new_account-21.11s ± 0%1.13s ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/call_empty_contract_function-233.4ms ± 0%32.9ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/emit_event-248.2ms ± 0%47.3ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/borrow_array_from_storage-2117ms ± 0%118ms ± 0%~(p=1.000 n=1+1)
RuntimeTransaction/copy_array_from_storage-2116ms ± 0%116ms ± 0%~(p=1.000 n=1+1)
 
computationdelta
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-2802 ± 0%802 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-22.40k ± 0%2.40k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-2402 ± 0%402 ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-21.10k ± 0%1.10k ± 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
RuntimeTransaction/reference_tx-247.8k ± 0%47.8k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string-247.8k ± 0%47.8k ± 0%~(all equal)
RuntimeTransaction/convert_int_to_string_and_concatenate_it-247.8k ± 0%47.8k ± 0%~(all equal)
RuntimeTransaction/get_signer_address-247.8k ± 0%47.8k ± 0%~(all equal)
RuntimeTransaction/get_public_account-247.8k ± 0%47.8k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_balance-216.8M ± 0%16.8M ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_available_balance-25.33M ± 0%5.33M ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_used-250.6k ± 0%50.6k ± 0%~(all equal)
RuntimeTransaction/get_account_and_get_storage_capacity-25.32M ± 0%5.32M ± 0%~(all equal)
RuntimeTransaction/get_signer_vault-249.1k ± 0%49.1k ± 0%~(all equal)
RuntimeTransaction/get_signer_receiver-249.5k ± 0%49.5k ± 0%~(all equal)
RuntimeTransaction/transfer_tokens-250.2k ± 0%50.2k ± 0%~(all equal)
RuntimeTransaction/load_and_save_empty_string_on_signers_address-251.3k ± 0%51.3k ± 0%~(all equal)
RuntimeTransaction/load_and_save_long_string_on_signers_address-256.3k ± 0%56.3k ± 0%~(all equal)
RuntimeTransaction/create_new_account-28.47M ± 0%8.47M ± 0%~(all equal)
RuntimeTransaction/call_empty_contract_function-248.0k ± 0%48.0k ± 0%~(all equal)
RuntimeTransaction/emit_event-248.0k ± 0%48.0k ± 0%~(all equal)
RuntimeTransaction/borrow_array_from_storage-254.2k ± 0%54.2k ± 0%~(all equal)
RuntimeTransaction/copy_array_from_storage-254.2k ± 0%54.2k ± 0%~(all equal)
 

@yhassanzadeh13 yhassanzadeh13 marked this pull request as ready for review May 30, 2022 17:41
@yhassanzadeh13 yhassanzadeh13 merged commit 12250a6 into master May 31, 2022
@yhassanzadeh13 yhassanzadeh13 deleted the yahya/upgrading-pubsub-version branch May 31, 2022 16:56
zhangchiqing added a commit that referenced this pull request Jun 1, 2022
* Remove payload key comparison in Payload.Equals()

Payload key is auxiliary data and shouldn't be included in payload
equality test.

Although payload key was used in this comparison for about a year,
this is an edge case that isn't triggered by our current use of MTrie.
So this change isn't expected to cause compatibility issues.

* Add comments to say payload key shouldn't change

In Mtrie, it is expected that for a specific path,
the payload's key never changes.

* Add Payload.ValueEquals() to compare payload values

Restore Payload.Equals() to compare payload keys and values.

Add Payload.ValueEquals() to compare payload values.

Add and update tests.

* fix race condition

* skip long running test

* outdated blocks are logged as errors, despite being expected during normal operations

* add jobqueue README

* addressed reviewer feedback

* Add module that requests execution data for newly sealed blocks

* Add timeout to blob fetch

* Add edr to staked access node, and improve backfilling

* Updates to access node config

* Refactor requester to use queues and notifications

* removed old requester files

* fix imports in builders

* fixes from testing

* refactor from review

* always cache new execution datas

* throw irrecoverable on invalid blobs

* cleanup comments and logging

* merge notifications and cache and refactor into heap

* add config, add restart handling, code cleanup

* improve error handling, cleanup refactoring

* cleanup error handling, refactor status

* fix unittest import cycle, and some cleanup

* make more options configurable

* cleanup comments and naming

* add metrics

* remove metrics comment

* add tests and fix bugs for status module

* fix bug in status duplicate height processing

* fix scoping in edr setup

* use a single implementation of blob_service

* add testing for RestartableComponent

* add more comments and unittests

* update check function prototype

* convert to use jobqueue

* fix rebase conflict

* Update module/state_synchronization/requester/execution_data_requester.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* add unit tests for notification_heap

* Move ExecutionDataRequester interface to state_synchronization package

* improve requester comments

* cleanup requester bootstrapping

* add tests for ExecutionDataService

* remove unused consumer member, rename wrapped structs to readydoneaware

* add unittest for starting from halted state

* Update module/state_synchronization/requester/execution_data_requester.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* Update module/state_synchronization/requester/execution_data_requester.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* switch to storage.headers

* save halted reason, improve logging

* move pause/resume to jobqueue

* Remove min heap from status

* add unittest for ReadyDoneAwareConsumer

* remove halts, switch to stateless notification status

* refactor datastore check into separate struct, add unit tests

* Apply suggestions from code review

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* remove halted metric

* refactor start block to be more explicit

* move finalized block reader and sealed header reader to jobqueue namespace

* reorder consumer args, add pre/post notifiers

* remove execution data cache, move ed get into reader

* fix sealed reader test, improve comments

* fix datastore check tests and components lifecycle

* rename ReadyDoneAwareConsumer to ComponentConsumer

* fix linting issues

* increase timeouts in check datastore tests

* comment improvements from review

* remove datastore checker

* remove now used execution data service check method

* add more details to execution data service get comments

* fix tests after rebase

* fix lint errors

* cleanup unused arguments

* apply comment update from review

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* add requester integration test

* add missing lock in module/jobqueue/consumer.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* add lock to jobqueue consumer size()

* remove changes to blob service, and create new test blobservice for local db only

* fix potential hang in consumer startup, update cli arg description

* update storage namespaces in access_node_builder

* Apply suggestions from code review

Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* update pull vs push

* Update module/component/component.go

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* Remove unused codeowners

* Add atree reporter to execution state extraction

Report:
- number of FVM/storage/slab payloads
- number of slab array meta/data payloads
- number of slab map meta/data payloads
- number of first level hash collision groups in slab maps
- number of first level hash collisions in each collision group
- etc.

* Fix lint error

* validate guarantors

* handle error from FindGuarantors

* add comment

* add go workspace files to .dockerignore and .gitignore

* Add Ledger.GetSingleValue for speedups and memory

- Added Ledger.GetSingleValue() to improve speed, alloc/op, and
allocs/op
- Modified delta.View.readFunc to use Ledger.GetSingleValue()
- Optimized existing mtrie batch read for single path

Bench comparision between current Ledger.Get() vs
Ledger.GetSingleValue()

Benchstat results:
name                           old time/op    new time/op    delta
LedgerGetOneValue/batch_get-4    6.54µs ± 0%    5.24µs ± 0%  -19.92%
(p=0.000 n=9+10)

name                           old alloc/op   new alloc/op   delta
LedgerGetOneValue/batch_get-4    1.74kB ± 0%    1.62kB ± 0%   -6.85%
(p=0.000 n=10+10)

name                           old allocs/op  new allocs/op  delta
LedgerGetOneValue/batch_get-4      21.0 ± 0%      15.0 ± 0%  -28.57%
(p=0.000 n=10+10)

Bench comparision between optimized new Ledger.Get() vs
Ledger.GetSingleValue()

Benchstat results:
name                           old time/op    new time/op    delta
LedgerGetOneValue/batch_get-4    5.70µs ± 0%    5.23µs ± 0%   -8.24%
(p=0.000 n=9+10)

name                           old alloc/op   new alloc/op   delta
LedgerGetOneValue/batch_get-4    1.69kB ± 0%    1.62kB ± 0%   -3.79%
(p=0.000 n=10+10)

name                           old allocs/op  new allocs/op  delta
LedgerGetOneValue/batch_get-4      18.0 ± 0%      15.0 ± 0%  -16.67%
(p=0.000 n=10+10)

* Update ledger mock

* Check Mtrie root integrity after read in tests

* add comment

* Apply suggestions from code review

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* Reduce allocs/op by 79% in ledger read

Change Forest.Read to return []ledger.Value without deep copying
payload keys.  This avoids 4 heap allocation per key.

This change doesn't affect the caller (Ledger.Get) because it
discards the payload keys.

name        old time/op    new time/op    delta
TrieRead-4     524µs ± 1%     420µs ± 1%  -19.77%  (p=0.000 n=10+10)

name        old alloc/op   new alloc/op   delta
TrieRead-4     190kB ± 0%      95kB ± 0%  -50.04%  (p=0.000 n=10+10)

name        old allocs/op  new allocs/op  delta
TrieRead-4     1.52k ± 0%     0.32k ± 0%  -79.17%  (p=0.000 n=10+10)

* Fix tests

* Update Forest.Read() callers to use new API

* Speedup and reduce allocs/op in ledger single read

Changed Forest.ReadSingleValue to return ledger.Value without deep
copying payload keys.  This avoid 4 heap allocation per key.

This change doesn't affect the caller (Ledger.GetSingleValue) because
it discards the payload key.

* Move a line outside for-loop in readSinglePayload

This optimization improves speed by 0.49% on Haswell CPU.

Thanks for suggestion @tarakby.

Co-authored-by: Tarak Ben Youssef <tarak.benyoussef@dapperlabs.com>

* Read memory limit from state

* Fixed memory metering crasher and tests

lint

* fixed tests

* add tests for script mutations

* update cadence dep

* mod tidy

* go mod tidy integration

* do not split always fatal error

* add tests to manager for discarding computations in scripts

* fix test

* assume service account exists

* update test for discarding storage mutations

* test fail on error

* if memory limit not set in service acct then default to max

* lint

* better fix: return was missing

* accidentally added files

* removed finished TODO

* handling unexpected error

* cleanup unused fields from loader

* add test cases

* update tests

* fix lint

* fix error wrapping

* upload log level for memory weights

* add test

* [Fix] Adding sync request message as a cluster type message (#2495)

* adds sync request as an authorized cluster channel type

* use isClusterChannel and add test

* fix test vars and comments

* Update topic_validator_test.go

* Update topic_validator_test.go

* explicitly check cluster msg codes, handle cluster sync request

Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>

* upgrade cadence to secure-cadence-m8

* update error handling

* fix test case

* flaky tests fix test async uploader - Test_AsyncUploader/stopping_component_stops_retrying (#2453)

* fixed flaky test - WIP - Test_AsyncUploader/stopping_component_stops_retrying

* fix flaky test - Test_AsyncUploader/stopping_component_stops_retrying - added wait group to ensure upload started before shutting down component

* updated comments, renamed wait group to be more clear

* minor refactoring - var renaming

* added more explicit retry mechanism to better test that retry is not working when component is shutting down

* uses unit.RequireCloseBefore() to check closed channel

* added clarification comments on why test is trying to increment callCount more than once

* Review updates

* Test fix

* change test fix

* chore(tests): fix init-light

* chore(tests): mark localnet and benchmarks owned by the performance team

* fix test case

* add blockheight to transaction result response

* fix test case

* add test case

* handle guarantee.ReferenceBlockID not found

* fix assigner tests

* fix linter

* fix reader tests

* upgrade cadence to secure-cadence-m9

* tidy go mods

* fix naming in limit script

* Upgrades libp2p pubsub version (#2515)

* generate mocks, make tidy

* error fix

* go mod update

* update mocks

* Suggestion for PR #2462 (#2517)

* consolidated errors from signer indices decoding into a single error type

* fix error type and tests

* fix typo

Co-authored-by: Leo Zhang (zhangchiqing) <zhangchiqing@gmail.com>

* Apply suggestions from code review

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

* fix linter

* exit validation early for empty collections

* lint

* Update state/cluster/badger/mutator.go

* fix tests

* update test

* extract decodeSignerIndices

* small refactor

* update to Cadence 8b113c539a2c

* block nil check

* workaround for handling hotstuff error

* check that reference block is known

otherwise the cluster committee may return an error as it is unable to
determine the identities for the block (determined by the reference
block)

* height is already set here

* add blockheight

* blockheight for historical transaction request

* test blockheight equivalence

* upgrade flow to v0.3.1

* fix linter

Co-authored-by: Faye Amacker <33205765+fxamacker@users.noreply.github.com>
Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: Gregor G <75445744+sideninja@users.noreply.github.com>
Co-authored-by: Tarak Ben Youssef <50252200+tarakby@users.noreply.github.com>
Co-authored-by: Alexey Ivanov <SaveTheRbtz@GMail.com>
Co-authored-by: Tarak Ben Youssef <tarak.benyoussef@dapperlabs.com>
Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
Co-authored-by: robert <robert.davidson@forecastfoundation.org>
Co-authored-by: Daniel Sainati <sainatidaniel@gmail.com>
Co-authored-by: Robert E. Davidson III <45945043+robert-e-davidson3@users.noreply.github.com>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Co-authored-by: Yahya Hassanzadeh <yahya@dapperlabs.com>
Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>
Co-authored-by: Misha <misha.rybalov@dapperlabs.com>
Co-authored-by: lolpuddle <oocean.cheung@gmail.com>
Co-authored-by: Jan Bernatik <jan.bernatik@dapperlabs.com>
Co-authored-by: Bastian Müller <bastian@axiomzen.co>
zhangchiqing added a commit that referenced this pull request Jun 2, 2022
* always cache new execution datas

* throw irrecoverable on invalid blobs

* cleanup comments and logging

* merge notifications and cache and refactor into heap

* add config, add restart handling, code cleanup

* improve error handling, cleanup refactoring

* cleanup error handling, refactor status

* fix unittest import cycle, and some cleanup

* make more options configurable

* cleanup comments and naming

* add metrics

* remove metrics comment

* add tests and fix bugs for status module

* fix bug in status duplicate height processing

* fix scoping in edr setup

* use a single implementation of blob_service

* add testing for RestartableComponent

* add more comments and unittests

* update check function prototype

* convert to use jobqueue

* fix rebase conflict

* Update module/state_synchronization/requester/execution_data_requester.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* add unit tests for notification_heap

* Move ExecutionDataRequester interface to state_synchronization package

* improve requester comments

* cleanup requester bootstrapping

* add tests for ExecutionDataService

* remove unused consumer member, rename wrapped structs to readydoneaware

* add unittest for starting from halted state

* Update module/state_synchronization/requester/execution_data_requester.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* Update module/state_synchronization/requester/execution_data_requester.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* switch to storage.headers

* save halted reason, improve logging

* move pause/resume to jobqueue

* Remove min heap from status

* add unittest for ReadyDoneAwareConsumer

* remove halts, switch to stateless notification status

* refactor datastore check into separate struct, add unit tests

* Apply suggestions from code review

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* remove halted metric

* refactor start block to be more explicit

* move finalized block reader and sealed header reader to jobqueue namespace

* reorder consumer args, add pre/post notifiers

* remove execution data cache, move ed get into reader

* fix sealed reader test, improve comments

* fix datastore check tests and components lifecycle

* rename ReadyDoneAwareConsumer to ComponentConsumer

* fix linting issues

* increase timeouts in check datastore tests

* comment improvements from review

* remove datastore checker

* remove now used execution data service check method

* add more details to execution data service get comments

* fix tests after rebase

* fix lint errors

* cleanup unused arguments

* apply comment update from review

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* add requester integration test

* add missing lock in module/jobqueue/consumer.go

Co-authored-by: Leo Zhang <zhangchiqing@gmail.com>

* add lock to jobqueue consumer size()

* remove changes to blob service, and create new test blobservice for local db only

* fix potential hang in consumer startup, update cli arg description

* update storage namespaces in access_node_builder

* Apply suggestions from code review

Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>

* update pull vs push

* Update module/component/component.go

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* temp

* switched to component manager / queues and workers

* comments

* removed 'wait for engine to be done processing' assertions

* go fmt

* go fmt, comments

* switched to one message handler

* removed queue metrics, updated queue capacity

* temporary fix to unit tests

* undo removing requestMissingCollections

* temp

* check pipeline

* Remove unused codeowners

* option #1  flag

* trying to fix access_test.go

* trying to fix access_test.go

* trying to fix access_test.go

* trying to fix access_test.go

* trying to fix access_test.go

* it's working locallygit add *.go!

* Add atree reporter to execution state extraction

Report:
- number of FVM/storage/slab payloads
- number of slab array meta/data payloads
- number of slab map meta/data payloads
- number of first level hash collision groups in slab maps
- number of first level hash collisions in each collision group
- etc.

* Fix lint error

* switch 'IsTest' to 'DisableProcessBackground', and component shutdown test

* fix linting error

* validate guarantors

* handle error from FindGuarantors

* add comment

* add go workspace files to .dockerignore and .gitignore

* Add Ledger.GetSingleValue for speedups and memory

- Added Ledger.GetSingleValue() to improve speed, alloc/op, and
allocs/op
- Modified delta.View.readFunc to use Ledger.GetSingleValue()
- Optimized existing mtrie batch read for single path

Bench comparision between current Ledger.Get() vs
Ledger.GetSingleValue()

Benchstat results:
name                           old time/op    new time/op    delta
LedgerGetOneValue/batch_get-4    6.54µs ± 0%    5.24µs ± 0%  -19.92%
(p=0.000 n=9+10)

name                           old alloc/op   new alloc/op   delta
LedgerGetOneValue/batch_get-4    1.74kB ± 0%    1.62kB ± 0%   -6.85%
(p=0.000 n=10+10)

name                           old allocs/op  new allocs/op  delta
LedgerGetOneValue/batch_get-4      21.0 ± 0%      15.0 ± 0%  -28.57%
(p=0.000 n=10+10)

Bench comparision between optimized new Ledger.Get() vs
Ledger.GetSingleValue()

Benchstat results:
name                           old time/op    new time/op    delta
LedgerGetOneValue/batch_get-4    5.70µs ± 0%    5.23µs ± 0%   -8.24%
(p=0.000 n=9+10)

name                           old alloc/op   new alloc/op   delta
LedgerGetOneValue/batch_get-4    1.69kB ± 0%    1.62kB ± 0%   -3.79%
(p=0.000 n=10+10)

name                           old allocs/op  new allocs/op  delta
LedgerGetOneValue/batch_get-4      18.0 ± 0%      15.0 ± 0%  -16.67%
(p=0.000 n=10+10)

* Update ledger mock

* Check Mtrie root integrity after read in tests

* add comment

* Apply suggestions from code review

Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>

* Reduce allocs/op by 79% in ledger read

Change Forest.Read to return []ledger.Value without deep copying
payload keys.  This avoids 4 heap allocation per key.

This change doesn't affect the caller (Ledger.Get) because it
discards the payload keys.

name        old time/op    new time/op    delta
TrieRead-4     524µs ± 1%     420µs ± 1%  -19.77%  (p=0.000 n=10+10)

name        old alloc/op   new alloc/op   delta
TrieRead-4     190kB ± 0%      95kB ± 0%  -50.04%  (p=0.000 n=10+10)

name        old allocs/op  new allocs/op  delta
TrieRead-4     1.52k ± 0%     0.32k ± 0%  -79.17%  (p=0.000 n=10+10)

* Fix tests

* Update Forest.Read() callers to use new API

* Speedup and reduce allocs/op in ledger single read

Changed Forest.ReadSingleValue to return ledger.Value without deep
copying payload keys.  This avoid 4 heap allocation per key.

This change doesn't affect the caller (Ledger.GetSingleValue) because
it discards the payload key.

* Move a line outside for-loop in readSinglePayload

This optimization improves speed by 0.49% on Haswell CPU.

Thanks for suggestion @tarakby.

Co-authored-by: Tarak Ben Youssef <tarak.benyoussef@dapperlabs.com>

* Read memory limit from state

* Fixed memory metering crasher and tests

lint

* fixed tests

* add tests for script mutations

* update cadence dep

* mod tidy

* go mod tidy integration

* do not split always fatal error

* add tests to manager for discarding computations in scripts

* fix test

* assume service account exists

* update test for discarding storage mutations

* test fail on error

* if memory limit not set in service acct then default to max

* lint

* better fix: return was missing

* accidentally added files

* removed finished TODO

* handling unexpected error

* cleanup unused fields from loader

* add test cases

* update tests

* fix lint

* fix error wrapping

* upload log level for memory weights

* add test

* [Fix] Adding sync request message as a cluster type message (#2495)

* adds sync request as an authorized cluster channel type

* use isClusterChannel and add test

* fix test vars and comments

* Update topic_validator_test.go

* Update topic_validator_test.go

* explicitly check cluster msg codes, handle cluster sync request

Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>

* upgrade cadence to secure-cadence-m8

* update error handling

* fix test case

* flaky tests fix test async uploader - Test_AsyncUploader/stopping_component_stops_retrying (#2453)

* fixed flaky test - WIP - Test_AsyncUploader/stopping_component_stops_retrying

* fix flaky test - Test_AsyncUploader/stopping_component_stops_retrying - added wait group to ensure upload started before shutting down component

* updated comments, renamed wait group to be more clear

* minor refactoring - var renaming

* added more explicit retry mechanism to better test that retry is not working when component is shutting down

* uses unit.RequireCloseBefore() to check closed channel

* added clarification comments on why test is trying to increment callCount more than once

* Review updates

* Test fix

* change test fix

* chore(tests): fix init-light

* use seeded math rand and log the seed

* bump the samples in a statistical test

* chore(tests): do not go through the resolver for localhost

* chore(tests): mark localnet and benchmarks owned by the performance team

* fix test case

* add blockheight to transaction result response

* fix test case

* add test case

* handle guarantee.ReferenceBlockID not found

* fix assigner tests

* fix linter

* fix reader tests

* upgrade cadence to secure-cadence-m9

* tidy go mods

* chore(tests): remove unused batchLoadGenerator

* added ctx timeout, constant var for queue size, used ProcessLocal in OnFinalized block

* fix naming in limit script

* added TearDownTest to cancel context

* cancel context

* processAvailableMessage approach

* go fmt

* go fmt

* fix lint error

* Upgrades libp2p pubsub version (#2515)

* generate mocks, make tidy

* error fix

* go mod update

* update mocks

* Suggestion for PR #2462 (#2517)

* consolidated errors from signer indices decoding into a single error type

* fix error type and tests

* fix typo

Co-authored-by: Leo Zhang (zhangchiqing) <zhangchiqing@gmail.com>

* Apply suggestions from code review

Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>

* fix linter

* exit validation early for empty collections

* lint

* Update state/cluster/badger/mutator.go

* fix tests

* update test

* extract decodeSignerIndices

* small refactor

* update to Cadence 8b113c539a2c

* block nil check

* workaround for handling hotstuff error

* check that reference block is known

otherwise the cluster committee may return an error as it is unable to
determine the identities for the block (determined by the reference
block)

* removed disableProcessBackground flag

* height is already set here

* add blockheight

* blockheight for historical transaction request

* test blockheight equivalence

* upgrade flow to v0.3.1 (#2534)

* add explicit mapping of cluster prefix -> message codes

Co-authored-by: Peter Argue <89119817+peterargue@users.noreply.github.com>
Co-authored-by: Simon Zhu <simon.zsiyan@gmail.com>
Co-authored-by: Jordan Schalm <jordan@dapperlabs.com>
Co-authored-by: danielholmes839 <danielh839@gmail.com>
Co-authored-by: Gregor G <75445744+sideninja@users.noreply.github.com>
Co-authored-by: Tarak Ben Youssef <50252200+tarakby@users.noreply.github.com>
Co-authored-by: Faye Amacker <33205765+fxamacker@users.noreply.github.com>
Co-authored-by: Alexey Ivanov <SaveTheRbtz@GMail.com>
Co-authored-by: Tarak Ben Youssef <tarak.benyoussef@dapperlabs.com>
Co-authored-by: Janez Podhostnik <janez.podhostnik@gmail.com>
Co-authored-by: robert <robert.davidson@forecastfoundation.org>
Co-authored-by: Daniel Sainati <sainatidaniel@gmail.com>
Co-authored-by: Robert E. Davidson III <45945043+robert-e-davidson3@users.noreply.github.com>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Co-authored-by: Yahya Hassanzadeh <yahya@dapperlabs.com>
Co-authored-by: Khalil Claybon <khalil.claybon@dapperlabs.com>
Co-authored-by: Misha <misha.rybalov@dapperlabs.com>
Co-authored-by: lolpuddle <oocean.cheung@gmail.com>
Co-authored-by: Jan Bernatik <jan.bernatik@dapperlabs.com>
Co-authored-by: Daniel Holmes <43529937+danielholmes839@users.noreply.github.com>
Co-authored-by: Alexander Hentschel <alex.hentschel@axiomzen.co>
Co-authored-by: Bastian Müller <bastian@axiomzen.co>
Co-authored-by: Kan Zhang <kan@axiomzen.co>
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.

None yet

3 participants