This relands "Load isolate from parent's kernel in Isolate.spawn calls.".

Original PR was reverted because it broke hot-reload bots. This CL adds steps to initialize kernel_service compiler for child isolate. Compiler is needed to support hot-reload request. Child isolate's compiler is created from full kernel file produced by main isolate's compiler. Changes since original PR are pkg/vm/bin/kernel_service.dart in pkg/vm/lib/incremental_compiler.dart.

Further this CL changes kernel fingerprint calculation for interface types so it calculates the hash of the canonical names themselves, rather than indices(that might change from one compilation to another).

This reverts commit 63fd8f63e6.

Change-Id: I6fe5b2ef99f209b32cd4087dfd1c8cac229c2d8b
Reviewed-on: https://dart-review.googlesource.com/c/87265
Commit-Queue: Alexander Aprelev <aam@google.com>
Reviewed-by: Liam Appelbe <liama@google.com>
This commit is contained in:
Alexander Aprelev 2018-12-14 23:45:57 +00:00 committed by commit-bot@chromium.org
parent c91a245e30
commit 7d14785115
14 changed files with 245 additions and 47 deletions

View file

@ -27,7 +27,9 @@ import 'dart:isolate';
import 'dart:typed_data' show Uint8List;
import 'package:build_integration/file_system/multi_root.dart';
import 'package:front_end/src/api_prototype/memory_file_system.dart';
import 'package:front_end/src/api_unstable/vm.dart';
import 'package:kernel/binary/ast_to_binary.dart';
import 'package:kernel/kernel.dart' show Component, Procedure;
import 'package:kernel/target/targets.dart' show TargetFlags;
import 'package:vm/bytecode/gen_bytecode.dart' show generateBytecode;
@ -63,14 +65,19 @@ bool allowDartInternalImport = false;
abstract class Compiler {
final FileSystem fileSystem;
final Uri platformKernelPath;
bool suppressWarnings;
bool bytecode;
String packageConfig;
final List<String> errors = new List<String>();
CompilerOptions options;
Compiler(this.fileSystem, Uri platformKernelPath,
{bool suppressWarnings: false,
bool bytecode: false,
String packageConfig: null}) {
Compiler(this.fileSystem, this.platformKernelPath,
{this.suppressWarnings: false,
this.bytecode: false,
this.packageConfig: null}) {
Uri packagesUri = null;
if (packageConfig != null) {
packagesUri = Uri.parse(packageConfig);
@ -134,6 +141,23 @@ abstract class Compiler {
Future<Component> compileInternal(Uri script);
}
class FileSink implements Sink<List<int>> {
MemoryFileSystemEntity entityForUri;
List<int> bytes = <int>[];
FileSink(this.entityForUri);
@override
void add(List<int> data) {
bytes.addAll(data);
}
@override
void close() {
this.entityForUri.writeAsBytesSync(bytes);
}
}
class IncrementalCompilerWrapper extends Compiler {
IncrementalCompiler generator;
@ -157,6 +181,30 @@ class IncrementalCompilerWrapper extends Compiler {
void accept() => generator.accept();
void invalidate(Uri uri) => generator.invalidate(uri);
Future<IncrementalCompilerWrapper> clone(int isolateId) async {
IncrementalCompilerWrapper clone = IncrementalCompilerWrapper(
fileSystem, platformKernelPath,
suppressWarnings: suppressWarnings,
bytecode: bytecode,
packageConfig: packageConfig);
generator.resetDeltaState();
Component fullComponent = await generator.compile();
// Assume fileSystem is HybridFileSystem because that is the setup where
// clone should be used for.
MemoryFileSystem memoryFileSystem = (fileSystem as HybridFileSystem).memory;
String filename = 'full-component-$isolateId.dill';
Sink sink = FileSink(memoryFileSystem.entityForUri(Uri.file(filename)));
new BinaryPrinter(sink).writeComponentFile(fullComponent);
await sink.close();
clone.generator = new IncrementalCompiler(options, generator.entryPoint,
initializeFromDillUri: Uri.file(filename));
return clone;
}
}
class SingleShotCompilerWrapper extends Compiler {
@ -201,17 +249,28 @@ Future<Compiler> lookupOrBuildNewIncrementalCompiler(int isolateId,
updateSources(compiler, sourceFiles);
invalidateSources(compiler, sourceFiles);
} else {
FileSystem fileSystem = _buildFileSystem(
sourceFiles, platformKernel, multirootFilepaths, multirootScheme);
// This is how identify scenario where child isolate hot reload requests
// requires setting up actual compiler first: non-empty sourceFiles list has
// no actual content specified for the source file.
if (sourceFiles != null &&
sourceFiles.length > 0 &&
sourceFiles[1] == null) {
// Just use first compiler that should represent main isolate as a source for cloning.
var source = isolateCompilers.entries.first;
compiler = await source.value.clone(isolateId);
} else {
FileSystem fileSystem = _buildFileSystem(
sourceFiles, platformKernel, multirootFilepaths, multirootScheme);
// TODO(aam): IncrementalCompilerWrapper instance created below have to be
// destroyed when corresponding isolate is shut down. To achieve that kernel
// isolate needs to receive a message indicating that particular
// isolate was shut down. Message should be handled here in this script.
compiler = new IncrementalCompilerWrapper(fileSystem, platformKernelPath,
suppressWarnings: suppressWarnings,
bytecode: bytecode,
packageConfig: packageConfig);
// TODO(aam): IncrementalCompilerWrapper instance created below have to be
// destroyed when corresponding isolate is shut down. To achieve that kernel
// isolate needs to receive a message indicating that particular
// isolate was shut down. Message should be handled here in this script.
compiler = new IncrementalCompilerWrapper(fileSystem, platformKernelPath,
suppressWarnings: suppressWarnings,
bytecode: bytecode,
packageConfig: packageConfig);
}
isolateCompilers[isolateId] = compiler;
}
return compiler;

View file

@ -32,6 +32,8 @@ class IncrementalCompiler {
Uri initializeFromDillUri;
Uri _entryPoint;
Uri get entryPoint => _entryPoint;
IncrementalCompiler(this._compilerOptions, this._entryPoint,
{this.initializeFromDillUri}) {
_generator = new IncrementalKernelGenerator(
@ -44,6 +46,7 @@ class IncrementalCompiler {
/// If [entryPoint] is specified, that points to new entry point for the
/// compilation. Otherwise, previously set entryPoint is used.
Future<Component> compile({Uri entryPoint}) async {
_entryPoint = entryPoint ?? _entryPoint;
Component component = await _generator.computeDelta(
entryPoint: entryPoint, fullComponent: fullComponent);
initialized = true;

View file

@ -21,8 +21,7 @@ IsolateData::IsolateData(const char* url,
dependencies_(NULL),
resolved_packages_config_(NULL),
kernel_buffer_(NULL),
kernel_buffer_size_(0),
owns_kernel_buffer_(false) {
kernel_buffer_size_(0) {
if (package_root != NULL) {
ASSERT(packages_file == NULL);
this->package_root = strdup(package_root);
@ -43,10 +42,6 @@ IsolateData::~IsolateData() {
packages_file = NULL;
free(resolved_packages_config_);
resolved_packages_config_ = NULL;
if (owns_kernel_buffer_) {
ASSERT(kernel_buffer_ != NULL);
free(kernel_buffer_);
}
kernel_buffer_ = NULL;
kernel_buffer_size_ = 0;
delete app_snapshot_;

View file

@ -5,6 +5,9 @@
#ifndef RUNTIME_BIN_ISOLATE_DATA_H_
#define RUNTIME_BIN_ISOLATE_DATA_H_
#include <memory>
#include <utility>
#include "include/dart_api.h"
#include "platform/assert.h"
#include "platform/globals.h"
@ -40,13 +43,37 @@ class IsolateData {
char* package_root;
char* packages_file;
const uint8_t* kernel_buffer() const { return kernel_buffer_; }
const std::shared_ptr<uint8_t>& kernel_buffer() const {
return kernel_buffer_;
}
intptr_t kernel_buffer_size() const { return kernel_buffer_size_; }
void set_kernel_buffer(uint8_t* buffer, intptr_t size, bool take_ownership) {
ASSERT(kernel_buffer_ == NULL);
kernel_buffer_ = buffer;
// Associate the given kernel buffer with this IsolateData without giving it
// ownership of the buffer.
void SetKernelBufferUnowned(uint8_t* buffer, intptr_t size) {
ASSERT(kernel_buffer_.get() == NULL);
kernel_buffer_ = std::shared_ptr<uint8_t>(buffer, FreeUnownedKernelBuffer);
kernel_buffer_size_ = size;
}
// Associate the given kernel buffer with this IsolateData and give it
// ownership of the buffer. This IsolateData is the first one to own the
// buffer.
void SetKernelBufferNewlyOwned(uint8_t* buffer, intptr_t size) {
ASSERT(kernel_buffer_.get() == NULL);
kernel_buffer_ = std::shared_ptr<uint8_t>(buffer, free);
kernel_buffer_size_ = size;
}
// Associate the given kernel buffer with this IsolateData and give it
// ownership of the buffer. The buffer is already owned by another
// IsolateData.
void SetKernelBufferAlreadyOwned(std::shared_ptr<uint8_t> buffer,
intptr_t size) {
ASSERT(kernel_buffer_.get() == NULL);
kernel_buffer_ = std::move(buffer);
kernel_buffer_size_ = size;
owns_kernel_buffer_ = take_ownership;
}
void UpdatePackagesFile(const char* packages_file_) {
@ -91,9 +118,10 @@ class IsolateData {
AppSnapshot* app_snapshot_;
MallocGrowableArray<char*>* dependencies_;
char* resolved_packages_config_;
uint8_t* kernel_buffer_;
std::shared_ptr<uint8_t> kernel_buffer_;
intptr_t kernel_buffer_size_;
bool owns_kernel_buffer_;
static void FreeUnownedKernelBuffer(uint8_t*) {}
DISALLOW_COPY_AND_ASSIGN(IsolateData);
};

View file

@ -5,6 +5,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <memory>
#include "include/bin/dart_io_api.h"
#include "include/dart_api.h"
@ -213,7 +214,7 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
#if !defined(DART_PRECOMPILED_RUNTIME)
IsolateData* isolate_data =
reinterpret_cast<IsolateData*>(Dart_IsolateData(isolate));
const uint8_t* kernel_buffer = isolate_data->kernel_buffer();
const uint8_t* kernel_buffer = isolate_data->kernel_buffer().get();
intptr_t kernel_buffer_size = isolate_data->kernel_buffer_size();
#endif
@ -274,9 +275,8 @@ static Dart_Isolate IsolateSetupHelper(Dart_Isolate isolate,
Dart_ShutdownIsolate();
return NULL;
}
isolate_data->set_kernel_buffer(application_kernel_buffer,
application_kernel_buffer_size,
true /*take ownership*/);
isolate_data->SetKernelBufferNewlyOwned(application_kernel_buffer,
application_kernel_buffer_size);
kernel_buffer = application_kernel_buffer;
kernel_buffer_size = application_kernel_buffer_size;
}
@ -433,9 +433,9 @@ static Dart_Isolate CreateAndSetupKernelIsolate(const char* script_uri,
dfe.LoadKernelService(&kernel_service_buffer, &kernel_service_buffer_size);
ASSERT(kernel_service_buffer != NULL);
isolate_data = new IsolateData(uri, package_root, packages_config, NULL);
isolate_data->set_kernel_buffer(const_cast<uint8_t*>(kernel_service_buffer),
kernel_service_buffer_size,
false /* take_ownership */);
isolate_data->SetKernelBufferUnowned(
const_cast<uint8_t*>(kernel_service_buffer),
kernel_service_buffer_size);
isolate = Dart_CreateIsolateFromKernel(
DART_KERNEL_ISOLATE_NAME, main, kernel_service_buffer,
kernel_service_buffer_size, flags, isolate_data, error);
@ -527,11 +527,13 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
const char* package_root,
const char* packages_config,
Dart_IsolateFlags* flags,
void* callback_data,
char** error,
int* exit_code) {
int64_t start = Dart_TimelineGetMicros();
ASSERT(script_uri != NULL);
uint8_t* kernel_buffer = NULL;
std::shared_ptr<uint8_t> parent_kernel_buffer;
intptr_t kernel_buffer_size = 0;
AppSnapshot* app_snapshot = NULL;
@ -565,7 +567,16 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
&isolate_snapshot_data, &isolate_snapshot_instructions);
}
}
if (!isolate_run_app_snapshot) {
if (flags->copy_parent_code && callback_data) {
IsolateData* parent_isolate_data =
reinterpret_cast<IsolateData*>(callback_data);
parent_kernel_buffer = parent_isolate_data->kernel_buffer();
kernel_buffer = parent_kernel_buffer.get();
kernel_buffer_size = parent_isolate_data->kernel_buffer_size();
}
if (kernel_buffer == NULL && !isolate_run_app_snapshot) {
dfe.ReadScript(script_uri, &kernel_buffer, &kernel_buffer_size);
}
#endif // !defined(DART_PRECOMPILED_RUNTIME)
@ -573,8 +584,13 @@ static Dart_Isolate CreateIsolateAndSetupHelper(bool is_main_isolate,
IsolateData* isolate_data =
new IsolateData(script_uri, package_root, packages_config, app_snapshot);
if (kernel_buffer != NULL) {
isolate_data->set_kernel_buffer(kernel_buffer, kernel_buffer_size,
true /*take ownership*/);
if (parent_kernel_buffer) {
isolate_data->SetKernelBufferAlreadyOwned(std::move(parent_kernel_buffer),
kernel_buffer_size);
} else {
isolate_data->SetKernelBufferNewlyOwned(kernel_buffer,
kernel_buffer_size);
}
}
if (is_main_isolate && (Options::depfile() != NULL)) {
isolate_data->set_dependencies(new MallocGrowableArray<char*>());
@ -640,7 +656,7 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
const char* package_root,
const char* package_config,
Dart_IsolateFlags* flags,
void* data,
void* callback_data,
char** error) {
// The VM should never call the isolate helper with a NULL flags.
ASSERT(flags != NULL);
@ -667,8 +683,8 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
}
bool is_main_isolate = false;
return CreateIsolateAndSetupHelper(is_main_isolate, script_uri, main,
package_root, package_config, flags, error,
&exit_code);
package_root, package_config, flags,
callback_data, error, &exit_code);
}
char* BuildIsolateName(const char* script_name, const char* func_name) {
@ -802,7 +818,8 @@ bool RunMainIsolate(const char* script_name, CommandLineOptions* dart_options) {
Dart_Isolate isolate = CreateIsolateAndSetupHelper(
is_main_isolate, script_name, "main", Options::package_root(),
Options::packages_file(), &flags, &error, &exit_code);
Options::packages_file(), &flags, NULL /* callback_data */, &error,
&exit_code);
if (isolate == NULL) {
delete[] isolate_name;

View file

@ -166,9 +166,9 @@ static Dart_Isolate CreateIsolateAndSetup(const char* script_uri,
ASSERT(kernel_service_buffer != NULL);
isolate_data =
new bin::IsolateData(script_uri, package_root, packages_config, NULL);
isolate_data->set_kernel_buffer(const_cast<uint8_t*>(kernel_service_buffer),
kernel_service_buffer_size,
false /* take_ownership */);
isolate_data->SetKernelBufferUnowned(
const_cast<uint8_t*>(kernel_service_buffer),
kernel_service_buffer_size);
isolate = Dart_CreateIsolateFromKernel(
script_uri, main, kernel_service_buffer, kernel_service_buffer_size,
flags, isolate_data, error);

View file

@ -553,7 +553,7 @@ typedef struct {
* for each part.
*/
#define DART_FLAGS_CURRENT_VERSION (0x0000000a)
#define DART_FLAGS_CURRENT_VERSION (0x0000000b)
typedef struct {
int32_t version;
@ -565,6 +565,7 @@ typedef struct {
bool use_bare_instructions;
bool load_vmservice_library;
bool unsafe_trust_strong_mode_types;
bool copy_parent_code;
} Dart_IsolateFlags;
/**

View file

@ -230,6 +230,10 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnFunction, 0, 10) {
isolate->spawn_count_monitor(), isolate->spawn_count(),
utf8_package_root, utf8_package_config, paused.value(), fatal_errors,
on_exit_port, on_error_port);
// Since this is a call to Isolate.spawn, copy the parent isolate's code.
state->isolate_flags()->copy_parent_code = true;
ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
isolate->IncrementSpawnCount();
@ -357,6 +361,9 @@ DEFINE_NATIVE_ENTRY(Isolate_spawnUri, 0, 12) {
flags->enable_asserts = checked.value();
}
// Since this is a call to Isolate.spawnUri, don't copy the parent's code.
state->isolate_flags()->copy_parent_code = false;
ThreadPool::Task* spawn_task = new SpawnIsolateTask(state);
isolate->IncrementSpawnCount();

View file

@ -261,7 +261,14 @@ void KernelFingerprintHelper::CalculateOptionalDartTypeFingerprint() {
}
void KernelFingerprintHelper::CalculateInterfaceTypeFingerprint(bool simple) {
BuildHash(ReadUInt()); // read klass_name.
NameIndex kernel_class = ReadCanonicalNameReference();
ASSERT(H.IsClass(kernel_class));
const String& class_name = H.DartClassName(kernel_class);
NameIndex kernel_library = H.CanonicalNameParent(kernel_class);
const String& library_name =
H.DartSymbolPlain(H.CanonicalNameString(kernel_library));
BuildHash(class_name.Hash());
BuildHash(library_name.Hash());
if (!simple) {
CalculateListOfDartTypesFingerprint(); // read list of types.
}

View file

@ -780,6 +780,7 @@ void Isolate::FlagsInitialize(Dart_IsolateFlags* api_flags) {
#undef INIT_FROM_FLAG
api_flags->entry_points = NULL;
api_flags->load_vmservice_library = false;
api_flags->copy_parent_code = false;
}
void Isolate::FlagsCopyTo(Dart_IsolateFlags* api_flags) const {
@ -790,6 +791,7 @@ void Isolate::FlagsCopyTo(Dart_IsolateFlags* api_flags) const {
#undef INIT_FROM_FIELD
api_flags->entry_points = NULL;
api_flags->load_vmservice_library = should_load_vmservice();
api_flags->copy_parent_code = false;
}
void Isolate::FlagsCopyFrom(const Dart_IsolateFlags& api_flags) {

View file

@ -134,7 +134,7 @@ typedef FixedCache<intptr_t, CatchEntryMovesRefPtr, 16> CatchEntryMovesCache;
// List of Isolate flags with corresponding members of Dart_IsolateFlags and
// corresponding global command line flags.
//
// V(when, name, Dart_IsolateFlags-member-name, command-line-flag-name)
// V(when, name, bit-name, Dart_IsolateFlags-name, command-line-flag-name)
//
#define ISOLATE_FLAG_LIST(V) \
V(NONPRODUCT, asserts, EnableAsserts, enable_asserts, FLAG_enable_asserts) \

View file

@ -0,0 +1,74 @@
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
// 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.
// Testing that Isolate.spawn copies the source code of the parent isolate,
// rather than rereading the parent's source URI.
// https://github.com/dart-lang/sdk/issues/6610
// Isolate structure:
// Root 1 -> Branch 1 -> Leaf 1
// /
// main
// \
// Root 2 -> Branch 2 -> Leaf 2
library spawn_tests;
import "dart:io";
import 'dart:isolate';
import 'package:expect/expect.dart';
void main() {
HttpServer.bind("127.0.0.1", 0).then((server) {
var count = 0;
server.listen((HttpRequest request) {
++count;
request.response.write("""
import 'dart:isolate';
void main(_, SendPort port) {
root(port);
}
void root(SendPort port) {
port.send("Root ${count}");
Isolate.spawn(branch, port);
}
void branch(SendPort port) {
port.send("Branch ${count}");
Isolate.spawn(leaf, port);
}
void leaf(SendPort port) {
port.send("Leaf ${count}");
}
""");
request.response.close();
});
ReceivePort port = new ReceivePort();
var messageSet = Set();
port.listen((message) {
messageSet.add(message);
if (messageSet.length >= 6) {
server.close();
port.close();
Expect.setEquals([
"Root 1",
"Root 2",
"Branch 1",
"Branch 2",
"Leaf 1",
"Leaf 2",
], messageSet);
}
});
Isolate.spawnUri(
Uri.parse("http://127.0.0.1:${server.port}"), [], port.sendPort);
Isolate.spawnUri(
Uri.parse("http://127.0.0.1:${server.port}"), [], port.sendPort);
});
}

View file

@ -211,6 +211,7 @@ isolate/isolate_complex_messages_test: Skip # Isolate.spawnUri
isolate/issue_21398_parent_isolate1_test: Skip # Isolate.spawnUri
isolate/issue_21398_parent_isolate_test: Skip # Isolate.spawnUri
isolate/issue_24243_parent_isolate_test: Skip # Isolate.spawnUri
isolate/issue_6610_test: Skip # Isolate.spawnUri
isolate/mandel_isolate_test: Skip # Isolate.spawnUri
isolate/message2_test: Skip # Isolate.spawnUri
isolate/message_test: Skip # Isolate.spawnUri

View file

@ -251,6 +251,9 @@ typed_data/float32x4_static_test: MissingCompileTimeError
typed_data/int32x4_static_test/01: MissingCompileTimeError
typed_data/int32x4_static_test/02: MissingCompileTimeError
[ $hot_reload && ($compiler == dartk || $compiler == dartkb) ]
isolate/issue_6610_test: RuntimeError, Crash # Sources are looked up on every reload request.
[ $hot_reload_rollback && ($compiler == dartk || $compiler == dartkb) ]
isolate/illegal_msg_function_test: Skip # Timeout
isolate/pause_test: Skip # Timeout
@ -260,6 +263,7 @@ isolate/pause_test: Skip # Timeout
# batch mode.
[ $strong && ($arch == simarm || $arch == simarm64 || $arch == simdbc64) && ($compiler == dartk || $compiler == dartkb) ]
isolate/mandel_isolate_test: Pass, Timeout
isolate/nested_spawn2_test: Pass, RuntimeError # RuntimeError caused by timeout
mirrors/library_uri_io_test: RuntimeError # Please triage.
[ $strong && ($compiler == dartk || $compiler == dartkb) ]