Skip to content

Commit c6f55f7

Browse files
a-sivacommit-bot@chromium.org
authored andcommitted
[VM] Use the incremental compiler to compile sources only when the observatory
server is started. When the incremental compiler is used it accumulates state that increases the footprint of the process. This state is needed only when the 'reload' or 'expression evaluation' functionality is used in the debugger. Currently in the VM we do not have a good way of detecting when the debugger will be used as the vm service observatory functionality can be turned on dynamically after the program has started. This CL turns on the incremental compiler only when command line options are used to start the observatory server. We could run into issues with 'expression evaluation' when the observatory is started dynamically after the program is loaded. Change-Id: I18c17698622071bca428018f7fdac1a84b0f195e Reviewed-on: https://dart-review.googlesource.com/64667 Reviewed-by: Ryan Macnak <[email protected]> Commit-Queue: Siva Annamalai <[email protected]>
1 parent 7ebe83f commit c6f55f7

File tree

6 files changed

+41
-17
lines changed

6 files changed

+41
-17
lines changed

pkg/vm/bin/kernel_service.dart

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -305,18 +305,20 @@ void _recordDependencies(
305305
int isolateId, Component component, String packageConfig) {
306306
final dependencies = isolateDependencies[isolateId] ??= new List<Uri>();
307307

308-
for (var lib in component.libraries) {
309-
if (lib.importUri.scheme == "dart") continue;
310-
311-
dependencies.add(lib.fileUri);
312-
for (var part in lib.parts) {
313-
final fileUri = lib.fileUri.resolve(part.partUri);
314-
if (fileUri.scheme != "" && fileUri.scheme != "file") {
315-
// E.g. part 'package:foo/foo.dart';
316-
// Maybe the front end should resolve this?
317-
continue;
308+
if (component != null) {
309+
for (var lib in component.libraries) {
310+
if (lib.importUri.scheme == "dart") continue;
311+
312+
dependencies.add(lib.fileUri);
313+
for (var part in lib.parts) {
314+
final fileUri = lib.fileUri.resolve(part.partUri);
315+
if (fileUri.scheme != "" && fileUri.scheme != "file") {
316+
// E.g. part 'package:foo/foo.dart';
317+
// Maybe the front end should resolve this?
318+
continue;
319+
}
320+
dependencies.add(fileUri);
318321
}
319-
dependencies.add(fileUri);
320322
}
321323
}
322324

@@ -447,7 +449,7 @@ Future _processLoadRequest(request) async {
447449
FileSystem fileSystem = _buildFileSystem(
448450
sourceFiles, platformKernel, multirootFilepaths, multirootScheme);
449451
compiler = new SingleShotCompilerWrapper(fileSystem, platformKernelPath,
450-
requireMain: sourceFiles.isEmpty,
452+
requireMain: false,
451453
strongMode: strong,
452454
suppressWarnings: suppressWarnings,
453455
syncAsync: syncAsync,
@@ -461,12 +463,17 @@ Future _processLoadRequest(request) async {
461463
}
462464

463465
Component component = await compiler.compile(script);
464-
_recordDependencies(isolateId, component, packageConfig);
465466

466467
if (compiler.errors.isNotEmpty) {
467-
result = new CompilationResult.errors(compiler.errors,
468-
serializeComponent(component, filter: (lib) => !lib.isExternal));
468+
if (component != null) {
469+
result = new CompilationResult.errors(compiler.errors,
470+
serializeComponent(component, filter: (lib) => !lib.isExternal));
471+
} else {
472+
result = new CompilationResult.errors(compiler.errors, null);
473+
}
469474
} else {
475+
// Record dependencies only if compilation was error free.
476+
_recordDependencies(isolateId, component, packageConfig);
470477
// We serialize the component excluding vm_platform.dill because the VM has
471478
// these sources built-in. Everything loaded as a summary in
472479
// [kernelForProgram] is marked `external`, so we can use that bit to

runtime/bin/dfe.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ static char* GetDirectoryPrefixFromExeName() {
8181

8282
DFE::DFE()
8383
: use_dfe_(false),
84+
use_incremental_compiler_(false),
8485
frontend_filename_(NULL),
8586
application_kernel_buffer_(NULL),
8687
application_kernel_buffer_size_(0) {}
@@ -218,8 +219,8 @@ void DFE::CompileAndReadScript(const char* script_uri,
218219
int* exit_code,
219220
bool strong,
220221
const char* package_config) {
221-
Dart_KernelCompilationResult result =
222-
CompileScript(script_uri, strong, true, package_config);
222+
Dart_KernelCompilationResult result = CompileScript(
223+
script_uri, strong, use_incremental_compiler(), package_config);
223224
switch (result.status) {
224225
case Dart_KernelCompilationStatus_Ok:
225226
*kernel_buffer = result.kernel;

runtime/bin/dfe.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ class DFE {
3434
void set_use_dfe(bool value = true) { use_dfe_ = value; }
3535
bool UseDartFrontend() const { return use_dfe_; }
3636

37+
void set_use_incremental_compiler(bool value) {
38+
use_incremental_compiler_ = value;
39+
}
40+
bool use_incremental_compiler() const { return use_incremental_compiler_; }
41+
3742
// Returns the platform binary file name if the path to
3843
// kernel binaries was set using SetKernelBinaries.
3944
const char* GetPlatformBinaryFilename();
@@ -100,6 +105,7 @@ class DFE {
100105

101106
private:
102107
bool use_dfe_;
108+
bool use_incremental_compiler_;
103109
char* frontend_filename_;
104110

105111
// Kernel binary specified on the cmd line.

runtime/bin/main_options.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,9 @@ bool Options::ProcessEnableVmServiceOption(const char* arg,
296296
"Use --enable-vm-service[=<port number>[/<bind address>]]\n");
297297
return false;
298298
}
299+
#if !defined(DART_PRECOMPILED_RUNTIME)
300+
dfe()->set_use_incremental_compiler(true);
301+
#endif // !defined(DART_PRECOMPILED_RUNTIME)
299302

300303
return true;
301304
}
@@ -320,6 +323,9 @@ bool Options::ProcessObserveOption(const char* arg,
320323
vm_options->AddArgument("--pause-isolates-on-unhandled-exceptions");
321324
vm_options->AddArgument("--profiler");
322325
vm_options->AddArgument("--warn-on-pause-with-no-debugger");
326+
#if !defined(DART_PRECOMPILED_RUNTIME)
327+
dfe()->set_use_incremental_compiler(true);
328+
#endif // !defined(DART_PRECOMPILED_RUNTIME)
323329
return true;
324330
}
325331

tests/lib_2/mirrors/dynamic_load_test.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ main() async {
4343
}
4444
print(error);
4545
Expect.isTrue(error.toString().contains("Cannot open file") ||
46+
error.toString().contains("file not found") ||
4647
error.toString().contains("No such file or directory") ||
4748
error.toString().contains("The system cannot find the file specified"));
4849
Expect.isTrue(error.toString().contains("DOES_NOT_EXIST"));

tests/standalone_2/standalone_2_kernel.status

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,5 +203,8 @@ package/package_isolate_test: CompileTimeError
203203
[ $fasta && !$strong ]
204204
regress_29350_test/none: MissingCompileTimeError
205205

206+
[ $strong && ($compiler == dartk || $compiler == dartkp) ]
207+
io/named_pipe_script_test: RuntimeError # Issue 33842
208+
206209
[ !$strong && ($compiler == dartk || $compiler == dartkp || $runtime == dart_precompiled || $runtime == vm) ]
207210
*: SkipByDesign # standalone_2 is only supported in strong mode.

0 commit comments

Comments
 (0)