Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add v8::Isolate::snapshot_creator_from_existing_snapshot API #973

Merged
merged 36 commits into from
Oct 14, 2022

Conversation

crowlKats
Copy link
Member

@crowlKats crowlKats commented May 18, 2022

This commit adds "v8::Isolate::snapshot_creator_from_existing_snapshot" API,
that allows to create a new snapshot from already existing snapshot.

Following APIs were added as well:

  • "v8::Context::from_snapshot"
  • "v8::Isolate::add_context"

@CLAassistant
Copy link

CLAassistant commented May 18, 2022

CLA assistant check
All committers have signed the CLA.

src/snapshot.rs Outdated Show resolved Hide resolved
src/snapshot.rs Outdated Show resolved Hide resolved
src/snapshot.rs Outdated Show resolved Hide resolved
Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allocate StartupData data in C++ and fill it in Rust.

@bartlomieju
Copy link
Member

Allocate StartupData data in C++ and fill it in Rust.

We'll address that, once we got working end-to-end prototype in deno_core and deno_runtime.

src/snapshot.rs Outdated Show resolved Hide resolved
/// is no way to provide a global object template since we do not create
/// a new global object from template, but we can reuse a global object.
pub fn from_snapshot<'s>(
scope: &mut HandleScope<'s, ()>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the scope here? Can this just take an &mut Isolate?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases you need to use scope immediately after calling this function, I think it's not worth the complexity.

@bartlomieju
Copy link
Member

bartlomieju commented Oct 6, 2022

Blocked on #1087
Blocked on #1098

@bartlomieju bartlomieju changed the title feat: add existing_blob to SnapshotCreator constructor feat: add v8::Isolate::snapshot_creator_from_existing_snapshot API Oct 14, 2022
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bartlomieju bartlomieju merged commit 885e016 into denoland:main Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants