dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.27k stars 1.58k forks source link

[vm][cfe][frontend_server] Make "native_assets.yaml" reusable #55826

Open dcharkes opened 5 months ago

dcharkes commented 5 months ago

For native assets, we've added the ability to embed some extra information in a kernel file that is read by the vm.

For adding data assets (https://github.com/dart-lang/sdk/issues/54003), we need something similar, but we don't care about Target/Abi. So shoehorning it into native_assets.yaml is not clean.

Adding a data_assets.yaml and an extra parameter everywhere in the kernel compilation (CFE, frontend server, etc.) doesn't seem very clean. Instead we should consider replacing the native_assets.yaml with some embed_in_kernel.json, that has as a top level key a native_code_assets which contains the current native_assets.yaml contents. Then subsequent types of info to be embedded in a kernel file and accessed from them VM don't have to do any changes in all the various places where kernel is compiled.

We could ease the roll of this migration by having the current flag/param being able to consume the new format first, and adding the flag/param everywhere before removing the old one.

Any new keys added would still be validated in pkg/vm/lib/native_assets/validator.dart because we'd only be adding metadata that is read by the vm. (The file should be renamed to pkg/vm/lib/metadata/validator.dart.)

Naming:

Possibilities:

@mkustermann Any suggestions for better naming?

@alexmarkov's suggestion is "metadata" https://github.com/dart-lang/sdk/issues/50152#issuecomment-1270354163

FYI @mosuem (We need to make this piece of infra reusable before putting the data assets mapping in the kernel file)

FYI @jensjoha Sorry for the churn, now I want to rename everything we did in the CFE and frontend_server! 😅

dcharkes commented 3 months ago

Data assets might also be supported in other backends than the VM in the future. So we should probably not call it vm_metadata either. metadata seems too generic though.

dcharkes commented 1 month ago

In https://dart-review.googlesource.com/c/sdk/+/379781, @mkustermann prototyped opaque "embedder-info". The fact it's opaque to the VM makes "embedder-info" a good name. (The prototype nests it in the native-assets-yaml path which this issue is tracking to make more generic.)

Naming:

The data assets prototype (https://dart-review.googlesource.com/c/sdk/+/379841) already uses the data in the embedder, and we'd like to migrate the native assets mapping to be read by the Dart standalone embedder rather than the VM (https://github.com/flutter/flutter/issues/154425). So, I don't believe we have a use case for a generic "vm-info" counterpart to the "embedder-info".

mkustermann commented 1 month ago

I think the main questions are (I made a choice in my prototype, but it's not the only choice):

These are somewhat intertwined: If the access is in C++ and the encoding is bytes and the embedder info being stored in the bytes is json, then the C++ code may need a C++ json decoder), ...