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

[x/programs] serialize/deserialize using serde #388

Merged
merged 35 commits into from
Aug 28, 2023
Merged

[x/programs] serialize/deserialize using serde #388

merged 35 commits into from
Aug 28, 2023

Conversation

samliok
Copy link
Contributor

@samliok samliok commented Aug 22, 2023

Serialization using Serde

Why?

In order to support more complex data structures moving across the WASM boundary we would need to define our own ways to convert them to/from bytes. This is what ProgramValues were used for. ProgramValue's implemented the Store trait with methods as_bytes, from_bytes & as_tag. Crates like Serde & serde_bare manage & implement these methods for us, and for more complex structs. This PR uses the from_slice and to_vec functions from the serde_bare crate to serialize/deserialize objects.

Advantages

  • Developers are now limited to Serde's serialization types which are much more expansive than ProgramValues
  • Program UX is greatly simplified. No need to convert to/from ProgramValues, can just use native types/user defined structs. See before and after at the bottom.
  • Don't have to program serialization/deserialization which can be a non-trivial task.
  • Doesn't increase binary size that much. TokenProgram went from ~228kb to ~222kb.

Disadvantages

  • At least for now, we still need to define our own serialization for parameters. This includes any type that implements the Argument trait. So far these are ProgramContext, i64, Bytes32, and Address. The reason for this is b/c when we invoke an external program, we need to serialize all params into one byte array. Then in go we need to deserialize in order to pass on the parameters to the desired external function.
  • Performance costs see @hexfusion's comment see performance section

Changes

  • Remove ProgramValue, Store and related stuff
  • Uses the from_slice and to_vec functions from the serde_bare crate to serialize/deserialize objects.
  • Get and Store functions take in a param that implements Serialize or DeserializedOwned
  • Argument trait, used during program_invoke
  • Added Pokemon example to show using serde for custom structs
  • Update all program examples
  • Changes ProgramContext -> Context
  • Added go tests for Pokemon and Lottery examples.

Performance

goos: darwin
goarch: arm64
pkg: github.com/ava-labs/hypersdk/x/programs/examples
                                        │   old.txt   │              new.txt               │
                                        │   sec/op    │   sec/op     vs base               │
TokenProgram/benchmark_token_program-10   8.948m ± 4%   9.777m ± 3%  +9.27% (p=0.000 n=10)

                                        │   old.txt    │               new.txt               │
                                        │     B/op     │     B/op      vs base               │
TokenProgram/benchmark_token_program-10   4.809Mi ± 0%   4.737Mi ± 0%  -1.50% (p=0.000 n=10)

                                        │   old.txt   │              new.txt               │
                                        │  allocs/op  │  allocs/op   vs base               │
TokenProgram/benchmark_token_program-10   43.32k ± 0%   46.83k ± 0%  +8.09% (p=0.000 n=10)

Compiling Rust using
./scripts/build.sh

Running test with
go test -v -timeout 30s -run ^TestTokenProgram$ github.com/ava-labs/hypersdk/x/programs/examples --count 1

Before

Screenshot 2023-08-24 at 2 31 07 PM

After

Screenshot 2023-08-24 at 2 30 55 PM

Copy link
Contributor Author

@samliok samliok left a comment

Choose a reason for hiding this comment

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

comments on bug

@hexfusion
Copy link
Contributor

The perf cost of this in non trivial

goos: linux
goarch: amd64
pkg: github.com/ava-labs/hypersdk/x/programs/examples
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
                                        │   old.txt   │               new.txt                │
                                        │   sec/op    │   sec/op     vs base                 │
TokenProgram/benchmark_token_program-12   16.62m ± 4%   37.37m ± 2%  +124.88% (p=0.000 n=10)

                                        │   old.txt    │               new.txt                │
                                        │     B/op     │     B/op      vs base                │
TokenProgram/benchmark_token_program-12   4.811Mi ± 0%   7.611Mi ± 0%  +58.20% (p=0.000 n=10)

                                        │   old.txt   │                new.txt                │
                                        │  allocs/op  │  allocs/op    vs base                 │
TokenProgram/benchmark_token_program-12   43.33k ± 0%   116.28k ± 0%  +168.35% (p=0.000 n=10)

@samliok samliok self-assigned this Aug 28, 2023
@samliok samliok changed the title [x/programs] Use Serde to serialize/deserialize bytes across the host [x/programs] serialize/deserialize bytes across the host using serde Aug 28, 2023
@samliok samliok changed the title [x/programs] serialize/deserialize bytes across the host using serde [x/programs] serialize/deserialize using serde Aug 28, 2023
Copy link
Contributor

@richardpringle richardpringle left a comment

Choose a reason for hiding this comment

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

Minor comments, feel free to open an issue to fix later if you want to get this PR landed

samliok and others added 7 commits August 28, 2023 16:50
Co-authored-by: Richard Pringle <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
Co-authored-by: Richard Pringle <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
Co-authored-by: Richard Pringle <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
Co-authored-by: Richard Pringle <[email protected]>
Signed-off-by: Sam Liokumovich <[email protected]>
Copy link
Contributor

@richardpringle richardpringle left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -10,65 +9,52 @@ static TOKEN_PROGRAM_NAME: &str = "token_contract";
/// Initializes the program.
#[expose]
fn init_program() -> i64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this to init?

@patrick-ogrady patrick-ogrady merged commit a5b0bb1 into main Aug 28, 2023
@patrick-ogrady patrick-ogrady deleted the serde branch August 28, 2023 22:38
@samliok samliok linked an issue Aug 29, 2023 that may be closed by this pull request
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.

[x/programs] Improve Type System
4 participants