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

Borrowing of data in callRustSync #131

Open
janpaul123 opened this issue Mar 9, 2022 · 0 comments
Open

Borrowing of data in callRustSync #131

janpaul123 opened this issue Mar 9, 2022 · 0 comments
Labels
enhancement New feature or request flux

Comments

@janpaul123
Copy link
Contributor

Currently when using mutable buffers (and presumably, in the future when using mutable pointers; #125) we transfer ownership to Rust when passing it into callRustSync. This means that afterwards, the buffer can't be used. However, it's common to want to mutate some data in Rust, and then use it again. Currently you have to use the following pattern, e.g. when doing a matrix operation:

  1. Create the matrix, e.g. using zaplib.createMutableBuffer
  2. Send it to Rust to mutate it a bunch there.
  3. Return the same exact buffer from Rust (though it's mutated there now).
  4. Update the variable in JS to point to the new buffer.

This is pretty inconvenient, especially since we know that when we keep the data on the same thread (synchronously) that there is no opportunity for data races.

It would be nice if we can pass a borrowed buffer into Rust, which means that Rust will enforce that it doesn't get used on a different thread.

If we do this, we'll have to differentiate between whether a mutable buffer is passed in to borrow, or to move. (Read-only buffers are not affected by any of this.) I see a few options:

  1. Have some sort of special function on the JS side. E.g. zaplib.move(buffer) or buffer.move() or zaplib.borrow(buffer). We'll have to choose what the default is — I personally think for callRustSync borrowing should be the default, but for callRustAsync borrowing makes no sense and should not even be allowed. So should moving then be the default for callRustAsync? Or should you still use a syntax like zaplib.move(buffer) (I'd vote for the latter, I think!)
  2. We could determine whether to borrow or move on the Rust side. E.g. if you use as_u8_slice then you borrow, or if you say into_u8_vec then you move. We'd have to communicate this information back to JS, but since it's a synchronous call, that can wait until the call is over, so hopefully this can be pretty low-overhead. It could be a bit unclear what is happening though in this case, especially since you might also want to take a u8 slice in the async case (e.g. if that is what the function that you call expects) even though in the async case ownership is always moved.

Another consideration here is static typing; in the first case we can carry the "borrow vs owned" information forward through the type system, and even enforce it better across the boundary once we implement #99.

@janpaul123 janpaul123 added enhancement New feature or request flux labels Mar 9, 2022
@janpaul123 janpaul123 added this to the Basic stabilization milestone Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request flux
Projects
Status: Backlog
Development

No branches or pull requests

1 participant