Skip to content

Commit

Permalink
src: allow blobs in addition to FILE*s in embedder snapshot API
Browse files Browse the repository at this point in the history
This is a shared follow-up to 06bb6b4 and a466fea
now that both have been merged.

PR-URL: #46491
Refs: #45888
Refs: #46463
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
  • Loading branch information
addaleax authored and MylesBorins committed Feb 18, 2023
1 parent a51fe3c commit c34bac2
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 29 deletions.
13 changes: 11 additions & 2 deletions src/api/embed_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ EmbedderSnapshotData::Pointer EmbedderSnapshotData::BuiltinSnapshotData() {
SnapshotBuilder::GetEmbeddedSnapshotData(), false)};
}

EmbedderSnapshotData::Pointer EmbedderSnapshotData::FromFile(FILE* in) {
EmbedderSnapshotData::Pointer EmbedderSnapshotData::FromBlob(
const std::vector<char>& in) {
SnapshotData* snapshot_data = new SnapshotData();
CHECK_EQ(snapshot_data->data_ownership, SnapshotData::DataOwnership::kOwned);
EmbedderSnapshotData::Pointer result{
Expand All @@ -302,8 +303,16 @@ EmbedderSnapshotData::Pointer EmbedderSnapshotData::FromFile(FILE* in) {
return result;
}

EmbedderSnapshotData::Pointer EmbedderSnapshotData::FromFile(FILE* in) {
return FromBlob(ReadFileSync(in));
}

std::vector<char> EmbedderSnapshotData::ToBlob() const {
return impl_->ToBlob();
}

void EmbedderSnapshotData::ToFile(FILE* out) const {
impl_->ToBlob(out);
impl_->ToFile(out);
}

EmbedderSnapshotData::EmbedderSnapshotData(const SnapshotData* impl,
Expand Down
6 changes: 4 additions & 2 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -517,11 +517,13 @@ struct SnapshotData {
// v8::ScriptCompiler::CachedData is not copyable.
std::vector<builtins::CodeCacheInfo> code_cache;

void ToBlob(FILE* out) const;
void ToFile(FILE* out) const;
std::vector<char> ToBlob() const;
// If returns false, the metadata doesn't match the current Node.js binary,
// and the caller should not consume the snapshot data.
bool Check() const;
static bool FromBlob(SnapshotData* out, FILE* in);
static bool FromFile(SnapshotData* out, FILE* in);
static bool FromBlob(SnapshotData* out, const std::vector<char>& in);
static const SnapshotData* FromEmbedderWrapper(
const EmbedderSnapshotData* data);
EmbedderSnapshotData::Pointer AsEmbedderWrapper() const;
Expand Down
4 changes: 2 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ ExitCode GenerateAndWriteSnapshotData(const SnapshotData** snapshot_data_ptr,

FILE* fp = fopen(snapshot_blob_path.c_str(), "wb");
if (fp != nullptr) {
(*snapshot_data_ptr)->ToBlob(fp);
(*snapshot_data_ptr)->ToFile(fp);
fclose(fp);
} else {
fprintf(stderr,
Expand Down Expand Up @@ -1168,7 +1168,7 @@ ExitCode LoadSnapshotDataAndRun(const SnapshotData** snapshot_data_ptr,
return exit_code;
}
std::unique_ptr<SnapshotData> read_data = std::make_unique<SnapshotData>();
bool ok = SnapshotData::FromBlob(read_data.get(), fp);
bool ok = SnapshotData::FromFile(read_data.get(), fp);
fclose(fp);
if (!ok) {
// If we fail to read the customized snapshot, simply exit with 1.
Expand Down
2 changes: 2 additions & 0 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -518,11 +518,13 @@ class EmbedderSnapshotData {
// The FILE* handle can be closed immediately following this call.
// If the snapshot is invalid, this returns an empty pointer.
static Pointer FromFile(FILE* in);
static Pointer FromBlob(const std::vector<char>& in);

// Write this EmbedderSnapshotData object to an output file.
// Calling this method will not close the FILE* handle.
// The FILE* handle can be closed immediately following this call.
void ToFile(FILE* out) const;
std::vector<char> ToBlob() const;

// Returns whether custom snapshots can be used. Currently, this means
// that V8 was configured without the shared-readonly-heap feature.
Expand Down
29 changes: 13 additions & 16 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ size_t SnapshotSerializer::Write(const SnapshotMetadata& data) {
// [ ... ] env_info
// [ ... ] code_cache

void SnapshotData::ToBlob(FILE* out) const {
std::vector<char> SnapshotData::ToBlob() const {
SnapshotSerializer w;
w.Debug("SnapshotData::ToBlob()\n");

Expand All @@ -858,9 +858,14 @@ void SnapshotData::ToBlob(FILE* out) const {
written_total += w.Write<EnvSerializeInfo>(env_info);
w.Debug("Write code_cache\n");
written_total += w.WriteVector<builtins::CodeCacheInfo>(code_cache);
size_t num_written = fwrite(w.sink.data(), w.sink.size(), 1, out);
CHECK_EQ(num_written, 1);
w.Debug("SnapshotData::ToBlob() Wrote %d bytes\n", written_total);
return w.sink;
}

void SnapshotData::ToFile(FILE* out) const {
const std::vector<char> sink = ToBlob();
size_t num_written = fwrite(sink.data(), sink.size(), 1, out);
CHECK_EQ(num_written, 1);
}

const SnapshotData* SnapshotData::FromEmbedderWrapper(
Expand All @@ -872,20 +877,12 @@ EmbedderSnapshotData::Pointer SnapshotData::AsEmbedderWrapper() const {
return EmbedderSnapshotData::Pointer{new EmbedderSnapshotData(this, false)};
}

bool SnapshotData::FromBlob(SnapshotData* out, FILE* in) {
CHECK_EQ(ftell(in), 0);
int err = fseek(in, 0, SEEK_END);
CHECK_EQ(err, 0);
size_t size = ftell(in);
CHECK_NE(size, static_cast<size_t>(-1L));
err = fseek(in, 0, SEEK_SET);
CHECK_EQ(err, 0);

std::vector<char> sink(size);
size_t num_read = fread(sink.data(), size, 1, in);
CHECK_EQ(num_read, 1);
bool SnapshotData::FromFile(SnapshotData* out, FILE* in) {
return FromBlob(out, ReadFileSync(in));
}

SnapshotDeserializer r(sink);
bool SnapshotData::FromBlob(SnapshotData* out, const std::vector<char>& in) {
SnapshotDeserializer r(in);
r.Debug("SnapshotData::FromBlob()\n");

DCHECK_EQ(out->data_ownership, SnapshotData::DataOwnership::kOwned);
Expand Down
15 changes: 15 additions & 0 deletions src/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,21 @@ int ReadFileSync(std::string* result, const char* path) {
return 0;
}

std::vector<char> ReadFileSync(FILE* fp) {
CHECK_EQ(ftell(fp), 0);
int err = fseek(fp, 0, SEEK_END);
CHECK_EQ(err, 0);
size_t size = ftell(fp);
CHECK_NE(size, static_cast<size_t>(-1L));
err = fseek(fp, 0, SEEK_SET);
CHECK_EQ(err, 0);

std::vector<char> contents(size);
size_t num_read = fread(contents.data(), size, 1, fp);
CHECK_EQ(num_read, 1);
return contents;
}

void DiagnosticFilename::LocalTime(TIME_TYPE* tm_struct) {
#ifdef _WIN32
GetLocalTime(tm_struct);
Expand Down
2 changes: 2 additions & 0 deletions src/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,8 @@ std::unique_ptr<T> static_unique_pointer_cast(std::unique_ptr<U>&& ptr) {
// Returns a non-zero code if it fails to open or read the file,
// aborts if it fails to close the file.
int ReadFileSync(std::string* result, const char* path);
// Reads all contents of a FILE*, aborts if it fails.
std::vector<char> ReadFileSync(FILE* fp);

v8::Local<v8::FunctionTemplate> NewFunctionTemplate(
v8::Isolate* isolate,
Expand Down
35 changes: 30 additions & 5 deletions test/embedding/embedtest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,30 @@ int RunNodeInstance(MultiIsolatePlatform* platform,
std::find(args.begin(), args.end(), "--embedder-snapshot-create");
auto snapshot_arg_it =
std::find(args.begin(), args.end(), "--embedder-snapshot-blob");
auto snapshot_as_file_it =
std::find(args.begin(), args.end(), "--embedder-snapshot-as-file");
if (snapshot_arg_it < args.end() - 1 &&
snapshot_build_mode_it == args.end()) {
FILE* fp = fopen((snapshot_arg_it + 1)->c_str(), "r");
const char* filename = (snapshot_arg_it + 1)->c_str();
FILE* fp = fopen(filename, "r");
assert(fp != nullptr);
snapshot = node::EmbedderSnapshotData::FromFile(fp);
fclose(fp);
if (snapshot_as_file_it != args.end()) {
snapshot = node::EmbedderSnapshotData::FromFile(fp);
} else {
uv_fs_t req;
int statret = uv_fs_stat(nullptr, &req, filename, nullptr);
assert(statret == 0);
size_t filesize = req.statbuf.st_size;
uv_fs_req_cleanup(&req);

std::vector<char> vec(filesize);
size_t read = fread(vec.data(), filesize, 1, fp);
assert(read == 1);
snapshot = node::EmbedderSnapshotData::FromBlob(vec);
}
assert(snapshot);
int ret = fclose(fp);
assert(ret == 0);
}

std::vector<std::string> errors;
Expand Down Expand Up @@ -125,8 +143,15 @@ int RunNodeInstance(MultiIsolatePlatform* platform,

FILE* fp = fopen((snapshot_arg_it + 1)->c_str(), "w");
assert(fp != nullptr);
snapshot->ToFile(fp);
fclose(fp);
if (snapshot_as_file_it != args.end()) {
snapshot->ToFile(fp);
} else {
const std::vector<char> vec = snapshot->ToBlob();
size_t written = fwrite(vec.data(), vec.size(), 1, fp);
assert(written == 1);
}
int ret = fclose(fp);
assert(ret == 0);
}

node::Stop(env);
Expand Down
7 changes: 5 additions & 2 deletions test/embedding/test-embedding.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,18 @@ function getReadFileCodeForPath(path) {
}

// Basic snapshot support
{
for (const extraSnapshotArgs of [[], ['--embedder-snapshot-as-file']]) {
// readSync + eval since snapshots don't support userland require() (yet)
const snapshotFixture = fixtures.path('snapshot', 'echo-args.js');
const blobPath = path.join(tmpdir.path, 'embedder-snapshot.blob');
const buildSnapshotArgs = [
`eval(${getReadFileCodeForPath(snapshotFixture)})`, 'arg1', 'arg2',
'--embedder-snapshot-blob', blobPath, '--embedder-snapshot-create',
...extraSnapshotArgs,
];
const runEmbeddedArgs = [
'--embedder-snapshot-blob', blobPath, ...extraSnapshotArgs, 'arg3', 'arg4',
];
const runEmbeddedArgs = ['--embedder-snapshot-blob', blobPath, 'arg3', 'arg4'];

fs.rmSync(blobPath, { force: true });
assert.strictEqual(child_process.spawnSync(binary, [
Expand Down

0 comments on commit c34bac2

Please sign in to comment.