search

fuzzybinary / dart_shared_library

An attempt to package Dart into a usable shared library (.dll / .so)
Other
77 stars 10 forks source link

Some improvements #10

Open tritao opened 11 months ago

tritao commented 11 months ago

Hey, just started playing with this library and wanted to document some issues/improvements.

1) Shared objects are exporting all symbols, this makes them a lot larger than they need to be. Ideally non Dart public API symbols will be hidden (by using -fvisibility=hidden for instance).

0000000002580fd0 b _ZZNSt3__212_GLOBAL__N_14makeINS_8time_getIcNS_19istreambuf_iteratorIcNS_11char_traitsIcEEEEEEJjEEERT_DpT0_E3buf
0000000002580ff0 b

2) Debug info could be split from the shared object using -gsplit-dwarf or by running separate commands after build (with strip and objcopy)

3) Missing some include files like dart_tools_api.h, dart_native_api.h, dart_api_dl.h, dart_tools_api.h and dart_version.h

fuzzybinary commented 11 months ago

Hi @tritao, thanks for the feedback!

Agreed about the first point about using -fvisibility=hidden. Would you be up for submitting a PR to get that in?

The second point about debug info might make sense for release builds, but at the moment since I'm still doing a lot of debugging / experimentation, having the debug info in there is super helpful. Maybe a future improvement.

For the third point -- dart_tools_api.h is in there (or should be) and you're right dart_native_api.h should probably be included. I'll try to get something up to include it in the artifacts soon.

But dart_api_dl.h and dart_version.h I think are specifically for Dart extensions that are using the Dart shared library. I don't think they're applicable for this case. Please let me know if I'm wrong about that.

tritao commented 11 months ago

Agreed about the first point about using -fvisibility=hidden. Would you be up for submitting a PR to get that in?

I can probably cook something up, just might need to learn a bit about how GN works.

The second point about debug info might make sense for release builds, but at the moment since I'm still doing a lot of debugging / experimentation, having the debug info in there is super helpful. Maybe a future improvement.

Sure, agreed.

But dart_api_dl.h and dart_version.h I think are specifically for Dart extensions that are using the Dart shared library. I don't think they're applicable for this case. Please let me know if I'm wrong about that.

I guess so, I was thinking it could be useful in some cases. Let's say an app embds this library to provide Dart scripting, and it also allows native plugins. In that case it could be useful to provide that header for the native plugins to be able to access Dart API. But it's not really something I will need, so not worth spending time on it I think.

I've been trying to get this to work via Rust and dart_dll.h has some C++ constructs which trip bindgen. I've added some ifdef's for now to make it C-compatible, and got something to work. Would you like a PR for that?

And are some Rust examples something that would be interesting on this repo for you and should I just make a separate project for that?

fuzzybinary commented 11 months ago

I've been trying to get this to work via Rust and dart_dll.h has some C++ constructs which trip bindgen. I've added some ifdef's for now to make it C-compatible, and got something to work. Would you like a PR for that?

Yes. I'd like the header to be C-compatible, so long as it doesn't break C++ those changes would be welcome.

And are some Rust examples something that would be interesting on this repo for you and should I just make a separate project for that?

Submit them as a separate PR and I'll take a look, I think it would be useful for people to see yes.