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

feat: Make it easy to convert data to IDLValue and candid text format #502

Merged
merged 3 commits into from
Dec 7, 2023

Conversation

bitdivine
Copy link
Member

@bitdivine bitdivine commented Dec 6, 2023

Overview
It is not well documented how to convert types that implement CandidType into an IDLValue.

Requirements
It is easy to convert instances of Rust types into IDLValues using the best available method.

Considered Solutions

  • Provide a Candid library method for the conversion. (implemented in this PR)
  • Document the current approach. However the current approach seems inefficient and can probably be improved.

Recommended Solution
Provide a library method for converting data to Candid.

The current method - serializing and parsing - can probably be improved greatly, so it seems preferable to offer a conversion method in the candid library and then improve that method rather than document a method that is sub-optimal and then expect developers to update their code when a better method is documented.

Considerations

  • I think that this will have no negative influence on performance.
  • This makes conversion of data to text-format candid a one-liner: IDLValue::try_from_candid_type(data).unwrap().to_string()

Copy link

github-actions bot commented Dec 6, 2023

Benchmark for adea395

Click to view benchmark
Test Base PR %
Blob/&str 605.5±76.77µs 615.2±94.71µs +1.60%
Blob/ByteBuf 347.8±89.01µs 342.1±92.79µs -1.64%
Blob/Bytes 397.8±80.24µs 393.3±74.29µs -1.13%
Blob/String 633.6±242.90µs 638.5±249.09µs +0.77%
Collections/vec (text, nat) 51.5±1.85ms 51.5±0.90ms 0.00%
Collections/vec int 23.2±0.25ms 23.2±0.78ms 0.00%
Collections/vec int64 16.2±0.42ms 15.7±0.09ms -3.09%
Collections/vec nat8 12.1±0.81ms 12.0±0.09ms -0.83%
option list/1024 978.0±12.43µs 971.9±5.77µs -0.62%
profiles/1024 1896.0±16.02µs 1941.3±31.64µs +2.39%
variant list/1024 795.2±11.03µs 793.6±4.43µs -0.20%

@bitdivine bitdivine marked this pull request as ready for review December 6, 2023 15:20
@bitdivine bitdivine changed the title Make it easy to convert data to candid text format feat: Make it easy to convert data to IDLValue and candid text format Dec 6, 2023
Copy link
Contributor

@chenyan-dfinity chenyan-dfinity left a comment

Choose a reason for hiding this comment

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

annotate_types is a best-effort implementation, notably subtyping is not supported. A better way is to use from_bytes_with_types which implements everything from the official deserializer.

Copy link

github-actions bot commented Dec 6, 2023

Benchmark for c8a2efc

Click to view benchmark
Test Base PR %
Blob/&str 610.2±103.64µs 591.1±90.18µs -3.13%
Blob/ByteBuf 347.6±89.32µs 353.2±85.53µs +1.61%
Blob/Bytes 381.2±64.49µs 354.1±61.29µs -7.11%
Blob/String 324.7±136.71µs 362.0±129.40µs +11.49%
Collections/vec (text, nat) 53.1±2.59ms 53.0±0.66ms -0.19%
Collections/vec int 23.0±0.10ms 22.9±0.09ms -0.43%
Collections/vec int64 16.2±0.06ms 15.8±0.22ms -2.47%
Collections/vec nat8 11.6±0.99ms 11.5±0.10ms -0.86%
option list/1024 996.8±21.77µs 985.6±11.31µs -1.12%
profiles/1024 1865.3±29.68µs 1865.1±35.30µs -0.01%
variant list/1024 789.9±9.75µs 795.2±10.88µs +0.67%

Copy link

github-actions bot commented Dec 6, 2023

Benchmark for 0c3da76

Click to view benchmark
Test Base PR %
Blob/&str 621.2±91.03µs 607.3±89.08µs -2.24%
Blob/ByteBuf 357.8±86.55µs 354.9±89.94µs -0.81%
Blob/Bytes 390.0±62.84µs 363.6±60.91µs -6.77%
Blob/String 331.5±138.84µs 366.5±130.36µs +10.56%
Collections/vec (text, nat) 54.0±4.82ms 52.9±1.28ms -2.04%
Collections/vec int 22.9±0.13ms 22.8±0.14ms -0.44%
Collections/vec int64 16.7±0.09ms 16.1±0.09ms -3.59%
Collections/vec nat8 11.6±0.44ms 11.5±0.13ms -0.86%
option list/1024 984.1±18.32µs 987.4±19.58µs +0.34%
profiles/1024 1927.1±49.76µs 1934.6±58.63µs +0.39%
variant list/1024 803.1±13.60µs 809.4±15.99µs +0.78%

@bitdivine
Copy link
Member Author

@chenyan-dfinity Thank you!

@bitdivine bitdivine merged commit 16c167a into master Dec 7, 2023
5 checks passed
@bitdivine bitdivine deleted the idl-value-from-data branch December 7, 2023 03:08
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.

2 participants