Does both dartdev.dart.snapshot and dartdev.dill have to be included in the SDK?

[jensjoha]

Inside the out/ReleaseX64/dart-sdk/bin/snapshots/ directory we have both dartdev.dart.snapshot and dartdev.dill. The test pkg/dartdev/test/load_from_dill_test.dart talks about needing both because "The DartDev snapshot includes the --use_field_guards flag. If --no-use-field-guards is passed, the VM will fail to load the snapshot and should fall back to using the DartDev dill file."

I asked @mraleph if the --use-field-guards thing it was true, considering is it then - apparently - only true for the dartdev snapshot as the built-in CFE is also - at least I think - a snapshot. @mraleph is of the opinion that (paraphrasing) the test is outdated and that the VM forces the isolate to use a specific set of options just like the kernel isolate.

@bkonyi suggested the test should potentially be updated to correctly test the fallback logic.

So either

• the test should be updated if the dill file is necessary, or
• the dill file should be removed if it's not necessary, or
• if the dill is currently necessary it should be made so it isn't, and then removed.

Again, considering dartdev is the only "snapshot" that is included as dill as well I'm - maybe naively? - thinking that one of the last two would/should be the way to go.

[mraleph]

I am not sure this fallback logic is worth it. I'd vote for removing it.

[bkonyi]

Unless we can also remove the fallback logic for the kernel service, we need to keep this functionality. There's more details on the original issue for the dartdev fallback logic: #42804.

[jensjoha]

Is there a fallback for the kernel service? There at least doesn't seem to be any dill included in the "snapshot" folder.

[bkonyi]

Oh, maybe we got rid of it since then?

[mraleph]

We probably removed it because using isolate specific flags and pinning them for auxiliary isolates is a better way to fix this.

(Users should not be touching most of these flags anyway - so shipping fallback kernel is just a waste of SDK binary size)

[bkonyi]

@derekxu16 can I trouble you to take a look at this when you have a chance?

[derekxu16]

Removing dartdev.dill caused some failures (#50706) so we have to figure out how to prevent those.

[a-siva]

I believe for the kernel service the dill file is part of the executable, see

bin_to_linkable("kernel_service_dill_linkable") {
  deps = [ "../../utils/kernel-service:kernel_service_dill" ]
  input = "$root_gen_dir/kernel_service.dill"
  symbol = "kKernelServiceDill"
  size_symbol = "kKernelServiceDillSize"
  executable = false
}

[a-siva]

I believe for some of the architrectures (simarm, simarm64, riscv etc.) the kernel_service snapshot is not built and on these platforms the dill file is used.

[mit-mit]

Could we then not make the SDK bundling include the snapshot just on those, and not elsewhere?

[a-siva]

I guess you are saying if we generate a kernel_service or dartdev snapshot for an archtecture then we should not include the .dill file

[mit-mit]

Yup. Remove the .dill file on just those architectures where it's not in use. I think that means keeping it just on those architectures where we don't build the kernel_service snapshot ?

[a-siva]

https://dart-review.googlesource.com/c/sdk/+/306134