Skip to content

Question: Exposing more of dart_api.h in dart_api_dl.h #50494

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

Open
nielsenko opened this issue Nov 17, 2022 · 5 comments
Open

Question: Exposing more of dart_api.h in dart_api_dl.h #50494

nielsenko opened this issue Nov 17, 2022 · 5 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug

Comments

@nielsenko
Copy link

Would exposing functions to CRU Dart objects from native in dart_api_dl.h, ie. functions like Dart_NewExternalUTF16String, etc. be something the SDK team would be okay with exposing?

@mraleph
Copy link
Member

mraleph commented Nov 17, 2022

Dart_NewExternalUTF16String seems okay.

We should consider if we want to also add equivalent operations on the dart:ffi side similar to asTypedData so that pointers could be quickly converted to external strings.

/cc @dcharkes

@dcharkes
Copy link
Contributor

@nielsenko What is the list of functions you would like to expose?

Would you be able to draft a PR along the lines of https://dart-review.googlesource.com/c/sdk/+/269742.

We should consider if we want to also add equivalent operations on the dart:ffi side similar to asTypedData so that pointers could be quickly converted to external strings.

This is tracked in:

@devoncarew devoncarew added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Nov 17, 2022
@nielsenko
Copy link
Author

@dcharkes Yes we would be prepared to do the "work" ourself. Also this is not someting we need short term. The use of these are pending a rewrite of our binding layer, that we have not committed to finally yet. I'm just investigating the possibilities.

The list of functions as best I can tell at present would be:

DART_EXPORT Dart_Handle Dart_NewInteger(int64_t value);
DART_EXPORT Dart_Handle Dart_IntegerToInt64(Dart_Handle integer, int64_t* value);
DART_EXPORT Dart_Handle Dart_NewDouble(double value);
DART_EXPORT Dart_Handle Dart_DoubleValue(Dart_Handle double_obj, double* value);
DART_EXPORT Dart_Handle Dart_NewBoolean(bool value);
DART_EXPORT Dart_Handle Dart_BooleanValue(Dart_Handle boolean_obj, bool* value);
DART_EXPORT Dart_Handle Dart_NewStringFromUTF8(const uint8_t* utf8_array, ntptr_t length);

Dart_NewExternalUTF16String(const uint16_t* utf16_array,
                            intptr_t length,
                            void* peer,
                            intptr_t external_allocation_size,
                            Dart_HandleFinalizer callback);

DART_EXPORT Dart_Handle Dart_StringToUTF8(Dart_Handle str,
                                          uint8_t** utf8_array,
                                          intptr_t* length);

DART_EXPORT Dart_Handle Dart_NewByteBuffer(Dart_Handle typed_data);

Dart_New(Dart_Handle type,
         Dart_Handle constructor_name,
         int number_of_arguments,
         Dart_Handle* arguments);        

As an aside, it would be cool if the vm supported utf8 intrinsically, so that there would be a

Dart_NewExternalUTF8String(const uint8_t* utf8_array,
                            intptr_t length,
                            void* peer,
                            intptr_t external_allocation_size,
                            Dart_HandleFinalizer callback);

but that we would not attempt ourselves, and I guess that is a bit of a stretch to ask for.

@mraleph
Copy link
Member

mraleph commented Nov 18, 2022

DART_EXPORT Dart_Handle Dart_NewInteger(int64_t value);
DART_EXPORT Dart_Handle Dart_IntegerToInt64(Dart_Handle integer, int64_t* value);
DART_EXPORT Dart_Handle Dart_NewDouble(double value);
DART_EXPORT Dart_Handle Dart_DoubleValue(Dart_Handle double_obj, double* value);
DART_EXPORT Dart_Handle Dart_NewBoolean(bool value);
DART_EXPORT Dart_Handle Dart_BooleanValue(Dart_Handle boolean_obj, bool* value);

These ones are a bit problematic because these operations are definitely more efficient when performed by the FFI trampoline itself, e.g doing Dart_NewDouble would create and return a box - while the bridge has a chance to keep the value unboxed etc.

Do you have a design doc of some sort for the rewrite?

@nielsenko
Copy link
Author

No design doc yet.

I see your point. Lets drop the ones that ffi can avoid boxing, ie. int, bool, and double. We do pay for boxing anyway on the Dart side today, but that would definitely be something we would like to avoid as part of a rewrite.

@a-siva a-siva added the type-enhancement A request for a change that isn't a bug label Nov 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants