Skip to content

Commit

Permalink
Reland "[vm/concurrency] Introduce concept of Isolate Groups"
Browse files Browse the repository at this point in the history 
An Isolate Group (IG) is a collection of isolates which were spawned from the
same source. This allows the VM to:

  * have a guarantee that all isolates within one IG can safely exchange
    structured objects (currently we rely on embedder for this
    guarantee)

  * hot-reload all isolates together (currently we only reload one
    isolate, leaving same-source isolates in inconsistent state)

  * make a shared heap for all isolates from the same IG, which paves
    the way for faster communication and sharing of immutable objects.

All isolates within one IG will share the same IsolateGroupSource.

**Embedder changes**

This change makes breaking embedder API changes to support this new
concept of Isolate Groups: The existing isolate lifecycle callbacks
given to Dart_Initialize will become Isolate Group lifecycle callbacks.
A new callback `initialize_isolate` callback will be added which can
initialize a new isolate within an existing IG.

Existing embedders can be updated by performing the following renames

  Dart_CreateIsolate -> Dart_CreateIsolateGroup
  Dart_IsolateCreateCallback -> Dart_IsolateGroupCreateCallback
  Dart_IsolateCleanupCallback -> Dart_IsolateGroupShutdownCallback
  Dart_CreateIsolateFromKernel -> Dart_CreateIsolateGroupFromKernel
  Dart_CurrentIsolateData -> Dart_CurrentIsolateGroupData
  Dart_IsolateData -> Dart_IsolateGroupData
  Dart_GetNativeIsolateData -> Dart_GetNativeIsolateGroupData
  Dart_InitializeParams.create -> Dart_InitializeParams.create_group
  Dart_InitializeParams.cleanup -> Dart_InitializeParams.shutdown_group
  Dart_InitializeParams.shutdown -> Dart_InitializeParams.shutdown_isolate

By default `Isolate.spawn` will cause the creation of a new IG.

Though an embedder can opt-into supporting multiple isolates within one IG by
providing a callback to the newly added `Dart_InitializeParams.initialize_isolate`.
The responsibility of this new callback is to initialize an existing
isolate (which was setup by re-using source code from the spawning
isolate - i.e. the one which used `Isolate.spawn`) by setting native
resolvers, initializing global state, etc.

Issue #36648
Issue #36097

Original review: https://dart-review.googlesource.com/c/sdk/+/105241

Difference to original review:

  * Give each isolate it's own [Loader] (for now)
  * Sort classes during initialization for spawned isolates if app-jit is used (to match main isolate)
  * Fix IsolateData memory leak if isolate startup fails

Change-Id: I98277d3d10fe275aa9b8a16b6bdd446bbea0b100
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/107506
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>
  • Loading branch information
@mkustermann
mkustermann authored and [email protected] committed Jun 29, 2019
1 parent 000cf05 commit 67ab3be
Show file tree
Hide file tree
Showing 27 changed files with 1,199 additions and 458 deletions.
8 changes: 4 additions & 4 deletions runtime/bin/dart_embedder_api_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ Dart_Isolate CreateKernelServiceIsolate(const IsolateCreationData& data,
const uint8_t* buffer,
intptr_t buffer_size,
char** error) {
Dart_Isolate kernel_isolate = Dart_CreateIsolateFromKernel(
Dart_Isolate kernel_isolate = Dart_CreateIsolateGroupFromKernel(
data.script_uri, data.main, buffer, buffer_size, data.flags,
data.callback_data, error);
data.isolate_group_data, data.isolate_data, error);
if (kernel_isolate == nullptr) {
return nullptr;
}
Expand Down Expand Up @@ -78,9 +78,9 @@ Dart_Isolate CreateVmServiceIsolate(const IsolateCreationData& data,
}
data.flags->load_vmservice_library = true;

Dart_Isolate service_isolate = Dart_CreateIsolateFromKernel(
Dart_Isolate service_isolate = Dart_CreateIsolateGroupFromKernel(
data.script_uri, data.main, kernel_buffer, kernel_buffer_size, data.flags,
data.callback_data, error);
data.isolate_group_data, data.isolate_data, error);
if (service_isolate == nullptr) {
return nullptr;
}
Expand Down
24 changes: 13 additions & 11 deletions runtime/bin/gen_snapshot.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -745,23 +745,25 @@ static int CreateIsolateAndSnapshot(const CommandLineOptions& inputs) {
isolate_flags.entry_points = no_entry_points;
}

IsolateData* isolate_data = new IsolateData(NULL, NULL, NULL, NULL);
auto isolate_group_data =
new IsolateGroupData(nullptr, nullptr, nullptr, nullptr, false);
Dart_Isolate isolate;
char* error = NULL;
if (isolate_snapshot_data == NULL) {
// We need to capture the vmservice library in the core snapshot, so load it
// in the main isolate as well.
isolate_flags.load_vmservice_library = true;
isolate = Dart_CreateIsolateFromKernel(NULL, NULL, kernel_buffer,
kernel_buffer_size, &isolate_flags,
isolate_data, &error);
isolate = Dart_CreateIsolateGroupFromKernel(
NULL, NULL, kernel_buffer, kernel_buffer_size, &isolate_flags,
isolate_group_data, /*isolate_data=*/nullptr, &error);
} else {
isolate = Dart_CreateIsolate(NULL, NULL, isolate_snapshot_data,
isolate_snapshot_instructions, NULL, NULL,
&isolate_flags, isolate_data, &error);
isolate = Dart_CreateIsolateGroup(NULL, NULL, isolate_snapshot_data,
isolate_snapshot_instructions, NULL, NULL,
&isolate_flags, isolate_group_data,
/*isolate_data=*/nullptr, &error);
}
if (isolate == NULL) {
delete isolate_data;
delete isolate_group_data;
Syslog::PrintErr("%s\n", error);
free(error);
return kErrorExitCode;
Expand All @@ -783,9 +785,9 @@ static int CreateIsolateAndSnapshot(const CommandLineOptions& inputs) {
// If the input dill file does not have a root library, then
// Dart_LoadScript will error.
//
// TODO(kernel): Dart_CreateIsolateFromKernel should respect the root library
// in the kernel file, though this requires auditing the other loading paths
// in the embedders that had to work around this.
// TODO(kernel): Dart_CreateIsolateGroupFromKernel should respect the root
// library in the kernel file, though this requires auditing the other
// loading paths in the embedders that had to work around this.
result = Dart_SetRootLibrary(
Dart_LoadLibraryFromKernel(kernel_buffer, kernel_buffer_size));
CHECK_RESULT(result);
Expand Down
43 changes: 26 additions & 17 deletions runtime/bin/isolate_data.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,45 +9,54 @@
namespace dart {
namespace bin {

IsolateData::IsolateData(const char* url,
const char* package_root,
const char* packages_file,
AppSnapshot* app_snapshot)
IsolateGroupData::IsolateGroupData(const char* url,
const char* package_root,
const char* packages_file,
AppSnapshot* app_snapshot,
bool isolate_run_app_snapshot)
: script_url((url != NULL) ? strdup(url) : NULL),
package_root(NULL),
packages_file(NULL),
loader_(NULL),
app_snapshot_(app_snapshot),
dependencies_(NULL),
resolved_packages_config_(NULL),
kernel_buffer_(NULL),
kernel_buffer_size_(0) {
kernel_buffer_size_(0),
isolate_run_app_snapshot_(isolate_run_app_snapshot) {
if (package_root != NULL) {
ASSERT(packages_file == NULL);
this->package_root = strdup(package_root);
package_root = strdup(package_root);
} else if (packages_file != NULL) {
this->packages_file = strdup(packages_file);
packages_file_ = strdup(packages_file);
}
}

void IsolateData::OnIsolateShutdown() {
}

IsolateData::~IsolateData() {
IsolateGroupData::~IsolateGroupData() {
free(script_url);
script_url = NULL;
free(package_root);
package_root = NULL;
free(packages_file);
packages_file = NULL;
free(packages_file_);
packages_file_ = NULL;
free(resolved_packages_config_);
resolved_packages_config_ = NULL;
kernel_buffer_ = NULL;
kernel_buffer_size_ = 0;
delete app_snapshot_;
app_snapshot_ = NULL;
delete dependencies_;
}

IsolateData::IsolateData(IsolateGroupData* isolate_group_data)
: isolate_group_data_(isolate_group_data),
loader_(nullptr),
packages_file_(nullptr) {
if (isolate_group_data->packages_file_ != nullptr) {
packages_file_ = strdup(isolate_group_data->packages_file_);
}
}

IsolateData::~IsolateData() {
free(packages_file_);
packages_file_ = nullptr;
}

} // namespace bin
} // namespace dart
101 changes: 66 additions & 35 deletions runtime/bin/isolate_data.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,62 +28,54 @@ class AppSnapshot;
class EventHandler;
class Loader;

// Data associated with every isolate in the standalone VM
// Data associated with every isolate group in the standalone VM
// embedding. This is used to free external resources for each isolate
// when the isolate shuts down.
class IsolateData {
// group when the isolate group shuts down.
class IsolateGroupData {
public:
IsolateData(const char* url,
const char* package_root,
const char* packages_file,
AppSnapshot* app_snapshot);
~IsolateData();
IsolateGroupData(const char* url,
const char* package_root,
const char* packages_file,
AppSnapshot* app_snapshot,
bool isolate_run_app_snapshot);
~IsolateGroupData();

char* script_url;
char* package_root;
char* packages_file;

const std::shared_ptr<uint8_t>& kernel_buffer() const {
return kernel_buffer_;
}

intptr_t kernel_buffer_size() const { return kernel_buffer_size_; }

// Associate the given kernel buffer with this IsolateData without giving it
// ownership of the buffer.
// Associate the given kernel buffer with this IsolateGroupData 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
// Associate the given kernel buffer with this IsolateGroupData and give it
// ownership of the buffer. This IsolateGroupData 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
// Associate the given kernel buffer with this IsolateGroupData and give it
// ownership of the buffer. The buffer is already owned by another
// IsolateData.
// IsolateGroupData.
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;
}

void UpdatePackagesFile(const char* packages_file_) {
if (packages_file != NULL) {
free(packages_file);
packages_file = NULL;
}
packages_file = strdup(packages_file_);
}

const char* resolved_packages_config() const {
return resolved_packages_config_;
}
Expand All @@ -96,6 +88,54 @@ class IsolateData {
resolved_packages_config_ = strdup(packages_config);
}

MallocGrowableArray<char*>* dependencies() const { return dependencies_; }
void set_dependencies(MallocGrowableArray<char*>* deps) {
dependencies_ = deps;
}

bool RunFromAppSnapshot() const {
// If the main isolate is using an app snapshot the [app_snapshot_] pointer
// will be still nullptr (see main.cc:CreateIsolateGroupAndSetupHelper)
//
// Because of thus we have an additional boolean signaling whether the
// isolate was started from an app snapshot.
return app_snapshot_ != nullptr || isolate_run_app_snapshot_;
}

private:
friend class IsolateData; // For packages_file_

std::unique_ptr<AppSnapshot> app_snapshot_;
MallocGrowableArray<char*>* dependencies_;
char* resolved_packages_config_;
std::shared_ptr<uint8_t> kernel_buffer_;
intptr_t kernel_buffer_size_;
char* packages_file_ = nullptr;
bool isolate_run_app_snapshot_;

static void FreeUnownedKernelBuffer(uint8_t*) {}

DISALLOW_COPY_AND_ASSIGN(IsolateGroupData);
};

// Data associated with every isolate in the standalone VM
// embedding. This is used to free external resources for each isolate
// when the isolate shuts down.
class IsolateData {
public:
explicit IsolateData(IsolateGroupData* isolate_group_data);
~IsolateData();

IsolateGroupData* isolate_group_data() const { return isolate_group_data_; }

void UpdatePackagesFile(const char* packages_file) {
if (packages_file != nullptr) {
free(packages_file_);
packages_file_ = nullptr;
}
packages_file_ = strdup(packages_file);
}

// While loading a loader is associated with the isolate.
bool HasLoader() const { return loader_ != NULL; }
Loader* loader() const {
Expand All @@ -106,22 +146,13 @@ class IsolateData {
ASSERT((loader_ == NULL) || (loader == NULL));
loader_ = loader;
}
MallocGrowableArray<char*>* dependencies() const { return dependencies_; }
void set_dependencies(MallocGrowableArray<char*>* deps) {
dependencies_ = deps;
}

void OnIsolateShutdown();
const char* packages_file() const { return packages_file_; }

private:
IsolateGroupData* isolate_group_data_;
Loader* loader_;
AppSnapshot* app_snapshot_;
MallocGrowableArray<char*>* dependencies_;
char* resolved_packages_config_;
std::shared_ptr<uint8_t> kernel_buffer_;
intptr_t kernel_buffer_size_;

static void FreeUnownedKernelBuffer(uint8_t*) {}
char* packages_file_;

DISALLOW_COPY_AND_ASSIGN(IsolateData);
};
Expand Down
34 changes: 19 additions & 15 deletions runtime/bin/loader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,18 +263,18 @@ static bool PathContainsSeparator(const char* path) {

void Loader::AddDependencyLocked(Loader* loader, const char* resolved_uri) {
MallocGrowableArray<char*>* dependencies =
loader->isolate_data_->dependencies();
loader->isolate_group_data()->dependencies();
if (dependencies == NULL) {
return;
}
dependencies->Add(strdup(resolved_uri));
}

void Loader::ResolveDependenciesAsFilePaths() {
IsolateData* isolate_data =
reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
ASSERT(isolate_data != NULL);
MallocGrowableArray<char*>* dependencies = isolate_data->dependencies();
IsolateGroupData* isolate_group_data =
reinterpret_cast<IsolateGroupData*>(Dart_CurrentIsolateGroupData());
ASSERT(isolate_group_data != NULL);
MallocGrowableArray<char*>* dependencies = isolate_group_data->dependencies();
if (dependencies == NULL) {
return;
}
Expand Down Expand Up @@ -454,15 +454,15 @@ bool Loader::ProcessQueueLocked(ProcessResult process_result) {
return !hit_error;
}

void Loader::InitForSnapshot(const char* snapshot_uri) {
IsolateData* isolate_data =
reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
void Loader::InitForSnapshot(const char* snapshot_uri,
IsolateData* isolate_data) {
ASSERT(isolate_data != NULL);
ASSERT(!isolate_data->HasLoader());
// Setup a loader. The constructor does a bunch of leg work.
Loader* loader = new Loader(isolate_data);
// Send the init message.
loader->Init(isolate_data->package_root, isolate_data->packages_file,
loader->Init(isolate_data->isolate_group_data()->package_root,
isolate_data->packages_file(),
DartUtils::original_working_directory, snapshot_uri);
// Destroy the loader. The destructor does a bunch of leg work.
delete loader;
Expand Down Expand Up @@ -516,15 +516,15 @@ Dart_Handle Loader::SendAndProcessReply(intptr_t tag,
Dart_Handle url,
uint8_t** payload,
intptr_t* payload_length) {
IsolateData* isolate_data =
reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
auto isolate_data = reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
ASSERT(isolate_data != NULL);
ASSERT(!isolate_data->HasLoader());
Loader* loader = NULL;

// Setup the loader. The constructor does a bunch of leg work.
loader = new Loader(isolate_data);
loader->Init(isolate_data->package_root, isolate_data->packages_file,
loader->Init(isolate_data->isolate_group_data()->package_root,
isolate_data->packages_file(),
DartUtils::original_working_directory, NULL);
ASSERT(loader != NULL);
ASSERT(isolate_data->HasLoader());
Expand Down Expand Up @@ -565,6 +565,10 @@ Dart_Handle Loader::ResolveAsFilePath(Dart_Handle url,
payload_length);
}

IsolateGroupData* Loader::isolate_group_data() {
return isolate_data_->isolate_group_data();
}

#if defined(DART_PRECOMPILED_RUNTIME)
Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
Dart_Handle library,
Expand Down Expand Up @@ -679,8 +683,7 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,
}
}

IsolateData* isolate_data =
reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
auto isolate_data = reinterpret_cast<IsolateData*>(Dart_CurrentIsolateData());
ASSERT(isolate_data != NULL);
if ((tag == Dart_kScriptTag) && Dart_IsString(library)) {
// Update packages file for isolate.
Expand All @@ -707,7 +710,8 @@ Dart_Handle Loader::LibraryTagHandler(Dart_LibraryTag tag,

// Setup the loader. The constructor does a bunch of leg work.
loader = new Loader(isolate_data);
loader->Init(isolate_data->package_root, isolate_data->packages_file,
loader->Init(isolate_data->isolate_group_data()->package_root,
isolate_data->packages_file(),
DartUtils::original_working_directory,
(tag == Dart_kScriptTag) ? url_string : NULL);
} else {
Expand Down
Loading

0 comments on commit 67ab3be

Please sign in to comment.