From 11ab2dfe2fa6bf1679086f3c02cdd8e94be9171b Mon Sep 17 00:00:00 2001 From: Ben Konyi Date: Thu, 20 Feb 2025 15:17:16 -0800 Subject: [PATCH] Revert "[SDK] Switch dds and dtd to use an AOT snapshot" This reverts commit 6450d76f1f7aee3ff4ee46c34a7a539418f2c1d8. Reason for revert: Breaking Flutter G3 roll Original change's description: > [SDK] Switch dds and dtd to use an AOT snapshot > > TEST=ci > > Change-Id: Ib65ca1d1a05d3bc7b5f5cab25d90fc459ec8d853 > Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/387133 > Reviewed-by: Ben Konyi > Commit-Queue: Siva Annamalai Change-Id: I9985919063cacfc8673b3e2946eaa163e90c9cc3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/411200 Auto-Submit: Ben Konyi Reviewed-by: Siva Annamalai Bot-Commit: Rubber Stamper Commit-Queue: Siva Annamalai --- .../lib/src/commands/development_service.dart | 58 +++----------- .../lib/src/commands/tooling_daemon.dart | 43 +++-------- pkg/dartdev/lib/src/dds_runner.dart | 22 ++---- pkg/dartdev/lib/src/sdk.dart | 4 - pkg/dds/lib/src/devtools/dtd.dart | 77 ++----------------- runtime/bin/process_win.cc | 41 +--------- sdk/BUILD.gn | 50 ++++-------- .../_internal/vm/bin/vmservice_server.dart | 44 ++--------- utils/dartdev/BUILD.gn | 16 +--- utils/dds/BUILD.gn | 26 ------- utils/dtd/BUILD.gn | 26 ------- 11 files changed, 64 insertions(+), 343 deletions(-) diff --git a/pkg/dartdev/lib/src/commands/development_service.dart b/pkg/dartdev/lib/src/commands/development_service.dart index ab6e3b151cb..fbc5f88033c 100644 --- a/pkg/dartdev/lib/src/commands/development_service.dart +++ b/pkg/dartdev/lib/src/commands/development_service.dart @@ -3,14 +3,12 @@ // BSD-style license that can be found in the LICENSE file. import 'dart:async'; -import 'dart:io'; import 'package:dds/src/arg_parser.dart'; -import 'package:path/path.dart'; import '../core.dart'; import '../sdk.dart'; -import '../vm_interop_handler.dart'; +import '../utils.dart'; class DevelopmentServiceCommand extends DartdevCommand { static const String commandName = 'development-service'; @@ -32,50 +30,14 @@ class DevelopmentServiceCommand extends DartdevCommand { @override Future run() async { - final sdkDir = dirname(sdk.dart); - final fullSdk = sdkDir.endsWith('bin'); - var script = fullSdk - ? sdk.dartAotRuntime - : absolute(sdkDir, 'dartaotruntime${Platform.isWindows ? '.exe' : ''}'); - var snapshot = fullSdk - ? sdk.ddsAotSnapshot - : absolute(sdkDir, 'gen', 'dds_aot.dart.snapshot'); - var useExecProcess = true; - final args = argResults!.arguments; - - if (!Sdk.checkArtifactExists(snapshot, logError: false)) { - // On ia32 platforms we do not have an AOT snapshot and so we need - // to run the JIT snapshot. - useExecProcess = false; - script = fullSdk - ? sdk.ddsSnapshot - : absolute(sdkDir, 'gen', 'dds.dart.snapshot'); - if (!Sdk.checkArtifactExists(script, logError: false)) { - log.stderr('Error: launching development server failed : ' - 'Unable to find snapshot for the development server'); - return 255; - } - } - final ddsCommand = [ - if (useExecProcess) snapshot, - // Add the remaining args. - if (args.isNotEmpty) ...args, - ]; - try { - VmInteropHandler.run( - script, - ddsCommand, - packageConfigOverride: null, - useExecProcess: useExecProcess, - ); - return 0; - } catch (e, st) { - log.stderr('Error: launching development server failed'); - log.stderr(e.toString()); - if (verbose) { - log.stderr(st.toString()); - } - return 255; - } + // Need to make a copy as argResults!.arguments is an + // UnmodifiableListView object which cannot be passed as + // the args for spawnUri. + final args = [...argResults!.arguments]; + return await runFromSnapshot( + snapshot: sdk.ddsSnapshot, + args: args, + verbose: verbose, + ); } } diff --git a/pkg/dartdev/lib/src/commands/tooling_daemon.dart b/pkg/dartdev/lib/src/commands/tooling_daemon.dart index ea367d64802..9d87ea543af 100644 --- a/pkg/dartdev/lib/src/commands/tooling_daemon.dart +++ b/pkg/dartdev/lib/src/commands/tooling_daemon.dart @@ -8,7 +8,7 @@ import 'package:dtd_impl/dtd.dart' as dtd show DartToolingDaemonOptions; import '../core.dart'; import '../sdk.dart'; -import '../vm_interop_handler.dart'; +import '../utils.dart'; class ToolingDaemonCommand extends DartdevCommand { static const String commandName = 'tooling-daemon'; @@ -30,37 +30,14 @@ class ToolingDaemonCommand extends DartdevCommand { @override Future run() async { - var script = sdk.dartAotRuntime; - var snapshot = sdk.dtdAotSnapshot; - var useExecProcess = true; - final args = argResults!.arguments; - - if (!Sdk.checkArtifactExists(sdk.dtdAotSnapshot, logError: false)) { - // On ia32 platforms we do not have an AOT snapshot and so we need - // to run the JIT snapshot. - useExecProcess = false; - script = sdk.dtdSnapshot; - } - final dtdCommand = [ - if (useExecProcess) snapshot, - // Add the remaining args. - if (args.isNotEmpty) ...args, - ]; - try { - VmInteropHandler.run( - script, - dtdCommand, - packageConfigOverride: null, - useExecProcess : useExecProcess, - ); - return 0; - } catch (e, st) { - log.stderr('Error: launching tooling daemon failed'); - log.stderr(e.toString()); - if (verbose) { - log.stderr(st.toString()); - } - return 255; - } + // Need to make a copy as argResults!.arguments is an + // UnmodifiableListView object which cannot be passed as + // the args for spawnUri. + final args = [...argResults!.arguments]; + return await runFromSnapshot( + snapshot: sdk.dtdSnapshot, + args: args, + verbose: verbose, + ); } } diff --git a/pkg/dartdev/lib/src/dds_runner.dart b/pkg/dartdev/lib/src/dds_runner.dart index 22945a732b9..1f96feefad3 100644 --- a/pkg/dartdev/lib/src/dds_runner.dart +++ b/pkg/dartdev/lib/src/dds_runner.dart @@ -24,23 +24,11 @@ class DDSRunner { }) async { final sdkDir = dirname(sdk.dart); final fullSdk = sdkDir.endsWith('bin'); - var execName = fullSdk - ? sdk.dartAotRuntime - : absolute(sdkDir, 'dartaotruntime${Platform.isWindows ? '.exe' : ''}'); - var snapshotName = fullSdk - ? sdk.ddsAotSnapshot - : absolute(sdkDir, 'gen', 'dds_aot.dart.snapshot'); - final isAot = Sdk.checkArtifactExists(snapshotName) ? true : false; - if (!isAot) { - // On ia32 sdks we do not have an AOT runtime and so we would be - // using the regular executable. - snapshotName = fullSdk - ? sdk.ddsSnapshot - : absolute(sdkDir, 'gen', 'dds.dart.snapshot'); - if (!Sdk.checkArtifactExists(snapshotName)) { - return false; - } - execName = sdk.dart; + final execName = sdk.dart; + final snapshotName = + fullSdk ? sdk.ddsSnapshot : absolute(sdkDir, 'dds.dart.snapshot'); + if (!Sdk.checkArtifactExists(snapshotName)) { + return false; } final process = await Process.start( diff --git a/pkg/dartdev/lib/src/sdk.dart b/pkg/dartdev/lib/src/sdk.dart index d6bc329126a..a49a282632e 100644 --- a/pkg/dartdev/lib/src/sdk.dart +++ b/pkg/dartdev/lib/src/sdk.dart @@ -119,10 +119,6 @@ class Sdk { 'dart_tooling_daemon.dart.snapshot', ); - String get dtdAotSnapshot => _snapshotPathFor( - 'dart_tooling_daemon_aot.dart.snapshot', - ); - String get devToolsBinaries => path.absolute( _runFromBuildRoot ? sdkPath diff --git a/pkg/dds/lib/src/devtools/dtd.dart b/pkg/dds/lib/src/devtools/dtd.dart index 72b06ed5a70..38d2c210faa 100644 --- a/pkg/dds/lib/src/devtools/dtd.dart +++ b/pkg/dds/lib/src/devtools/dtd.dart @@ -12,64 +12,15 @@ import 'package:path/path.dart' as path; import 'utils.dart'; -String getDTDSnapshotDir() { - // This logic is originally from pkg/dartdev/lib/src/sdk.dart - // - // Find SDK path. - (String, bool)? trySDKPath(String executablePath) { - // The common case, and how cli_util.dart computes the Dart SDK directory, - // [path.dirname] called twice on Platform.executable. We confirm by - // asserting that the directory `./bin/snapshots/` exists in this directory: - var sdkPath = path.absolute(path.dirname(path.dirname(executablePath))); - var snapshotsDir = path.join(sdkPath, 'bin', 'snapshots'); - var runFromBuildRoot = false; - final type = FileSystemEntity.typeSync(snapshotsDir); - if (type != FileSystemEntityType.directory && - type != FileSystemEntityType.link) { - // This is the less common case where the user is in - // the checked out Dart SDK, and is executing `dart` via: - // ./out/ReleaseX64/dart ... or in google3. - sdkPath = path.absolute(path.dirname(executablePath)); - snapshotsDir = sdkPath; - runFromBuildRoot = true; - } - - // Try to locate the DartDev snapshot to determine if we're able to find - // the SDK snapshots with this SDK path. This is meant to handle - // non-standard SDK layouts that can involve symlinks (e.g., Brew - // installations, google3 tests, etc). - if (!File( - path.join(snapshotsDir, 'dartdev.dart.snapshot'), - ).existsSync()) { - return null; - } - return (sdkPath, runFromBuildRoot); - } - - final (sdkPath, runFromBuildRoot) = trySDKPath(Platform.resolvedExecutable) ?? - trySDKPath(Platform.executable)!; - - final String snapshotDir; - if (runFromBuildRoot) { - snapshotDir = sdkPath; - } else { - snapshotDir = path.absolute(sdkPath, 'bin', 'snapshots'); - } - - return snapshotDir; -} - Future startDtd({ required bool machineMode, required bool printDtdUri, }) async { - final snapshotDir = getDTDSnapshotDir(); - final dtdAotSnapshot = path.absolute( - snapshotDir, - 'dart_tooling_daemon_aot.dart.snapshot', - ); - final dtdSnapshot = path.absolute( - snapshotDir, + final sdkPath = File(Platform.resolvedExecutable).parent.parent.path; + final dtdSnapshot = path.absolute( + sdkPath, + 'bin', + 'snapshots', 'dart_tooling_daemon.dart.snapshot', ); @@ -114,29 +65,15 @@ Future startDtd({ }); try { - // Try to spawn an isolate using the AOT snapshot of the tooling daemon. await Isolate.spawnUri( - Uri.file(dtdAotSnapshot), + Uri.file(dtdSnapshot), ['--machine'], receivePort.sendPort, onExit: exitPort.sendPort, onError: errorPort.sendPort, ); } catch (_, __) { - // Spawning an isolate using the AOT snapshot of the tooling daemon failed, - // we are probably in a JIT VM, try again using the JIT snapshot of the - // tooling daemon. - try { - await Isolate.spawnUri( - Uri.file(dtdSnapshot), - ['--machine'], - receivePort.sendPort, - onExit: exitPort.sendPort, - onError: errorPort.sendPort, - ); - } catch (_, __) { - completeForError(); - } + completeForError(); } final result = await completer.future.timeout( diff --git a/runtime/bin/process_win.cc b/runtime/bin/process_win.cc index 43059b0d4dc..df4061ebdbf 100644 --- a/runtime/bin/process_win.cc +++ b/runtime/bin/process_win.cc @@ -573,45 +573,13 @@ class ProcessStarter { } int StartForExec() { - ASSERT(mode_ == kInheritStdio); - ASSERT(Process::ModeIsAttached(mode_)); - ASSERT(!Process::ModeHasStdio(mode_)); - // Setup info STARTUPINFOEXW startup_info; ZeroMemory(&startup_info, sizeof(startup_info)); startup_info.StartupInfo.cb = sizeof(startup_info); - - // Setup the handles to inherit. We only want to inherit the three - // handles for stdin, stdout and stderr. - HANDLE stdin_handle = GetStdHandle(STD_INPUT_HANDLE); - HANDLE stdout_handle = GetStdHandle(STD_OUTPUT_HANDLE); - HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); - startup_info.StartupInfo.hStdInput = stdin_handle; - startup_info.StartupInfo.hStdOutput = stdout_handle; - startup_info.StartupInfo.hStdError = stderr_handle; - startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES; - SIZE_T size = 0; - // The call to determine the size of an attribute list always fails with - // ERROR_INSUFFICIENT_BUFFER and that error should be ignored. - if (!InitializeProcThreadAttributeList(nullptr, 1, 0, &size) && - (GetLastError() != ERROR_INSUFFICIENT_BUFFER)) { - return CleanupAndReturnError(); - } - attribute_list_ = reinterpret_cast( - Dart_ScopeAllocate(size)); - ZeroMemory(attribute_list_, size); - if (!InitializeProcThreadAttributeList(attribute_list_, 1, 0, &size)) { - return CleanupAndReturnError(); - } - inherited_handles_ = {stdin_handle, stdout_handle, stderr_handle}; - if (!UpdateProcThreadAttribute( - attribute_list_, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, - inherited_handles_.data(), - inherited_handles_.size() * sizeof(HANDLE), nullptr, nullptr)) { - return CleanupAndReturnError(); - } - startup_info.lpAttributeList = attribute_list_; + ASSERT(mode_ == kInheritStdio); + ASSERT(Process::ModeIsAttached(mode_)); + ASSERT(!Process::ModeHasStdio(mode_)); PROCESS_INFORMATION process_info; ZeroMemory(&process_info, sizeof(process_info)); @@ -633,9 +601,6 @@ class ProcessStarter { } child_process_handle_ = process_info.hProcess; CloseHandle(process_info.hThread); - CloseHandle(stdin_handle); - CloseHandle(stdout_handle); - CloseHandle(stderr_handle); // Return process id. *id_ = process_info.dwProcessId; diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index 7e1bad90dcb..9122a88fc18 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -49,10 +49,8 @@ declare_args() { # ........dartdev.dart.snapshot (app-jit snapshot or kernel dill file) # ........dartdevc_aot.dart.snapshot (AOT snapshot, if not on ia32) # ........dartdevc.dart.snapshot (JIT snapshot only on ia32) -# ........dds_aot.dart.snapshot (AOT snapshot, if not on ia32) -# ........dds.dart.snapshot (JIT snapshot only on ia32) -# ........dart_tooling_daemon_aot.dart.snapshot (AOT snapshot, if not on ia32) -# ........dart_tooling_daemon.dart.snapshot (JIT snapshot only on ia32) +# ........dds.dart.snapshot +# ........dart_tooling_daemon.dart.snapshot # ........frontend_server_aot.dart.snapshot (AOT snapshot, if not on ia32) # ........frontend_server.dart.snapshot (JIT snapshot only on ia32) # ........gen_kernel_aot.dart.snapshot (if not on ia32) @@ -130,38 +128,24 @@ _platform_sdk_snapshots = [ "../utils/dtd:dtd", "dart_tooling_daemon", ], + [ + "dds", + "../utils/dds:dds", + "dds", + ], ] if (dart_target_arch != "ia32" && dart_target_arch != "x86") { - _platform_sdk_snapshots += [ - [ - "frontend_server_aot_product", - "../utils/kernel-service:frontend_server_aot_product", - "frontend_server_aot", - ], - [ - "dds_aot_product", - "../utils/dds:dds_aot", - "dds_aot", - ], - [ - "dart_tooling_daemon_aot_product", - "../utils/dtd:dtd_aot", - "dart_tooling_daemon_aot", - ], - ] + _platform_sdk_snapshots += [ [ + "frontend_server_aot_product", + "../utils/kernel-service:frontend_server_aot_product", + "frontend_server_aot", + ] ] } else { - _platform_sdk_snapshots += [ - [ - "frontend_server", - "../utils/kernel-service:frontend_server", - "frontend_server", - ], - [ - "dds", - "../utils/dds:dds", - "dds", - ], - ] + _platform_sdk_snapshots += [ [ + "frontend_server", + "../utils/kernel-service:frontend_server", + "frontend_server", + ] ] } if (dart_snapshot_kind == "app-jit") { _platform_sdk_snapshots += [ [ diff --git a/sdk/lib/_internal/vm/bin/vmservice_server.dart b/sdk/lib/_internal/vm/bin/vmservice_server.dart index e6455a0c436..cea7785b039 100644 --- a/sdk/lib/_internal/vm/bin/vmservice_server.dart +++ b/sdk/lib/_internal/vm/bin/vmservice_server.dart @@ -149,43 +149,20 @@ class _DebuggingSession { bool disableServiceAuthCodes, bool enableDevTools, ) async { - // This code is part of the SDK and it is ok to have a reference to the - // internals of the Dart SDK in terms of location of the snapshot etc. - // It is more efficient doing it this way instead of invoking the Dart CLI - // with the 'development-service' command which would then dispatch to the - // Dart AOT runtime. final dartDir = File(Platform.executable).parent.path; - final suffix = Platform.isWindows ? '.exe' : ''; - final dartAotRuntime = 'dartaotruntime${suffix}'; - final dart = 'dart${suffix}'; - var executable = [dartDir, dartAotRuntime].join(Platform.pathSeparator); - var script = [ - dartDir, - 'snapshots', - 'dds_aot.dart.snapshot', - ].join(Platform.pathSeparator); - if (FileSystemEntity.typeSync(script) == FileSystemEntityType.notFound) { - script = [ - dartDir, - 'gen', - 'dds_aot.dart.snapshot', - ].join(Platform.pathSeparator); - if (FileSystemEntity.typeSync(script) == FileSystemEntityType.notFound) { - executable = [dartDir, dart].join(Platform.pathSeparator); - script = 'development-service'; - } - } + final dart = 'dart${Platform.isWindows ? '.exe' : ''}'; + var executable = [dartDir, dart].join(Platform.pathSeparator); // If the directory of dart is '.' it's likely that dart is on the user's // PATH. If so, './dart' might not exist and we should be using 'dart' // instead. if (dartDir == '.' && - (FileSystemEntity.typeSync(executable)) == + (await FileSystemEntity.type(executable)) == FileSystemEntityType.notFound) { executable = dart; } - var process = await Process.start(executable, [ - script, + _process = await Process.start(executable, [ + 'development-service', '--vm-service-uri=$serverAddress', '--bind-address=$host', '--bind-port=$port', @@ -193,14 +170,9 @@ class _DebuggingSession { if (enableDevTools) '--serve-devtools', if (_enableServicePortFallback) '--enable-service-port-fallback', ], mode: ProcessStartMode.detachedWithStdio); - if (process == null) { - stderr.writeln('Could not start the VM service: Process.start failed\n'); - return false; - } - _process = process; // DDS will close stderr once it's finished launching. - final launchResult = await _process.stderr.transform(utf8.decoder).join(); + final launchResult = await _process!.stderr.transform(utf8.decoder).join(); void printError(String details) => stderr.writeln('Could not start the VM service:\n$details'); @@ -231,9 +203,9 @@ class _DebuggingSession { return true; } - void shutdown() => _process.kill(); + void shutdown() => _process!.kill(); - late Process _process; + Process? _process; } class Server { diff --git a/utils/dartdev/BUILD.gn b/utils/dartdev/BUILD.gn index ef949f3a748..5cd9e5e38ef 100644 --- a/utils/dartdev/BUILD.gn +++ b/utils/dartdev/BUILD.gn @@ -23,18 +23,10 @@ application_snapshot("generate_dartdev_snapshot") { main_dart = "../../pkg/dartdev/bin/dartdev.dart" training_args = [ "--help" ] - if (dart_target_arch != "ia32" && dart_target_arch != "x86") { - deps = [ - "../..:runtime_precompiled", - "../dds:dds_aot", - "../dtd:dtd_aot", - ] - } else { - deps = [ - "../dds:dds", - "../dtd:dtd", - ] - } + deps = [ + "../dds:dds", + "../dtd:dtd", + ] vm_args = [] output = "$root_gen_dir/dartdev.dart.snapshot" diff --git a/utils/dds/BUILD.gn b/utils/dds/BUILD.gn index 5e9c5307cfe..4b07b103c81 100644 --- a/utils/dds/BUILD.gn +++ b/utils/dds/BUILD.gn @@ -2,35 +2,9 @@ # for details. All rights reserved. Use of this source code is governed by a # BSD-style license that can be found in the LICENSE file. -import("../../runtime/runtime_args.gni") import("../aot_snapshot.gni") import("../application_snapshot.gni") -group("dds_aot") { - public_deps = [ - ":dds_aot_product_snapshot", - ":dds_aot_snapshot", - ] -} - -aot_snapshot("dds_aot_snapshot") { - main_dart = "../../pkg/dds/bin/dds.dart" - output = "$root_gen_dir/dds_aot.dart.snapshot" -} - -aot_snapshot("dds_aot_product_snapshot") { - main_dart = "../../pkg/dds/bin/dds.dart" - output = "$root_gen_dir/dds_aot_product.dart.snapshot" - - # dartaotruntime has dart_product_config applied to it, - # so it is built in product mode in both release and - # product builds, and is only built in debug mode in debug - # builds. The following line ensures that the dartaotruntime - # and dartdevc_aot snapshot in an SDK build are - # always compatible with each other. - force_product_mode = !dart_debug -} - group("dds") { public_deps = [ ":copy_dds_snapshot" ] } diff --git a/utils/dtd/BUILD.gn b/utils/dtd/BUILD.gn index 91f8c6b2f7c..876a7c292d6 100644 --- a/utils/dtd/BUILD.gn +++ b/utils/dtd/BUILD.gn @@ -2,35 +2,9 @@ # for details. All rights reserved. Use of this source code is governed by a # BSD-style license that can be found in the LICENSE file. -import("../../runtime/runtime_args.gni") import("../aot_snapshot.gni") import("../application_snapshot.gni") -group("dtd_aot") { - public_deps = [ - ":dtd_aot_product_snapshot", - ":dtd_aot_snapshot", - ] -} - -aot_snapshot("dtd_aot_snapshot") { - main_dart = "../../pkg/dtd_impl/bin/dtd.dart" - output = "$root_gen_dir/dart_tooling_daemon_aot.dart.snapshot" -} - -aot_snapshot("dtd_aot_product_snapshot") { - main_dart = "../../pkg/dtd_impl/bin/dtd.dart" - output = "$root_gen_dir/dart_tooling_daemon_aot_product.dart.snapshot" - - # dartaotruntime has dart_product_config applied to it, - # so it is built in product mode in both release and - # product builds, and is only built in debug mode in debug - # builds. The following line ensures that the dartaotruntime - # and dartdevc_aot snapshot in an SDK build are - # always compatible with each other. - force_product_mode = !dart_debug -} - group("dtd") { public_deps = [ ":copy_dtd_snapshot" ] }