Use PE/COFF AOT snapshots for Windows x64#187594
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for compiling and linking Windows x64 AOT snapshots into a DLL using Visual Studio's linker. Feedback on the changes includes a recommendation to remove {BUILD_DIR}/app.pdb from the build target outputs to prevent breaking incremental builds when --split-debug-info is enabled. Additionally, it is suggested to simplify embedder.cc by removing the redundant ResolveAOTSnapshotSymbol helper function and calling ResolveSymbol directly.
| if (targetPlatform == TargetPlatform.windows_x64) ...const <Source>[ | ||
| Source.pattern('{BUILD_DIR}/app.o'), | ||
| Source.pattern('{BUILD_DIR}/app.pdb'), | ||
| ], |
There was a problem hiding this comment.
Listing {BUILD_DIR}/app.pdb in the target's outputs will break incremental builds when --split-debug-info is enabled.
When --split-debug-info is used, the PDB is written to the split symbols directory and any stale PDB in {BUILD_DIR} is deleted (as implemented in AOTSnapshotter.build). Because {BUILD_DIR}/app.pdb is listed as an output but does not exist after a split-debug-info build, the Flutter build system's dependency tracker will always treat this target as out-of-date and force a full AOT compilation on every subsequent build.
Since debug symbols are not required outputs for downstream targets (similar to how .dSYM or other debug symbols are not listed in outputs for iOS/macOS/Android), we should remove {BUILD_DIR}/app.pdb from the outputs list.
| if (targetPlatform == TargetPlatform.windows_x64) ...const <Source>[ | |
| Source.pattern('{BUILD_DIR}/app.o'), | |
| Source.pattern('{BUILD_DIR}/app.pdb'), | |
| ], | |
| if (targetPlatform == TargetPlatform.windows_x64) ...const <Source>[ | |
| Source.pattern('{BUILD_DIR}/app.o'), | |
| ], |
There was a problem hiding this comment.
Fixed in d9cefc7: {BUILD_DIR}/app.pdb is no longer listed as an AOT target output. The snapshotter still produces/copies PDBs through the existing linker/split-debug-info path, but the build dependency tracker no longer requires {BUILD_DIR}/app.pdb to exist after split-debug-info builds.
| static const uint8_t* ResolveAOTSnapshotSymbol( | ||
| const fml::RefPtr<fml::NativeLibrary>& library, | ||
| const char* symbol_name) { | ||
| return library->ResolveSymbol(symbol_name); | ||
| } |
There was a problem hiding this comment.
This helper function is redundant and can be removed since we can call ResolveSymbol directly on the NativeLibrary instance.
References
- Suggest simplification and refactoring to enhance readability and maintainability. (link)
There was a problem hiding this comment.
Fixed in d9cefc7: removed ResolveAOTSnapshotSymbol and call NativeLibrary::ResolveSymbol directly.
| aot_data->vm_snapshot_data = ResolveAOTSnapshotSymbol( | ||
| aot_data->native_library, "_kDartSnapshotData"); | ||
| aot_data->vm_snapshot_instrs = ResolveAOTSnapshotSymbol( | ||
| aot_data->native_library, "_kDartSnapshotText"); |
There was a problem hiding this comment.
The ResolveAOTSnapshotSymbol helper function is redundant as it only forwards the call to library->ResolveSymbol. We can simplify the code and improve maintainability by calling ResolveSymbol directly.
aot_data->vm_snapshot_data =
aot_data->native_library->ResolveSymbol("_kDartSnapshotData");
aot_data->vm_snapshot_instrs =
aot_data->native_library->ResolveSymbol("_kDartSnapshotText");References
- Suggest simplification and refactoring to enhance readability and maintainability. (link)
There was a problem hiding this comment.
Fixed in d9cefc7: both _kDartSnapshotData and _kDartSnapshotText now resolve directly through aot_data->native_library->ResolveSymbol(...).
d9cefc7 to
bf60403
Compare
bf60403 to
b5aed1f
Compare
Summary
Enables Windows x64 profile/release AOT bundles to consume a PE/COFF AOT DLL instead of the previous ELF snapshot path.
This change:
app-aot-pecoff-objfor Windows x64 onlylink.exe--split-debug-info, the PDB is written only to the split symbols directoryCompanion Dart SDK PR: dart-lang/sdk#63532
Validation
dart formatover the changed flutter_tools Dart files andclang-formatover the changed engine C++ files.python flutter/testing/run_tests.py --variant=host_debug_unopt --type=engine --engine-filter=flutter_windows_unittestsflutter_windows_unittests.exe --repeat=2ninja -C out/host_profile_unopt gen_snapshot flutter_windowsflutter createWindows x64 profile app with the local engine and--extra-gen-snapshot-options=--no-strip.build/windows/x64/runner/Profile/data/app.soisCOFF-x86-64and exports only_kDartSnapshotDataand_kDartSnapshotText.datadirectory contains no PDB; the CodeView debug directory points at.dart_tool/flutter_build/.../app.pdb.llvm-readobj --unwind app.soreports 8211RuntimeFunctionentries and 47 sharedUnwindInfoAddressvalues.cdbthat a breakpoint inapp!RenderObject.layout+0xbunwinds through Dart AOT frames, Flutter native frames, and Windows system frames.Representative
cdb kvstack excerpt: