-
Notifications
You must be signed in to change notification settings - Fork 17
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
Native memory helpers #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please consider Uint8List toUint8List()
as part of this code review, as well.
This is not entirely safe. The We have a tracking issue for |
bool isNullOrNullPointer(Pointer? ptr) => ptr == null || ptr.isNullPointer; | ||
|
||
/// Extension method for converting a [String] to a `Pointer<Utf8>`. | ||
extension NativeFloats on List<double> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have a FloatList
from dart:typed_data
, and your FFI call is a leaf-call, you will not have to copy the list to native memory after dart-lang/sdk#44589. Can you structure your API around FloatList instead of List<Float>
?
|
||
/// Converts a pointer to a (representing a list of) floats to a list of | ||
/// Dart doubles. | ||
List<double> toDartListDouble(Pointer<Float> floats, {int? length}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd return FloatList
here. This would help with:
- If your users pass it in to anther FFI call you might not need to copy (see comment below).
- Indexed access is potentially not dynamic calls with the more specific types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see Float32List
and Float64List
, but no agnostic FloatList
. Is there a generic, adaptive FloatList
somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, my bad. No, I meant to use Float32List.
Do you have any estimates on the tracking issue? It doesn't seem about to land, given that you haven't gotten any responses to your questions. Absent that catch-all implementation, I'd love to quickly sync on the edge cases here to make sure I'm properly handling everything. Even with your helpful answer, I'm not sure how to exhaustively handle these edge cases. |
This PR is being replaced. |
Adds helpers for converting between Dart and native memory for the following scenarios:
Uint8List
<-> A nativePointer<Char>
.Additionally, I would like to submit the function
Uint8List toUint8List(Pointer<Char> val, {int? length})
for code review, even though it was committed previously.--
The context for this PR is work around Embeddings, which are currently emitting random results on sequential runs, and for which I want to rule out these low level memory mapping helpers.
cc @Piinks