Test or deprecate core-jit snapshots

[alexmarkov]

It looks like we're currently using only core snapshots (in JIT mode) in Dart and in Flutter on Android and iOS.
core-jit snapshots are not tested but could be used by Flutter on Fuchsia.

We should either add tests for core-jit snapshots in Dart SDK repo (e.g. build standalone VM with core-jit snapshot and run some tests against it), or deprecate and remove support for core-jit snapshots.

/cc @a-siva @rmacnak-google

[a-siva]

Do we know if Flutter on Fuchsia uses core-jit snapshots ?

[alexmarkov]

Yes, it looks like flutter_runner uses core-jit snapshots:

https://github.com/flutter/engine/blob/fe7861cafbfb7a1baa8675f4a2d1d0c2c36c86db/shell/platform/fuchsia/flutter/kernel/BUILD.gn#L72

and maybe dart_runner too:

https://github.com/flutter/engine/blob/fe7861cafbfb7a1baa8675f4a2d1d0c2c36c86db/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn#L68

[akbiggs]

(new to Dart/Flutter internals & snapshots, might be missing something obvious here)

I tried replacing "core-jit" with "core" but I end up getting the following segfault during compilation:

akbiggs@akbiggs ~/engine/src/flutter
 % $ENGINE_DIR/flutter/tools/gn --fuchsia --no-lto --no-goma               

Generating GN files in: out/fuchsia_debug_x64
Generating compile_commands took 57ms
Done. Made 1418 targets from 289 files in 737ms
akbiggs@akbiggs ~/engine/src/flutter
 % ninja -C $ENGINE_DIR/out/fuchsia_debug_x64               

ninja: Entering directory `/usr/local/google/home/akbiggs/engine/src/out/fuchsia_debug_x64'
[0/1] Regenerating ninja files
[13/111] ACTION //flutter/shell/platform/fuchsia/dart_runner/kernel:kernel_core_snapshot(//build/toolchain/fuchsia:fuchsia)
FAILED: gen/flutter/shell/platform/fuchsia/dart_runner/kernel/vm_data.bin gen/flutter/shell/platform/fuchsia/dart_runner/kernel/vm_instructions.bin gen/flutter/shell/platform/fuchsia/dart_runner/kernel/isolate_data.bin gen/flutter/shell/platform/fuchsia/dart_runner/kernel/isolate_instructions.bin gen/flutter/shell/platform/fuchsia/dart_runner/kernel/snapshot_profile.json 
python3 ../../build/gn_run_binary.py clang_x64/gen_snapshot --lazy_async_stacks --enable_mirrors=false --deterministic --snapshot-kind=core --vm_snapshot_data=gen/flutter/shell/platform/fuchsia/dart_runner/kernel/vm_data.bin --vm_snapshot_instructions=gen/flutter/shell/platform/fuchsia/dart_runner/kernel/vm_instructions.bin --isolate_snapshot_data=gen/flutter/shell/platform/fuchsia/dart_runner/kernel/isolate_data.bin --isolate_snapshot_instructions=gen/flutter/shell/platform/fuchsia/dart_runner/kernel/isolate_instructions.bin --write_v8_snapshot_profile_to=gen/flutter/shell/platform/fuchsia/dart_runner/kernel/snapshot_profile.json --enable_asserts /usr/local/google/home/akbiggs/engine/src/out/fuchsia_debug_x64/dart_runner_patched_sdk/platform_strong.dill

===== CRASH =====
si_signo=Segmentation fault(11), si_code=1, si_addr=0x68
version=2.15.0-178.0.dev (dev) (Mon Oct 4 02:01:23 2021 -0700) on "linux_x64"
pid=4134749, thread=4134749, isolate_group=isolate(0x943d40), isolate=isolate(0x947460)
isolate_instructions=0, vm_instructions=0
Stack dump aborted because InitialRegisterCheck failed.

[15/110] ACTION //flutter/shell/platform/fuchsia/flutter/kernel:kernel_core_snapshot(//build/toolchain/fuchsia:fuchsia)
FAILED: gen/flutter/shell/platform/fuchsia/flutter/kernel/vm_isolate_snapshot.bin gen/flutter/shell/platform/fuchsia/flutter/kernel/vm_snapshot_instructions.bin gen/flutter/shell/platform/fuchsia/flutter/kernel/isolate_snapshot.bin gen/flutter/shell/platform/fuchsia/flutter/kernel/isolate_snapshot_instructions.bin gen/flutter/shell/platform/fuchsia/flutter/kernel/snapshot_profile.json 
python3 ../../build/gn_run_binary.py clang_x64/gen_snapshot --lazy_async_stacks --enable_mirrors=false --deterministic --snapshot_kind=core --vm_snapshot_data=gen/flutter/shell/platform/fuchsia/flutter/kernel/vm_isolate_snapshot.bin --vm_snapshot_instructions=gen/flutter/shell/platform/fuchsia/flutter/kernel/vm_snapshot_instructions.bin --isolate_snapshot_data=gen/flutter/shell/platform/fuchsia/flutter/kernel/isolate_snapshot.bin --isolate_snapshot_instructions=gen/flutter/shell/platform/fuchsia/flutter/kernel/isolate_snapshot_instructions.bin --write_v8_snapshot_profile_to=gen/flutter/shell/platform/fuchsia/flutter/kernel/snapshot_profile.json --enable_asserts /usr/local/google/home/akbiggs/engine/src/out/fuchsia_debug_x64/flutter_runner_patched_sdk/platform_strong.dill

===== CRASH =====
si_signo=Segmentation fault(11), si_code=1, si_addr=0x68
version=2.15.0-178.0.dev (dev) (Mon Oct 4 02:01:23 2021 -0700) on "linux_x64"
pid=4134765, thread=4134765, isolate_group=isolate(0x15bcd30), isolate=isolate(0x15c0450)
isolate_instructions=0, vm_instructions=0
Stack dump aborted because InitialRegisterCheck failed.

[29/79] ACTION //flutter/shell/platform/fuchsia/flutter:_flutter_ru...ner_tests_dbg_symbols_unstripped(//build/toolchain/fuchsia:fuchsia)
ninja: build stopped: subcommand failed.

[alexmarkov]

I was able to reproduce the crash using the following patch in Flutter engine:

diff --git a/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn b/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn
index d831ccd47b..0211665947 100644
--- a/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn
+++ b/shell/platform/fuchsia/dart_runner/kernel/BUILD.gn
@@ -65,7 +65,7 @@ template("create_kernel_core_snapshot") {
       "--lazy_async_stacks",
       "--enable_mirrors=false",
       "--deterministic",
-      "--snapshot_kind=core-jit",
+      "--snapshot_kind=core",
       "--vm_snapshot_data=" + rebase_path(vm_snapshot_data, root_build_dir),
       "--vm_snapshot_instructions=" +
           rebase_path(vm_snapshot_instructions, root_build_dir),
diff --git a/shell/platform/fuchsia/flutter/kernel/BUILD.gn b/shell/platform/fuchsia/flutter/kernel/BUILD.gn
index 470f07964d..2919ffda7c 100644
--- a/shell/platform/fuchsia/flutter/kernel/BUILD.gn
+++ b/shell/platform/fuchsia/flutter/kernel/BUILD.gn
@@ -69,7 +69,7 @@ template("core_snapshot") {
       "--lazy_async_stacks",
       "--enable_mirrors=false",
       "--deterministic",
-      "--snapshot_kind=core-jit",
+      "--snapshot_kind=core",
       "--vm_snapshot_data=" + rebase_path(vm_snapshot_data, root_build_dir),
       "--vm_snapshot_instructions=" +
           rebase_path(vm_snapshot_instructions, root_build_dir),

Repro with goma:

flutter/tools/gn --fuchsia --no-lto
ninja -v -j1000 -C out/fuchsia_debug_x64

Crashed command:

clang_x64/gen_snapshot --lazy_async_stacks --enable_mirrors=false --deterministic --snapshot_kind=core --vm_snapshot_data=gen/flutter/shell/platform/fuchsia/flutter/kernel/vm_isolate_snapshot.bin --vm_snapshot_instructions=gen/flutter/shell/platform/fuchsia/flutter/kernel/vm_snapshot_instructions.bin --isolate_snapshot_data=gen/flutter/shell/platform/fuchsia/flutter/kernel/isolate_snapshot.bin --isolate_snapshot_instructions=gen/flutter/shell/platform/fuchsia/flutter/kernel/isolate_snapshot_instructions.bin --write_v8_snapshot_profile_to=gen/flutter/shell/platform/fuchsia/flutter/kernel/snapshot_profile.json --enable_asserts /.../engine/src/out/fuchsia_debug_x64/flutter_runner_patched_sdk/platform_strong.dill

Got the following stack trace in debugger for this crash:

#0  dart::V8SnapshotProfileWriter::NodeInfo::WriteDebug (this=this@entry=0x7ffff7fc3258, profile_writer=profile_writer@entry=0x0, 
    writer=writer@entry=0x7fffffffcd70) at ../../third_party/dart/runtime/vm/v8_snapshot_writer.cc:143
#1  0x000000000056e332 in dart::V8SnapshotProfileWriter::NodeInfo::ToCString (this=0x7fffffffcd78, this@entry=0x7ffff7fc3258, 
    profile_writer=0x1, profile_writer@entry=0x0, zone=0x811750) at ../../third_party/dart/runtime/vm/v8_snapshot_writer.cc:111
#2  0x000000000056e150 in dart::V8SnapshotProfileWriter::SetObjectTypeAndName (this=0x7ffff7fba020, object_id=..., 
    type=0x216a4b "Code", name=0x220121 "<stub code>") at ../../third_party/dart/runtime/vm/v8_snapshot_writer.cc:44
#3  0x00000000003d5f9a in dart::Serializer::AddBaseObject (this=0x7fffffffcf40, base_object=..., type=<optimized out>, 
    name=<optimized out>) at ../../third_party/dart/runtime/vm/app_snapshot.cc:6087
#4  dart::VMSerializationRoots::AddBaseObjects (this=<optimized out>, s=0x7fffffffcf40)
    at ../../third_party/dart/runtime/vm/app_snapshot.cc:5530
#5  0x00000000003ae342 in dart::Serializer::Serialize (this=this@entry=0x7fffffffcf40, roots=roots@entry=0x7fffffffcf20)
    at ../../third_party/dart/runtime/vm/app_snapshot.cc:6874
#6  0x00000000003b1c15 in dart::FullSnapshotWriter::WriteVMSnapshot (this=this@entry=0x7fffffffd1e8)
    at ../../third_party/dart/runtime/vm/app_snapshot.cc:7992
#7  0x00000000003b243d in dart::FullSnapshotWriter::WriteFullSnapshot (this=0x7fffffffd1e8, data=0x0)
    at ../../third_party/dart/runtime/vm/app_snapshot.cc:8101
#8  0x000000000077c471 in Dart_CreateSnapshot (vm_snapshot_data_buffer=0x7fffffffd410, vm_snapshot_data_size=0x7fffffffd2f8, 
    isolate_snapshot_data_buffer=0x7fffffffd308, isolate_snapshot_data_size=0x7fffffffd300, is_core=false)
    at ../../third_party/dart/runtime/vm/dart_api_impl.cc:1904
#9  0x000000000035fb2a in dart::bin::CreateAndWriteCoreSnapshot () at ../../third_party/dart/runtime/bin/gen_snapshot.cc:405
#10 dart::bin::CreateIsolateAndSnapshot (inputs=...) at ../../third_party/dart/runtime/bin/gen_snapshot.cc:812
#11 dart::bin::main (argc=<optimized out>, argv=<optimized out>) at ../../third_party/dart/runtime/bin/gen_snapshot.cc:937
#12 0x00007ffff7c9ae4a in __libc_start_main (main=0x3605d0 <main(int, char**)>, argc=12, argv=0x7fffffffd548, init=<optimized out>, 
    fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd538) at ../csu/libc-start.c:314
#13 0x000000000035ee5a in _start ()

It looks like --write_v8_snapshot_profile_to doesn't work with core snapshots. /cc @sstrickl @rmacnak-google

The build completes successfully if --write_v8_snapshot_profile_to=... argument is removed.

[sstrickl]

Taking a look at this now.

[sstrickl]

So the problem is that Serializer::GetProfileId calls Serializer::UnsafeRefId, which always returns RefId(Object::null()) for Code objects in core snapshots, as they don't include code. This means the v8 snapshot profile writer sees multiple items with the same profile id but different names (the name for null and the name for the first Code object reached) and attempts to fail with FATAL, but then we end up crashing when trying to generate the message for FATAL.

Thus, there's two things that need doing here:

1. Fix the crash caused by the FATAL call here, since it shouldn't.
2. Fix the Serializer so that artificial nodes are created for Code objects for non-Code-carrying snapshots and have UnsafeRefId return that artificial id, so that things like GetProfileId that can handle artificial refs works, and move the code that returns RefId(Object::null()) into RefId itself (which is called in cases where artificial refs should not be returned).

Working on the CL now.

[sstrickl]

Actually, we could simplify the fix by not worrying about creating artificial nodes for Code objects, and instead just really treat them like null. Trying to do that first, unless we decide that we want the artificial nodes after all for some reason.

[sstrickl]

Actually, it's not even the handling of Code objects that's the issue here, but the fact that we always assign base objects that are Code (e.g., stubs) reference ids (which also sets the name and type of the profile object id), even though we'll never use those reference ids (because we use the reference id for null instead). So the simpler fix is to not just do that on either the serialization or deserialization side.

[sstrickl]

Fix for both crash and for removing assignments of reference ids to Code base objects in non-Code-including snapshots in https://dart-review.googlesource.com/c/sdk/+/217643.

[a-siva]

Did we come to a conclusion on dropping support for core-jit snapshots?
@chaselatta I presume Fuchsia has switched their build model to not use core-jit snapshots.

[chaselatta]

Sorry @a-siva I missed this message. We have not changed our snapshots at all. We are still using core-jit snapshots but we have a bug to look into switching it. Do you have a timeframe that you would like us to switch over?

[a-siva]

Sorry @a-siva I missed this message. We have not changed our snapshots at all. We are still using core-jit snapshots but we have a bug to look into switching it. Do you have a timeframe that you would like us to switch over?

We would like to remove it once Fuchsia switches to not using it. I believe you will not see any degradation by switching the Fuchsia builds to use core snapshots instead of the core-jit snapshots that you are using currenlty, and I presume you use this flag only in the debug builds.

[chaselatta]

Ok, we will prioritize the work then.