Skip to content

Commit

Permalink
Revert "(Reland)[Bindings] Create and use V8 context snapshots."
Browse files Browse the repository at this point in the history 
This reverts commit f6a5016.

Reason for revert: Speculative revert for failing webkit tests at https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20%28dbg%29/builds/4057.

Original change's description:
> (Reland)[Bindings] Create and use V8 context snapshots.
> 
> This CL does two things.
> 
> 1. In compile time, creates a snapshot file, which consists of V8 contexts.
> 2. Creates v8::Context from the snapshot in LocalWindowProxy::CreateContext().
> 
> We expect this speeds up context creation for 3 times faster on Android.
> Detailed information is described in the design doc [1].
> 
> 
> [1] Design doc: https://docs.google.com/document/d/1jpQQX0piaxcHJPWakp_Kr_03g5Gnma5h5-Kdlqu7jVQ/edit#heading=h.k6iklq6rvd30
> 
> 
> This CL is a re-land of https://chromium-review.googlesource.com/c/594608/
> Test expectations are changed due to http://crbug.com/705364
> 
> BUG=588893, 617892, 705364
> TBR=rkc, jochen, dchen, kinuko, eroman, thakis
> 
> Change-Id: I8e98ff881a2219380edc3c48de38bc661c139fb1
> Reviewed-on: https://chromium-review.googlesource.com/596167
> Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Hitoshi Yoshida <peria@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#495105}

TBR=rkc@chromium.org,dcheng@chromium.org,peria@chromium.org,kinuko@chromium.org,thakis@chromium.org,eroman@chromium.org,yukishiino@chromium.org,haraken@chromium.org,yangguo@chromium.org,jochen@chromium.org

Change-Id: I1050b0b84372ec683f3409ef77b9e9c45e39528f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 588893, 617892, 705364
Reviewed-on: https://chromium-review.googlesource.com/618746
Reviewed-by: Max Morin <maxmorin@chromium.org>
Commit-Queue: Max Morin <maxmorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495122}
  • Loading branch information
Max Morin authored and Commit Bot committed Aug 17, 2017
1 parent a08575b commit f0d13c9
Show file tree
Hide file tree
Showing 62 changed files with 161 additions and 1,693 deletions.
3 changes: 1 addition & 2 deletions build/config/features.gni
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ declare_args() {

# Option controlling the use of GConf (the classic GNOME configuration
# system).
use_gconf = is_linux && !is_chromeos && !is_chromecast &&
current_toolchain == default_toolchain
use_gconf = is_linux && !is_chromeos && !is_chromecast

use_gio = is_linux && !is_chromeos && !is_chromecast
}
Expand Down
3 changes: 0 additions & 3 deletions chrome/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ if (!is_android && !is_mac) {
group("chrome") {
public_deps = [
":chrome_initial",
"//tools/v8_context_snapshot:v8_context_snapshot",
]
data_deps = [
":chrome_initial",
Expand Down Expand Up @@ -863,7 +862,6 @@ if (is_win) {
bundle_data("chrome_framework_resources") {
sources = [
"$root_out_dir/app_mode_loader.app",
"$root_out_dir/v8_context_snapshot.bin",

# This image is used to badge the lock icon in the
# authentication dialogs, such as those used for installation
Expand All @@ -883,7 +881,6 @@ if (is_win) {
public_deps = [
":packed_resources",
"//chrome/app_shim:app_mode_loader",
"//tools/v8_context_snapshot:v8_context_snapshot",
]

if (is_chrome_branded) {
Expand Down
52 changes: 18 additions & 34 deletions content/app/content_main_runner.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,18 @@
#include "content/public/common/content_switches.h"
#include "content/public/common/main_function_params.h"
#include "content/public/common/sandbox_init.h"
#include "gin/v8_initializer.h"
#include "media/base/media.h"
#include "media/media_features.h"
#include "ppapi/features/features.h"
#include "services/service_manager/embedder/switches.h"
#include "ui/base/ui_base_paths.h"
#include "ui/base/ui_base_switches.h"

#if defined(V8_USE_EXTERNAL_STARTUP_DATA) && \
!defined(CHROME_MULTIPLE_DLL_BROWSER)
#include "gin/v8_initializer.h"
#endif

#if defined(OS_WIN)
#include <malloc.h>
#include <cstring>
Expand Down Expand Up @@ -174,24 +178,6 @@ void InitializeFieldTrialAndFeatureList(
base::FeatureList::SetInstance(std::move(feature_list));
}

void LoadV8ContextSnapshotFile() {
#if defined(OS_POSIX) && !defined(OS_MACOSX)
base::FileDescriptorStore& file_descriptor_store =
base::FileDescriptorStore::GetInstance();
base::MemoryMappedFile::Region region;
base::ScopedFD fd = file_descriptor_store.MaybeTakeFD(
kV8ContextSnapshotDataDescriptor, &region);
if (fd.is_valid()) {
gin::V8Initializer::LoadV8ContextSnapshotFromFD(fd.get(), region.offset,
region.size);
return;
}
#endif // OS
#if !defined(CHROME_MULTIPLE_DLL_BROWSER)
gin::V8Initializer::LoadV8ContextSnapshot();
#endif // !CHROME_MULTIPLE_DLL_BROWSER
}

void InitializeV8IfNeeded(
const base::CommandLine& command_line,
const std::string& process_type) {
Expand All @@ -208,26 +194,24 @@ void InitializeV8IfNeeded(
if (v8_snapshot_fd.is_valid()) {
gin::V8Initializer::LoadV8SnapshotFromFD(v8_snapshot_fd.get(),
region.offset, region.size);
} else {
gin::V8Initializer::LoadV8Snapshot();
}
base::ScopedFD v8_natives_fd =
file_descriptor_store.MaybeTakeFD(kV8NativesDataDescriptor, &region);
if (v8_natives_fd.is_valid()) {
gin::V8Initializer::LoadV8NativesFromFD(v8_natives_fd.get(), region.offset,
region.size);
} else {
gin::V8Initializer::LoadV8Natives();
}
} else {
gin::V8Initializer::LoadV8Snapshot();
}
base::ScopedFD v8_natives_fd =
file_descriptor_store.MaybeTakeFD(kV8NativesDataDescriptor, &region);
if (v8_natives_fd.is_valid()) {
gin::V8Initializer::LoadV8NativesFromFD(v8_natives_fd.get(),
region.offset, region.size);
} else {
gin::V8Initializer::LoadV8Natives();
}
#else
#if !defined(CHROME_MULTIPLE_DLL_BROWSER)
gin::V8Initializer::LoadV8Snapshot();
gin::V8Initializer::LoadV8Natives();
gin::V8Initializer::LoadV8Snapshot();
gin::V8Initializer::LoadV8Natives();
#endif // !CHROME_MULTIPLE_DLL_BROWSER
#endif // OS_POSIX && !OS_MACOSX
#endif // V8_USE_EXTERNAL_STARTUP_DATA

LoadV8ContextSnapshotFile();
}

} // namespace
Expand Down
8 changes: 1 addition & 7 deletions content/public/app/mojo/content_renderer_manifest.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,6 @@
"v8_snapshot_64_data" : [{
"path": "assets/snapshot_blob_64.bin",
"platform": "android"
}],
"v8_context_snapshot_data" : [
{
"path": "v8_context_snapshot.bin",
"platform": "linux"
}
]
}]
}
}
4 changes: 1 addition & 3 deletions content/public/common/content_descriptor_keys.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,4 @@ const char kV8Snapshot32DataDescriptor[] = "v8_snapshot_32_data";

const char kV8Snapshot64DataDescriptor[] = "v8_snapshot_64_data";

const char kV8ContextSnapshotDataDescriptor[] = "v8_context_snapshot_data";

} // namespace content
} // namespace content
1 change: 0 additions & 1 deletion content/public/common/content_descriptor_keys.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ extern const char kV8NativesDataDescriptor[];
extern const char kV8SnapshotDataDescriptor[];
extern const char kV8Snapshot32DataDescriptor[];
extern const char kV8Snapshot64DataDescriptor[];
extern const char kV8ContextSnapshotDataDescriptor[];

} // namespace content

Expand Down
10 changes: 0 additions & 10 deletions content/shell/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,14 +542,6 @@ if (is_android) {
":pak",
]

public_deps = [
"//tools/v8_context_snapshot:v8_context_snapshot",
]

data = [
"$root_out_dir/v8_context_snapshot.bin",
]

if (is_win) {
deps += [ "//sandbox" ]

Expand Down Expand Up @@ -614,13 +606,11 @@ if (is_mac) {
bundle_data("content_shell_framework_resources") {
sources = [
"$root_out_dir/content_shell.pak",
"$root_out_dir/v8_context_snapshot.bin",
"resources/missingImage.png",
]

public_deps = [
":pak",
"//tools/v8_context_snapshot:v8_context_snapshot",
]

if (icu_use_data_file) {
Expand Down
2 changes: 0 additions & 2 deletions extensions/shell/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,12 +357,10 @@ if (is_mac) {
sources = [
"$root_gen_dir/extensions/shell/app_shell_resources.pak",
"$root_out_dir/extensions_shell_and_test.pak",
"$root_out_dir/v8_context_snapshot.bin",
]
public_deps = [
":resources_grit",
"//extensions:shell_and_test_pak",
"//tools/v8_context_snapshot:v8_context_snapshot",
]
if (icu_use_data_file) {
sources += [ "$root_out_dir/icudtl.dat" ]
Expand Down
81 changes: 35 additions & 46 deletions gin/isolate_holder.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "base/message_loop/message_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/sys_info.h"
#include "build/build_config.h"
#include "gin/debug_impl.h"
#include "gin/function_template.h"
#include "gin/per_isolate_data.h"
Expand All @@ -36,51 +35,61 @@ IsolateHolder::IsolateHolder(
IsolateHolder::IsolateHolder(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
AccessMode access_mode)
: IsolateHolder(std::move(task_runner),
access_mode,
kAllowAtomicsWait,
nullptr,
nullptr) {}
: IsolateHolder(std::move(task_runner), access_mode, kAllowAtomicsWait) {}

IsolateHolder::IsolateHolder(
scoped_refptr<base::SingleThreadTaskRunner> task_runner,
AccessMode access_mode,
AllowAtomicsWaitMode atomics_wait_mode,
intptr_t* reference,
v8::StartupData* startup_data)
AllowAtomicsWaitMode atomics_wait_mode)
: access_mode_(access_mode) {
v8::ArrayBuffer::Allocator* allocator = g_array_buffer_allocator;
CHECK(allocator) << "You need to invoke gin::IsolateHolder::Initialize first";

v8::Isolate::CreateParams params;
params.entry_hook = DebugImpl::GetFunctionEntryHook();
params.code_event_handler = DebugImpl::GetJitCodeEventHandler();
params.constraints.ConfigureDefaults(base::SysInfo::AmountOfPhysicalMemory(),
base::SysInfo::AmountOfVirtualMemory());
params.array_buffer_allocator = allocator;
params.allow_atomics_wait = atomics_wait_mode == kAllowAtomicsWait;
params.external_references = reference;

if (startup_data) {
CHECK(reference);
V8Initializer::GetV8ContextSnapshotData(&startup_data->data,
&startup_data->raw_size);
if (startup_data->data) {
params.snapshot_blob = startup_data;
}
}
isolate_ = v8::Isolate::New(params);

SetUp(std::move(task_runner));
isolate_data_.reset(
new PerIsolateData(isolate_, allocator, access_mode, task_runner));
isolate_memory_dump_provider_.reset(new V8IsolateMemoryDumpProvider(this));
#if defined(OS_WIN)
{
void* code_range;
size_t size;
isolate_->GetCodeRange(&code_range, &size);
Debug::CodeRangeCreatedCallback callback =
DebugImpl::GetCodeRangeCreatedCallback();
if (code_range && size && callback)
callback(code_range, size);
}
#endif
}

IsolateHolder::IsolateHolder(intptr_t* reference_table,
v8::StartupData* existing_blob)
: snapshot_creator_(
new v8::SnapshotCreator(reference_table, existing_blob)),
isolate_(snapshot_creator_->GetIsolate()),
access_mode_(AccessMode::kSingleThread) {
SetUp(nullptr);
access_mode_(kSingleThread) {
v8::ArrayBuffer::Allocator* allocator = g_array_buffer_allocator;
CHECK(allocator) << "You need to invoke gin::IsolateHolder::Initialize first";
isolate_ = snapshot_creator_->GetIsolate();
isolate_data_.reset(
new PerIsolateData(isolate_, allocator, access_mode_, nullptr));
isolate_memory_dump_provider_.reset(new V8IsolateMemoryDumpProvider(this));
#if defined(OS_WIN)
{
void* code_range;
size_t size;
isolate_->GetCodeRange(&code_range, &size);
Debug::CodeRangeCreatedCallback callback =
DebugImpl::GetCodeRangeCreatedCallback();
if (code_range && size && callback)
callback(code_range, size);
}
#endif
}

IsolateHolder::~IsolateHolder() {
Expand All @@ -100,7 +109,7 @@ IsolateHolder::~IsolateHolder() {
isolate_memory_dump_provider_.reset();
isolate_data_.reset();
isolate_->Dispose();
isolate_ = nullptr;
isolate_ = NULL;
}

// static
Expand Down Expand Up @@ -130,24 +139,4 @@ void IsolateHolder::EnableIdleTasks(
isolate_data_->EnableIdleTasks(std::move(idle_task_runner));
}

void IsolateHolder::SetUp(
scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
v8::ArrayBuffer::Allocator* allocator = g_array_buffer_allocator;
CHECK(allocator) << "You need to invoke gin::IsolateHolder::Initialize first";
isolate_data_.reset(
new PerIsolateData(isolate_, allocator, access_mode_, task_runner));
isolate_memory_dump_provider_.reset(new V8IsolateMemoryDumpProvider(this));
#if defined(OS_WIN)
{
void* code_range;
size_t size;
isolate_->GetCodeRange(&code_range, &size);
Debug::CodeRangeCreatedCallback callback =
DebugImpl::GetCodeRangeCreatedCallback();
if (code_range && size && callback)
callback(code_range, size);
}
#endif
}

} // namespace gin
8 changes: 2 additions & 6 deletions gin/public/isolate_holder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ class GIN_EXPORT IsolateHolder {
AccessMode access_mode);
IsolateHolder(scoped_refptr<base::SingleThreadTaskRunner> task_runner,
AccessMode access_mode,
AllowAtomicsWaitMode atomics_wait_mode,
intptr_t* reference_table,
v8::StartupData* startup_data);
AllowAtomicsWaitMode atomics_wait_mode);

// This constructor is to create V8 snapshot for Blink.
// Note this constructor calls isolate->Enter() internally.
Expand Down Expand Up @@ -112,13 +110,11 @@ class GIN_EXPORT IsolateHolder {
}

private:
void SetUp(scoped_refptr<base::SingleThreadTaskRunner> task_runner);

std::unique_ptr<v8::SnapshotCreator> snapshot_creator_;
v8::Isolate* isolate_;
std::unique_ptr<PerIsolateData> isolate_data_;
std::unique_ptr<RunMicrotasksObserver> task_observer_;
std::unique_ptr<V8IsolateMemoryDumpProvider> isolate_memory_dump_provider_;
std::unique_ptr<v8::SnapshotCreator> snapshot_creator_;
AccessMode access_mode_;

DISALLOW_COPY_AND_ASSIGN(IsolateHolder);
Expand Down
Loading

0 comments on commit f0d13c9

Please sign in to comment.