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.

Sign up for GitHub

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

Jump to bottom

Small js refactor #2373

Merged
merged 7 commits into from
Feb 4, 2022
Merged

Small js refactor #2373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7e2ba38
Move moduleVUImpl to be part of the js.VU
mstoykov Feb 4, 2022
b4764e8
Don't use context to get vuModuleImpl fields
mstoykov Feb 4, 2022
54f1786
Drop runtime field in init context
mstoykov Feb 4, 2022
7377205
Drop ctxPtr field in init context
mstoykov Feb 4, 2022
0752d37
Drop usage of common.Bind for initContext functions
mstoykov Feb 4, 2022
c878391
Drop common.Bind for console through dropping the context there
mstoykov Feb 4, 2022
8e9ecd0
Remove one unneeded usage of common.BindToGlobal
mstoykov Feb 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 16 additions & 13 deletions js/bundle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,22 +252,21 @@ func (b *Bundle) getExports(logger logrus.FieldLogger, rt *goja.Runtime, options
}

// Instantiate creates a new runtime from this bundle.
func (b *Bundle) Instantiate(logger logrus.FieldLogger, vuID uint64) (bi *BundleInstance, instErr error) {
// TODO: actually use a real context here, so that the instantiation can be killed
// Placeholder for a real context.
ctxPtr := new(context.Context)

func (b *Bundle) Instantiate(
logger logrus.FieldLogger, vuID uint64, vuImpl *moduleVUImpl,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger logrus.FieldLogger, vuID uint64, vuImpl *moduleVUImpl,
logger logrus.FieldLogger, vuID uint64, vuImpl *moduleVUImpl,

Could now we have vuID as part of vuImpl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

probably in another PR see #2373 (comment)

) (bi *BundleInstance, instErr error) {
// Instantiate the bundle into a new VM using a bound init context. This uses a context with a
// runtime, but no state, to allow module-provided types to function within the init context.
rt := goja.New()
init := newBoundInitContext(b.BaseInitContext, ctxPtr, rt)
if err := b.instantiate(logger, rt, init, vuID); err != nil {
vuImpl.runtime = goja.New()
Comment on lines 258 to +260
Copy link
Contributor

@codebien codebien Feb 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Could we now directly inject a vuImpl with an initialized runtime so we can maintain the initialization in one central place? (e.g. runner.newVU)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in a followign PR :P There are in practice a lot more changes, I just really needed moduleVUImpl to be part of VU and everything else was gravy

init := newBoundInitContext(b.BaseInitContext, vuImpl)
if err := b.instantiate(logger, vuImpl.runtime, init, vuID); err != nil {
return nil, err
}

rt := vuImpl.runtime
bi = &BundleInstance{
Runtime: rt,
Context: ctxPtr,
Context: vuImpl.ctxPtr,
exports: make(map[string]goja.Callable),
env: b.RuntimeOptions.Env,
}
Expand Down Expand Up @@ -315,7 +314,7 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *
}
rt.Set("__ENV", env)
rt.Set("__VU", vuID)
rt.Set("console", common.Bind(rt, newConsole(logger), init.ctxPtr))
_ = rt.Set("console", newConsole(logger))

if init.compatibilityMode == lib.CompatibilityModeExtended {
rt.Set("global", rt.GlobalObject())
Expand All @@ -329,9 +328,13 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *
CWD: init.pwd,
Registry: b.registry,
}
init.moduleVUImpl.initEnv = initenv
ctx := common.WithInitEnv(context.Background(), initenv)
*init.ctxPtr = common.WithRuntime(ctx, rt)
unbindInit := common.BindToGlobal(rt, common.Bind(rt, init, init.ctxPtr))
*init.moduleVUImpl.ctxPtr = common.WithRuntime(ctx, rt)
unbindInit := common.BindToGlobal(rt, map[string]interface{}{
"require": init.Require,
"open": init.Open,
})
if _, err := rt.RunProgram(b.Program); err != nil {
var exception *goja.Exception
if errors.As(err, &exception) {
Expand All @@ -340,7 +343,7 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *
return err
}
unbindInit()
*init.ctxPtr = nil
*init.moduleVUImpl.ctxPtr = nil

// If we've already initialized the original VU init context, forbid
// any subsequent VUs to open new files
Expand Down
18 changes: 9 additions & 9 deletions js/bundle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ func TestNewBundleFromArchive(t *testing.T) {
logger := testutils.NewLogger(t)
checkBundle := func(t *testing.T, b *Bundle) {
assert.Equal(t, lib.Options{VUs: null.IntFrom(12345)}, b.Options)
bi, err := b.Instantiate(logger, 0)
bi, err := b.Instantiate(logger, 0, newModuleVUImpl())
require.NoError(t, err)
val, err := bi.exports[consts.DefaultFn](goja.Undefined())
require.NoError(t, err)
Expand Down Expand Up @@ -598,7 +598,7 @@ func TestNewBundleFromArchive(t *testing.T) {
}
b, err := NewBundleFromArchive(logger, arc, lib.RuntimeOptions{}, metrics.NewRegistry())
require.NoError(t, err)
bi, err := b.Instantiate(logger, 0)
bi, err := b.Instantiate(logger, 0, newModuleVUImpl())
require.NoError(t, err)
val, err := bi.exports[consts.DefaultFn](goja.Undefined())
require.NoError(t, err)
Expand Down Expand Up @@ -742,7 +742,7 @@ func TestOpen(t *testing.T) {
for source, b := range map[string]*Bundle{"source": sourceBundle, "archive": arcBundle} {
b := b
t.Run(source, func(t *testing.T) {
bi, err := b.Instantiate(logger, 0)
bi, err := b.Instantiate(logger, 0, newModuleVUImpl())
require.NoError(t, err)
v, err := bi.exports[consts.DefaultFn](goja.Undefined())
require.NoError(t, err)
Expand Down Expand Up @@ -778,7 +778,7 @@ func TestBundleInstantiate(t *testing.T) {
require.NoError(t, err)
logger := testutils.NewLogger(t)

bi, err := b.Instantiate(logger, 0)
bi, err := b.Instantiate(logger, 0, newModuleVUImpl())
require.NoError(t, err)
v, err := bi.exports[consts.DefaultFn](goja.Undefined())
if assert.NoError(t, err) {
Expand All @@ -799,7 +799,7 @@ func TestBundleInstantiate(t *testing.T) {
require.NoError(t, err)
logger := testutils.NewLogger(t)

bi, err := b.Instantiate(logger, 0)
bi, err := b.Instantiate(logger, 0, newModuleVUImpl())
require.NoError(t, err)
bi.Runtime.Set("val", false)
v, err := bi.exports[consts.DefaultFn](goja.Undefined())
Expand All @@ -821,7 +821,7 @@ func TestBundleInstantiate(t *testing.T) {
require.NoError(t, err)
logger := testutils.NewLogger(t)

bi, err := b.Instantiate(logger, 0)
bi, err := b.Instantiate(logger, 0, newModuleVUImpl())
require.NoError(t, err)
// Ensure `options` properties are correctly marshalled
jsOptions := bi.Runtime.Get("options").ToObject(bi.Runtime)
Expand All @@ -833,7 +833,7 @@ func TestBundleInstantiate(t *testing.T) {
// Ensure options propagate correctly from outside to the script
optOrig := b.Options.VUs
b.Options.VUs = null.IntFrom(10)
bi2, err := b.Instantiate(logger, 0)
bi2, err := b.Instantiate(logger, 0, newModuleVUImpl())
assert.NoError(t, err)
jsOptions = bi2.Runtime.Get("options").ToObject(bi2.Runtime)
vus = jsOptions.Get("vus").Export()
Expand Down Expand Up @@ -869,7 +869,7 @@ func TestBundleEnv(t *testing.T) {
assert.Equal(t, "1", b.RuntimeOptions.Env["TEST_A"])
assert.Equal(t, "", b.RuntimeOptions.Env["TEST_B"])

bi, err := b.Instantiate(logger, 0)
bi, err := b.Instantiate(logger, 0, newModuleVUImpl())
if assert.NoError(t, err) {
_, err := bi.exports[consts.DefaultFn](goja.Undefined())
assert.NoError(t, err)
Expand Down Expand Up @@ -906,7 +906,7 @@ func TestBundleNotSharable(t *testing.T) {
t.Run(name, func(t *testing.T) {
t.Parallel()
for i := 0; i < vus; i++ {
bi, err := b.Instantiate(logger, uint64(i))
bi, err := b.Instantiate(logger, uint64(i), newModuleVUImpl())
require.NoError(t, err)
for j := 0; j < iters; j++ {
bi.Runtime.Set("__ITER", j)
Expand Down
31 changes: 11 additions & 20 deletions js/console.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package js

import (
"context"
"os"
"strings"

Expand Down Expand Up @@ -53,15 +52,7 @@ func newFileConsole(filepath string, formatter logrus.Formatter) (*console, erro
return &console{l}, nil
}

func (c console) log(ctx *context.Context, level logrus.Level, msgobj goja.Value, args ...goja.Value) {
if ctx != nil && *ctx != nil {
select {
case <-(*ctx).Done():
return
default:
}
}

func (c console) log(level logrus.Level, msgobj goja.Value, args ...goja.Value) {
msg := msgobj.String()
if len(args) > 0 {
strs := make([]string, 1+len(args))
Expand All @@ -84,22 +75,22 @@ func (c console) log(ctx *context.Context, level logrus.Level, msgobj goja.Value
}
}

func (c console) Log(ctx *context.Context, msg goja.Value, args ...goja.Value) {
c.Info(ctx, msg, args...)
func (c console) Log(msg goja.Value, args ...goja.Value) {
c.Info(msg, args...)
}

func (c console) Debug(ctx *context.Context, msg goja.Value, args ...goja.Value) {
c.log(ctx, logrus.DebugLevel, msg, args...)
func (c console) Debug(msg goja.Value, args ...goja.Value) {
c.log(logrus.DebugLevel, msg, args...)
}

func (c console) Info(ctx *context.Context, msg goja.Value, args ...goja.Value) {
c.log(ctx, logrus.InfoLevel, msg, args...)
func (c console) Info(msg goja.Value, args ...goja.Value) {
c.log(logrus.InfoLevel, msg, args...)
}

func (c console) Warn(ctx *context.Context, msg goja.Value, args ...goja.Value) {
c.log(ctx, logrus.WarnLevel, msg, args...)
func (c console) Warn(msg goja.Value, args ...goja.Value) {
c.log(logrus.WarnLevel, msg, args...)
}

func (c console) Error(ctx *context.Context, msg goja.Value, args ...goja.Value) {
c.log(ctx, logrus.ErrorLevel, msg, args...)
func (c console) Error(msg goja.Value, args ...goja.Value) {
c.log(logrus.ErrorLevel, msg, args...)
}
15 changes: 1 addition & 14 deletions js/console_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,33 +48,20 @@ func TestConsoleContext(t *testing.T) {
rt := goja.New()
rt.SetFieldNameMapper(common.FieldNameMapper{})

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
ctxPtr := &ctx

logger, hook := logtest.NewNullLogger()
rt.Set("console", common.Bind(rt, &console{logger}, ctxPtr))
_ = rt.Set("console", &console{logger})

_, err := rt.RunString(`console.log("a")`)
assert.NoError(t, err)
if entry := hook.LastEntry(); assert.NotNil(t, entry) {
assert.Equal(t, "a", entry.Message)
}

ctx, cancel = context.WithCancel(context.Background())
*ctxPtr = ctx
_, err = rt.RunString(`console.log("b")`)
assert.NoError(t, err)
if entry := hook.LastEntry(); assert.NotNil(t, entry) {
assert.Equal(t, "b", entry.Message)
}

cancel()
_, err = rt.RunString(`console.log("c")`)
assert.NoError(t, err)
if entry := hook.LastEntry(); assert.NotNil(t, entry) {
assert.Equal(t, "b", entry.Message)
}
}

func getSimpleRunner(tb testing.TB, filename, data string, opts ...interface{}) (*Runner, error) {
Expand Down
Loading