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.

Sign up for GitHub

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

Jump to bottom

Remove protos, replace with custom byte serialization #1170

Merged
merged 27 commits into from
Aug 16, 2019
Merged

Remove protos, replace with custom byte serialization #1170

merged 27 commits into from
Aug 16, 2019

Conversation

ilblackdragon
Copy link
Member

@ilblackdragon ilblackdragon commented Aug 14, 2019 •
edited
Loading

Fixes #1166
Fixes #1083

@evgenykuzyakov evgenykuzyakov added the C-housekeeping Category: Refactoring, cleanups, code quality label Aug 15, 2019
@ilblackdragon
Copy link
Member Author

Serialization benchmarks:

test deserialize_account ... bench:          49 ns/iter (+/- 9)
test deserialize_block   ... bench:   2,536,574 ns/iter (+/- 313,831)
test deserialize_tx      ... bench:       2,400 ns/iter (+/- 285)
test serialize_account   ... bench:         271 ns/iter (+/- 87)
test serialize_block     ... bench:     128,271 ns/iter (+/- 17,350)
test serialize_tx        ... bench:         591 ns/iter (+/- 302)

@weekly-digest weekly-digest bot mentioned this pull request Aug 16, 2019
Closed
evgenykuzyakov
evgenykuzyakov requested changes Aug 16, 2019
View reviewed changes
Copy link
Collaborator

@evgenykuzyakov evgenykuzyakov left a comment

Choose a reason for hiding this comment

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

Also, run clippy on it. A lot of things would popup like .map(|a| a.into())

chain/client/src/view_client.rs Outdated Show resolved Hide resolved
chain/network/src/codec.rs Outdated Show resolved Hide resolved
chain/network/src/codec.rs Outdated Show resolved Hide resolved
core/nbor/src/de/mod.rs Outdated Show resolved Hide resolved
core/primitives/src/block.rs Show resolved Hide resolved
core/primitives/src/rpc.rs Outdated Show resolved Hide resolved
core/primitives/src/rpc.rs Outdated Show resolved Hide resolved
runtime/runtime/src/state_viewer.rs Outdated Show resolved Hide resolved
test-utils/state-viewer/src/main.rs Outdated Show resolved Hide resolved
test-utils/testlib/src/user/runtime_user.rs Outdated Show resolved Hide resolved
@evgenykuzyakov evgenykuzyakov mentioned this pull request Aug 16, 2019
Merged
@MaksymZavershynskyi
Copy link
Contributor

Moving borsh to https://github.com/nearprotocol/borsh

@ilblackdragon ilblackdragon force-pushed the remove-protos-fix-nearlib branch from 9f91ab4 to 9b0224c Compare August 16, 2019 20:46
@ilblackdragon
Copy link
Member Author

Latest bench with BORsh:

test deserialize_account ... bench:          37 ns/iter (+/- 3)
test deserialize_block   ... bench:   2,120,731 ns/iter (+/- 433,084)
test deserialize_tx      ... bench:       2,027 ns/iter (+/- 168)
test serialize_account   ... bench:           0 ns/iter (+/- 0)
test serialize_block     ... bench:     338,078 ns/iter (+/- 28,825)
test serialize_tx        ... bench:         228 ns/iter (+/- 24)

@ilblackdragon ilblackdragon merged commit 5490abb into staging Aug 16, 2019
@ilblackdragon ilblackdragon deleted the remove-protos-fix-nearlib branch August 16, 2019 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evgenykuzyakov evgenykuzyakov evgenykuzyakov approved these changes

Assignees
No one assigned
Labels
C-housekeeping Category: Refactoring, cleanups, code quality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ilblackdragon @MaksymZavershynskyi @evgenykuzyakov @coqbot @vgrichina