Finschia / cometbft

CometBFT (fork of Tendermint Core): A distributed, Byzantine fault-tolerant, deterministic state machine replication engine
https://docs.cometbft.com
Apache License 2.0
0 stars 0 forks source link

improve performance of a crypt marshaller (ostracon#177) #2

Closed 0Tech closed 10 months ago

0Tech commented 1 year ago

The subject of the triage:

loloicci commented 12 months ago

This is a PR for performance test and this PR says

Please note that it's not production ready quality

If we need this improvement in the main branch, it may need other PRs to improve it.

Mdaiki0730 commented 11 months ago

Performance seems to be improved, so we can create a PR after considering what we need for main branch.

loloicci commented 11 months ago

@Mdaiki0730

Performance seems to be improved, so we can create a PR after considering what we need for main branch.

I think it is worth merging into the main form the view point of performance. But, do you know why it's not production-ready quality? Or, do you know some improvements for it?

Mdaiki0730 commented 11 months ago

@loloicci The custom marshaler code is duplicated and needs to be packaged.

we need more test code for production.

As for the further tests mentioned in the PR, I don't know what to do.

Mdaiki0730 commented 11 months ago

By the way, the relevant parts of the crypto package have changed significantly in cometbft v0.38.0. Looks like it was changed in tendermint#4950. cometbft v0.38.0: https://github.com/cometbft/cometbft/blob/v0.38.0/crypto/ed25519/ed25519.go#L68-L70 PR: https://github.com/tendermint/tendermint/pull/4950

Ostracon#177 doesn't seem to be necessary since it returns a byte slice directly instead of an amino-marshalled byte slice.

loloicci commented 10 months ago

Thank you @Mdaiki0730 . I close this issue.