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(ops): Basic buffer support for op2 #33

Merged
merged 11 commits into from
Jul 11, 2023
Merged

Conversation

mmastrac
Copy link
Contributor

@mmastrac mmastrac commented Jul 11, 2023

Adds:

  • #[buffer] &[u8]
  • #[buffer] &mut [u8]

TODO:

  • #[buffer] *const u8 (all primitive types) -- not recommended but used in FFI
  • #[buffer] *mut u8 (all primitive types)
  • #[buffer] bytes::Bytes
  • #[buffer] JSBuffer
  • #[buffer] V8Slice
  • #[buffer] Vec<..>
  • #[buffer] Box<[..]>

TODO: Alternative buffer modes:

  • #[buffer(shared)] (the default)
  • #[buffer(copy)]
  • #[buffer(detach)]

ops/op2/signature.rs Outdated Show resolved Hide resolved
@aapoalas
Copy link

thought: It's possible for V8 ArrayBuffers to be created from external pointers that do not follow the normal alignment rule of V8 ABs (code says always aligned to 4 bytes, experience seems to suggest this is 8 bytes for 64 bit environments). In these cases the TypedArrays will then happily allow for themselves to be created at odd offsets. This needs to be remembered when eg. a Float64Array is converted into a Rust slice.

@mmastrac
Copy link
Contributor Author

mmastrac commented Jul 11, 2023

thought: It's possible for V8 ArrayBuffers to be created from external pointers that do not follow the normal alignment rule of V8 ABs (code says always aligned to 4 bytes, experience seems to suggest this is 8 bytes for 64 bit environments). In these cases the TypedArrays will then happily allow for themselves to be created at odd offsets. This needs to be remembered when eg. a Float64Array is converted into a Rust slice.

Definitely -- this is automatic in the fast path, but we should throw alignment errors if this happens in the slow path as well. Right now the existing f32/f64/u32/etc ops probably "work" on x64 and will segfault on ARM64.

@mmastrac mmastrac changed the title [WIP] feat(ops): Buffer support for op2 feat(ops): Basic buffer support for op2 Jul 11, 2023
@mmastrac mmastrac mentioned this pull request Jul 11, 2023
18 tasks
core/runtime/ops.rs Outdated Show resolved Hide resolved
core/runtime/ops.rs Outdated Show resolved Hide resolved
ops/op2/dispatch_slow.rs Outdated Show resolved Hide resolved
ops/op2/signature.rs Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@mmastrac mmastrac enabled auto-merge (squash) July 11, 2023 22:26
@mmastrac mmastrac merged commit 694bd9b into denoland:main Jul 11, 2023
4 checks passed
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.

None yet

3 participants