Skip to content

Commit

Permalink
[web snapshot] Pass exports to the serializer as a Local<PrimitiveArray>
Browse files Browse the repository at this point in the history 
This CL updates WebSnapshotSerializer::TakeSnapshot() to accept exports
as a Local<PrimitiveArray>.

Bug: v8:11525, v8:11706
Change-Id: Ie3a752ac7dbcc51fc4fb258eb44ce42d0cfc6a0f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2930173
Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Vicky Kontoura <vkont@google.com>
Cr-Commit-Position: refs/heads/master@{#74936}
  • Loading branch information
Vicky Kontoura authored and V8 LUCI CQ committed Jun 3, 2021
1 parent a3f3f01 commit e30cbcc
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 47 deletions.
50 changes: 34 additions & 16 deletions src/d8/d8.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,10 @@ bool Shell::ExecuteString(Isolate* isolate, Local<String> source,
data->realm_current_ = data->realm_switch_;

if (options.web_snapshot_config) {
std::vector<std::string> exports;
if (!ReadLines(options.web_snapshot_config, exports)) {
MaybeLocal<PrimitiveArray> maybe_exports =
ReadLines(isolate, options.web_snapshot_config);
Local<PrimitiveArray> exports;
if (!maybe_exports.ToLocal(&exports)) {
isolate->ThrowError("Web snapshots: unable to read config");
CHECK(try_catch.HasCaught());
ReportException(isolate, &try_catch);
Expand Down Expand Up @@ -1824,19 +1826,20 @@ void Shell::RealmTakeWebSnapshot(
PerIsolateData* data = PerIsolateData::Get(isolate);
int index = data->RealmIndexOrThrow(args, 0);
if (index == -1) return;
// Create a std::vector<std::string> from the list of exports.
// TODO(v8:11525): Add an API in the serializer that accepts a Local<String>
// Array.
// Create a Local<PrimitiveArray> from the exports array.
Local<Context> current_context = isolate->GetCurrentContext();
Local<Array> exports_array = args[1].As<Array>();
std::vector<std::string> exports;
for (int i = 0, length = exports_array->Length(); i < length; ++i) {
Local<String> local_str = exports_array->Get(current_context, i)
.ToLocalChecked()
->ToString(current_context)
.ToLocalChecked();
std::string str = ToSTLString(isolate, local_str);
exports.push_back(str);
int length = exports_array->Length();
Local<PrimitiveArray> exports = PrimitiveArray::New(isolate, length);
for (int i = 0; i < length; ++i) {
Local<Value> value;
Local<String> str;
if (!exports_array->Get(current_context, i).ToLocal(&value) ||
!value->ToString(current_context).ToLocal(&str) || str.IsEmpty()) {
isolate->ThrowError("Invalid argument");
return;
}
exports->Set(isolate, i, str);
}
// Enter the realm.
Local<Context> realm = Local<Context>::New(isolate, data->realms_[index]);
Expand Down Expand Up @@ -3416,18 +3419,33 @@ char* Shell::ReadChars(const char* name, int* size_out) {
return chars;
}

bool Shell::ReadLines(const char* name, std::vector<std::string>& lines) {
MaybeLocal<PrimitiveArray> Shell::ReadLines(Isolate* isolate,
const char* name) {
int length;
const char* data = reinterpret_cast<const char*>(ReadChars(name, &length));
if (data == nullptr) {
return false;
return MaybeLocal<PrimitiveArray>();
}
std::stringstream stream(data);
std::string line;
std::vector<std::string> lines;
while (std::getline(stream, line, '\n')) {
lines.emplace_back(line);
}
return true;
// Create a Local<PrimitiveArray> off the read lines.
int size = static_cast<int>(lines.size());
Local<PrimitiveArray> exports = PrimitiveArray::New(isolate, size);
for (int i = 0; i < size; ++i) {
MaybeLocal<String> maybe_str = v8::String::NewFromUtf8(
isolate, lines[i].c_str(), NewStringType::kNormal,
static_cast<int>(lines[i].length()));
Local<String> str;
if (!maybe_str.ToLocal(&str)) {
return MaybeLocal<PrimitiveArray>();
}
exports->Set(isolate, i, str);
}
return exports;
}

void Shell::ReadBuffer(const v8::FunctionCallbackInfo<v8::Value>& args) {
Expand Down
3 changes: 2 additions & 1 deletion src/d8/d8.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,8 @@ class Shell : public i::AllStatic {
static void Version(const v8::FunctionCallbackInfo<v8::Value>& args);
static void ReadFile(const v8::FunctionCallbackInfo<v8::Value>& args);
static char* ReadChars(const char* name, int* size_out);
static bool ReadLines(const char* name, std::vector<std::string>& lines);
static MaybeLocal<PrimitiveArray> ReadLines(Isolate* isolate,
const char* name);
static void ReadBuffer(const v8::FunctionCallbackInfo<v8::Value>& args);
static Local<String> ReadFromStdin(Isolate* isolate);
static void ReadLine(const v8::FunctionCallbackInfo<v8::Value>& args) {
Expand Down
31 changes: 11 additions & 20 deletions src/web-snapshot/web-snapshot.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,23 +57,21 @@ WebSnapshotSerializer::WebSnapshotSerializer(v8::Isolate* isolate)

WebSnapshotSerializer::~WebSnapshotSerializer() {}

bool WebSnapshotSerializer::TakeSnapshot(
v8::Local<v8::Context> context, const std::vector<std::string>& exports,
WebSnapshotData& data_out) {
bool WebSnapshotSerializer::TakeSnapshot(v8::Local<v8::Context> context,
v8::Local<v8::PrimitiveArray> exports,
WebSnapshotData& data_out) {
if (string_ids_.size() > 0) {
Throw("Web snapshot: Can't reuse WebSnapshotSerializer");
return false;
}
v8::Isolate* v8_isolate = reinterpret_cast<v8::Isolate*>(isolate_);
for (const std::string& export_name : exports) {
if (export_name.length() == 0) {
for (int i = 0, length = exports->Length(); i < length; ++i) {
v8::Local<v8::String> str =
exports->Get(v8_isolate, i)->ToString(context).ToLocalChecked();
if (str.IsEmpty()) {
continue;
}
v8::ScriptCompiler::Source source(
v8::String::NewFromUtf8(v8_isolate, export_name.c_str(),
NewStringType::kNormal,
static_cast<int>(export_name.length()))
.ToLocalChecked());
v8::ScriptCompiler::Source source(str);
auto script = ScriptCompiler::Compile(context, &source).ToLocalChecked();
v8::MaybeLocal<v8::Value> script_result = script->Run(context);
v8::Local<v8::Object> v8_object;
Expand All @@ -85,7 +83,7 @@ bool WebSnapshotSerializer::TakeSnapshot(
}

auto object = Handle<JSObject>::cast(Utils::OpenHandle(*v8_object));
SerializeExport(object, export_name);
SerializeExport(object, Handle<String>::cast(Utils::OpenHandle(*str)));
}
WriteSnapshot(data_out.buffer, data_out.buffer_size);
return !has_error();
Expand Down Expand Up @@ -369,18 +367,11 @@ void WebSnapshotSerializer::SerializePendingObject(Handle<JSObject> object) {
// - String id (export name)
// - Object id (exported object)
void WebSnapshotSerializer::SerializeExport(Handle<JSObject> object,
const std::string& export_name) {
Handle<String> export_name) {
// TODO(v8:11525): Support exporting functions.
++export_count_;
// TODO(v8:11525): How to avoid creating the String but still de-dupe?
Handle<String> export_name_string =
isolate_->factory()
->NewStringFromOneByte(Vector<const uint8_t>(
reinterpret_cast<const uint8_t*>(export_name.c_str()),
static_cast<int>(export_name.length())))
.ToHandleChecked();
uint32_t string_id = 0;
SerializeString(export_name_string, string_id);
SerializeString(export_name, string_id);
uint32_t object_id = 0;
SerializeObject(object, object_id);
export_serializer_.WriteUint32(string_id);
Expand Down
4 changes: 2 additions & 2 deletions src/web-snapshot/web-snapshot.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class V8_EXPORT WebSnapshotSerializer
~WebSnapshotSerializer();

bool TakeSnapshot(v8::Local<v8::Context> context,
const std::vector<std::string>& exports,
v8::Local<v8::PrimitiveArray> exports,
WebSnapshotData& data_out);

// For inspecting the state after taking a snapshot.
Expand Down Expand Up @@ -121,7 +121,7 @@ class V8_EXPORT WebSnapshotSerializer
void SerializeContext(Handle<Context> context, uint32_t& id);
void SerializeObject(Handle<JSObject> object, uint32_t& id);
void SerializePendingObject(Handle<JSObject> object);
void SerializeExport(Handle<JSObject> object, const std::string& export_name);
void SerializeExport(Handle<JSObject> object, Handle<String> export_name);
void WriteValue(Handle<Object> object, ValueSerializer& serializer);

ValueSerializer string_serializer_;
Expand Down
24 changes: 16 additions & 8 deletions test/cctest/test-web-snapshots.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ void TestWebSnapshotExtensive(
v8::Context::Scope context_scope(new_context);

CompileRun(snapshot_source);
std::vector<std::string> exports;
exports.push_back("foo");
v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
v8::Local<v8::String> str =
v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
exports->Set(isolate, 0, str);
WebSnapshotSerializer serializer(isolate);
CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
CHECK(!serializer.has_error());
Expand Down Expand Up @@ -327,8 +329,10 @@ TEST(SFIDeduplication) {
"foo.inner = foo.outer('hi');";

CompileRun(snapshot_source);
std::vector<std::string> exports;
exports.push_back("foo");
v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
v8::Local<v8::String> str =
v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
exports->Set(isolate, 0, str);
WebSnapshotSerializer serializer(isolate);
CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
CHECK(!serializer.has_error());
Expand Down Expand Up @@ -385,8 +389,10 @@ TEST(SFIDeduplicationAfterBytecodeFlushing) {

CompileRun(snapshot_source);

std::vector<std::string> exports;
exports.push_back("foo");
v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
v8::Local<v8::String> str =
v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
exports->Set(isolate, 0, str);
WebSnapshotSerializer serializer(isolate);
CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
CHECK(!serializer.has_error());
Expand Down Expand Up @@ -463,8 +469,10 @@ TEST(SFIDeduplicationOfFunctionsNotInSnapshot) {
"}\n";

CompileRun(snapshot_source);
std::vector<std::string> exports;
exports.push_back("foo");
v8::Local<v8::PrimitiveArray> exports = v8::PrimitiveArray::New(isolate, 1);
v8::Local<v8::String> str =
v8::String::NewFromUtf8(isolate, "foo").ToLocalChecked();
exports->Set(isolate, 0, str);
WebSnapshotSerializer serializer(isolate);
CHECK(serializer.TakeSnapshot(new_context, exports, snapshot_data));
CHECK(!serializer.has_error());
Expand Down

0 comments on commit e30cbcc

Please sign in to comment.