-
-
Notifications
You must be signed in to change notification settings - Fork 240
Add SnapshotCreator API #281
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
base: master
Are you sure you want to change the base?
Changes from all commits
6b4e736
b6360e2
44297f8
88b05d0
b66880e
fbf1281
ebdfa43
acaa5fb
4030029
ef9e910
eec9dcd
ebb5fa8
3a6bb0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,118 @@ | ||||||
// Copyright 2021 the v8go contributors. All rights reserved. | ||||||
// Use of this source code is governed by a BSD-style license that can be | ||||||
// found in the LICENSE file. | ||||||
|
||||||
package v8go | ||||||
|
||||||
// #include <stdlib.h> | ||||||
// #include "v8go.h" | ||||||
import "C" | ||||||
import ( | ||||||
"errors" | ||||||
"unsafe" | ||||||
) | ||||||
|
||||||
type FunctionCodeHandling int | ||||||
|
||||||
// Clear - does not keep any compiled data prior to serialization/deserialization/verify pass | ||||||
// Keep - keeps any compiled data prior to serialization/deserialization/verify pass | ||||||
const ( | ||||||
FunctionCodeHandlingClear FunctionCodeHandling = iota | ||||||
FunctionCodeHandlingKeep | ||||||
) | ||||||
|
||||||
// StartupData stores the snapshot blob data | ||||||
type StartupData struct { | ||||||
data []byte | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we want to be able to persist or transfer the bytes so that the snapshot can be reused cross-process, similar to CompilerCachedData? To allow that, we need the bytes field to be exported so they can be used for IPC and so the struct can be constructed in another process.
Suggested change
|
||||||
raw_size C.int | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like this field could be removed, since the slice holds the data size |
||||||
} | ||||||
|
||||||
// SnapshotCreator allows creating snapshot. | ||||||
type SnapshotCreator struct { | ||||||
ptr C.SnapshotCreatorPtr | ||||||
iso *Isolate | ||||||
defaultContextAdded bool | ||||||
} | ||||||
|
||||||
// NewSnapshotCreator creates a new snapshot creator. | ||||||
func NewSnapshotCreator() *SnapshotCreator { | ||||||
v8once.Do(func() { | ||||||
C.Init() | ||||||
}) | ||||||
|
||||||
rtn := C.NewSnapshotCreator() | ||||||
|
||||||
return &SnapshotCreator{ | ||||||
ptr: rtn.creator, | ||||||
iso: &Isolate{ptr: rtn.iso}, | ||||||
defaultContextAdded: false, | ||||||
} | ||||||
} | ||||||
|
||||||
// GetIsolate returns the Isolate associated with the SnapshotCreator. | ||||||
// This Isolate must be used to create the contexts that later will be used to create the snapshot blob. | ||||||
func (s *SnapshotCreator) GetIsolate() (*Isolate, error) { | ||||||
if s.ptr == nil { | ||||||
return nil, errors.New("v8go: Cannot get Isolate after creating the blob") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth panicking here trying to call a function on a snapshot creator thats likely been disposed of (either through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that the pattern among other functions #118 (comment) Should we panic in all of the instances. It seems that it would make working with the Snapshot creator API much easier. snapshotCreator := v8.NewSnapshotCreator()
snapshotCreatorIso := snapshotCreator.GetIsolate()
snapshotCreatorCtx := v8.NewContext(snapshotCreatorIso)
defer snapshotCreatorCtx.Close()
snapshotCreatorCtx.RunScript(`const add = (a, b) => a + b`, "add.js")
snapshotCreatorCtx.RunScript(`function run() { return add(3, 4); }`, "main.js")
snapshotCreator.SetDefaultContext(snapshotCreatorCtx)
data := snapshotCreator.Create(v8.FunctionCodeHandlingClear)
iso := v8.NewIsolate(v8.WithStartupData(data))
defer iso.Dispose()
ctx, err := v8.NewContextFromSnapshot(iso, index)
defer ctx.Close() The only one that I'm not sure 100% we should panic is the one that errors when calling What do you think @genevieve? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets stick with the error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should return errors when we expect the caller to handle them. The V8 API already does that, so we can normally keep the API similar to V8 in that respect, except that we use a panic to protect against crashes. The API will especially be annoying when getters like this one return an error. |
||||||
} | ||||||
|
||||||
return s.iso, nil | ||||||
} | ||||||
|
||||||
// SetDefaultContext set the default context to be included in the snapshot blob. | ||||||
func (s *SnapshotCreator) SetDefaultContext(ctx *Context) error { | ||||||
if s.defaultContextAdded { | ||||||
return errors.New("v8go: Cannot set multiple default context for snapshot creator") | ||||||
} | ||||||
|
||||||
C.SetDefaultContext(s.ptr, ctx.ptr) | ||||||
s.defaultContextAdded = true | ||||||
ctx.ptr = nil | ||||||
genevieve marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
return nil | ||||||
} | ||||||
|
||||||
// AddContext add additional context to be included in the snapshot blob. | ||||||
// Returns the index of the context in the snapshot blob, that later can be use to call v8go.NewContextFromSnapshot. | ||||||
func (s *SnapshotCreator) AddContext(ctx *Context) (int, error) { | ||||||
if s.ptr == nil { | ||||||
return 0, errors.New("v8go: Cannot add context to snapshot creator after creating the blob") | ||||||
} | ||||||
|
||||||
index := C.AddContext(s.ptr, ctx.ptr) | ||||||
ctx.ptr = nil | ||||||
|
||||||
return int(index), nil | ||||||
} | ||||||
|
||||||
// Create creates a snapshot data blob. | ||||||
GustavoCaso marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
// The snapshot creator instance is unsable after creating the snapshot data blob. | ||||||
func (s *SnapshotCreator) Create(functionCode FunctionCodeHandling) (*StartupData, error) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. StartupData is basically just a wrapper around a slice. It may as well be returned without the extra pointer indirection, since the error would indicate whether or not it is an error. Since V8 doesn't actually expose an error message, |
||||||
if s.ptr == nil { | ||||||
return nil, errors.New("v8go: Cannot use snapshot creator after creating the blob") | ||||||
} | ||||||
|
||||||
if !s.defaultContextAdded { | ||||||
return nil, errors.New("v8go: Cannot create a snapshot without a default context") | ||||||
} | ||||||
|
||||||
rtn := C.CreateBlob(s.ptr, C.int(functionCode)) | ||||||
|
||||||
s.ptr = nil | ||||||
s.iso.ptr = nil | ||||||
genevieve marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
raw_size := rtn.raw_size | ||||||
data := C.GoBytes(unsafe.Pointer(rtn.data), raw_size) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The v8::SnapshotCreator::CreateBlob documentation says it returns "{ nullptr, 0 } on failure". If this function is going to return an error, then I would definitely expect an error to be returned when V8 would return an error. Unfortunately, V8 is quite vague on what possible reasons there are for an error and the API doesn't expose an error message. |
||||||
|
||||||
C.SnapshotBlobDelete(rtn) | ||||||
|
||||||
return &StartupData{data: data, raw_size: raw_size}, nil | ||||||
} | ||||||
|
||||||
// Dispose deletes the reference to the SnapshotCreator. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems worth mentioning that this also disposes of the isolate associated with it |
||||||
func (s *SnapshotCreator) Dispose() { | ||||||
if s.ptr != nil { | ||||||
C.DeleteSnapshotCreator(s.ptr) | ||||||
s.ptr = nil | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like the isolate's internal pointer should be marked as |
||||||
} | ||||||
GustavoCaso marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} |
Uh oh!
There was an error while loading. Please reload this page.