neon-bindings / rfcs

RFCs for changes to Neon
Apache License 2.0
14 stars 9 forks source link

Pre-RFC: Borrowing refactor #40

Open kjvalencik opened 3 years ago

kjvalencik commented 3 years ago

Borrow

Problem

The current borrowing API was designed to support borrowing both binary JavaScript data and the internal Rust data of classes. It is intended to prevent aliased borrows.

However, it has multiple soundness holes (JavaScript handles might alias the same object, multiple Lock can be created). It also has an API that includes types and helpers from before NLL that are awkward and unnecessary today.

Lastly, classes were removed in the N-API backend, leaving a generic API that only applies to borrowing binary data.

Proposal

Module

Removal

Buffer is a legacy type from before JavaScript included ArrayBuffer. It is now a sub-class of Uint8Array. Neon does not need any type level distinction for JsBuffer because it can be treated as an array view.

Addition

Neon does not currently understand views. We will need to add this. We most likely do not need to know the view type and can have a simple JsArrayView. If we want to include the type, it could be done with a generic phantom data element. JsArrayView<u8>.

Borrowing

In an initial implementation, we will only support borrowing ArrayBuffer. This way we can avoid complicated set math for determining if any of our borrows overlap. We can simply keep a HashSet of starting pointers.

Lock

The Lock type will be removed from the public API and moved to Context.

Views

Borrowing views will mostly be a convenience API. We will still borrow the entire ArrayBuffer. This will behave very similar to how borrowing a &[u8] from a Vec<u8> borrows the entire Vec.

This is necessary to remove complexity from users. If we don't provide this API, users cannot borrow buffers without first either copying data to an ArrayBuffer or passing the underlying buffer along with a Range.

dherman commented 3 years ago

Great proposal, thanks for writing it up!

A couple possible tweaks: