Skip to content
This repository has been archived by the owner on May 9, 2022. It is now read-only.

TY-1759 move serialized bytes allocation from dart to rust #52

Merged
merged 6 commits into from
Apr 27, 2021

Conversation

janpetschexain
Copy link
Contributor

@janpetschexain janpetschexain commented Apr 22, 2021

References

Summary

  • add an allocator for CBytes in rust and use it in the dart wrapper: we don't have to differentiate between owned/borrowed for the Bytes in dart, destructor always runs in rust; but this can potentially panic, which requires more complex try-catches in dart.
  • rearrange the rust ffi docs a bit: the blob is becomming quite large, hence i exposed the first layer of modules in the documentation to structure it in a more fine grained way

also the ExternError is getting more and more annoying (cumbersome message freeing, unnecessary *const/*mut casts, i32 limitation for the code, no impl From<Infallible> for ExternError, etc). depending on how we want to unify the c error handling with the wasm error handling after #51, we should think about refactoring this as well.

@janpetschexain janpetschexain marked this pull request as ready for review April 23, 2021 10:02
@acrrd
Copy link
Contributor

acrrd commented Apr 23, 2021

My idea when I wrote the comment was to move only the allocation and not the copy. So just bytes_new(size: 32) that behaves like a Vec::with_capacity(size).leak(). This will avoid the double allocation that you mention. Will not change much but we have allocation and free all in rust and we don't have two version of free.

@janpetschexain
Copy link
Contributor Author

My idea when I wrote the comment was to move only the allocation and not the copy. So just bytes_new(size: 32) that behaves like a Vec::with_capacity(size).leak(). This will avoid the double allocation that you mention. Will not change much but we have allocation and free all in rust and we don't have to version of free.

ah ok, i see what you meant, i'll update the pr accordingly.

@acrrd acrrd self-requested a review April 26, 2021 12:17
@acrrd acrrd self-requested a review April 27, 2021 07:52
@janpetschexain janpetschexain merged commit dc972a1 into master Apr 27, 2021
@janpetschexain janpetschexain deleted the TY-1759-move_cbytes branch April 27, 2021 09:00
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants