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.28k stars 1.58k forks source link

Can we add a tag in the header of a kernel file to indicate if it is a full kernel file or an incremental kernel file #31545

Open a-siva opened 6 years ago

a-siva commented 6 years ago

Would be nice if we had a tag in the header of a kernel file which indicated if the file is a full self contained kernel application file Vs an incremental kernel file.

This would help the VM at startup, if the specified file is a full self contained kernel file it would create an isolate using this file directly and if the file is an incremental kernel file it would create an isolate with the platform file and then load this kernel file into the isolate.

mraleph commented 6 years ago

I think we also need the following tags while we are at it:

mraleph commented 6 years ago

/cc @kmillikin @sjindel-google

a-siva commented 6 years ago

What would be the difference between a 'fully linked kernel file' and a 'full self contained kernel application file' ?

Would we consider the current vm-platform.dill file a 'fully linked kernel file'? It is not a 'full self contained kernel application file' per my definition above.

mraleph commented 6 years ago

I think I got confused by 'incremental': e.g. if we use Kernel binary as a script snapshot then it is not incremental in any way, it just does not contain core libraries.

Now that I reread your definitions I think 'fully linked kernel binaries' are the same as 'self contained kernel application file'.

Fully linked kernel binaries are used for AOT cases, while non-linked / partial / incremental (we need some sort of good name for this) are used for JIT cases.

kmillikin commented 6 years ago

Yes, we will add this.

kmillikin commented 6 years ago

The current design is:

  1. Kernel files are always self-contained, but they are not always "closed'. They have external libraries that give signatures for their external dependencies. This allows them to always be processed separately without resorting to reading and linking to their dependencies. Some Kernel files happen to be closed in the sense that they have no exernal libraries, though this isn't a very interesting property. We don't indicate this directly but we can easily check by iterating the libraries. (For example, if we compile the Dart platform libraries, they are closed because they have no (non-native) external dependencies.)

  2. Kernel files can be linked or not. Linked Kernel files are always closed, they never have external dependencies. Additionally they have had whole-program transformations applied and those transformations have possibly made closed-world assumptions. We do not currently indicate whether a file is linked or not, but we should. There is currently no good way to sniff for whether a file is linked or not, because the absence of external libraries alone is not enough. We could actually remove the canonical names ("link table") from linked files and use direct references instead, and we could sniff for the canonical names. Or we could also have a more direct indication (e.g., change the magic number).

  3. Kernel files can optionally have a distinguished entry point ("main procedure") that was specified at compile time. This is easy to sniff for.

There are a couple of other things that we use that I think we should consider to be VM-specific:

  1. The VM's platform libraries are linked, though they do not actually form a closed program. This limits the link-time optimizations that can be applied to the platform libraries to those that do not make closed-world assumptions.

  2. The VM's incremental Kernel files are not self-contained because the external libraries have been stripped out. This prevents them from being processed separately from their dependencies, which is not an issue for the way the VM uses them. Additionally they have been linked and had whole program transformations applied, which again limits the transformations that can be applied.

We should continue to think of these as snapshot formats owned by the VM team. We should probably indicate them by again using a different magic number (and then, we can put anything at all that we want in the header since we will know how to interpret it).

As far as strong mode, I'm not entirely sure what we should do here. Eventually it will be the only mode. In any case, I'd like to come up with a property that makes sense for Kernel files independently of Dart and the way it was compiled from Dart by the front end (consider that Kernel code might be generated directly by some other tool). What is the property we are really interested in? Is it a flag that indicates that the program is not intended to type check?

Proposal: I will introduce a pair of new magic numbers that we will use to indicate (a) that a file has been linked and (b) that it's a VM-specific Kernel-based format.

mraleph commented 6 years ago

What is the property we are really interested in? Is it a flag that indicates that the program is not intended to type check?

There are two ways to look at this:

Maybe this is also a good moment to stop thinking about "strong mode" separately from other Dart 2 features. Maybe instead of "strong mode" flag we actually just need a Dart version flag on the Kernel binary.

Proposal: I will introduce a pair of new magic numbers that we will use to indicate (a) that a file has been linked and (b) that it's a VM-specific Kernel-based format.

One important thing to consider is persistence: if you deserialize program and then serialize it again the magic number needs to be correct. So I suggest instead of making it a magic number making it a flag on the root node (e.g. Program) or introducing a new node.

a-siva commented 6 years ago

Since we will be using kernel only in Dart 2.0 we should consider all kernel files as being 'strong mode'. We have this current temporary state where we seem to be operating in both srtong mode and non strong mode configurations while using kernel but I don't think we should worry too much about this transient state. Considering that I am not too sure about the value of incorporating 'strong mode' into the kernel format.

One further requirement is we should be able to determine quickly from the kernel file it it is a

vsmenon commented 6 years ago

@kmillikin - I bumped this to Beta 3. Can you move to Beta 4 or Dart 2 if one of those is more appropriate?

dgrove commented 6 years ago

@a-siva any further thoughts on what is needed here? I agree that strong/nonstrong isn't needed.

a-siva commented 6 years ago

We need a state in the header file that would help us identify a kernel file type, the various types being

kmillikin commented 6 years ago

Let's do this to get unblocked: add a header metadata field that tools can write anything they want into. We can make it variable-sized eventually, but for now let's make it a fixed-size (1 byte). It'll be easy to skip, and easy to sniff.

Since the Dart team currently controls all the tools and transformations, we can use it however we want. I'll create a doc somewhere we we can record the way that we use the field.

dgrove commented 6 years ago

Still required for 2.1?

kmillikin commented 6 years ago

Unrelated to Dart 2.1.

jacob314 commented 6 years ago

Missing this feature makes it really hard for us to track whether a Flutter app was build with the --track-widget-creation transformer or not which is critical to let users turn on that flag safely. I think I have an acceptable workaround implemented purely within flutter_engine to unblock but having the right solution would be really nice to improve code health.

Background info: https://dart-review.googlesource.com/c/sdk/+/52403 was dropped waiting for this feature to land but we can't wait anymore. I think I have a more limited fix than https://dart-review.googlesource.com/c/sdk/+/52403 but it is even more of a hack it needs to be a temporary solution. @aam

jacob314 commented 6 years ago

I think this should be P1 not P2.

jacob314 commented 6 years ago

Workaround CLs: https://dart-review.googlesource.com/c/sdk/+/80401 https://github.com/flutter/engine/pull/6562 This workaround is more expensive than reading a header field as worst case we have to load the whole dill file and then discard it as the transforms were incompatible. Typical case should be fine as there is not a significant slowdown if the transforms are compatible which is normally the case.