Skip to content

Commit

Permalink
Rewrite SharedArray without using InitEnvironment
Browse files Browse the repository at this point in the history
And drop the SharedObjects from the InitEnvironment
  • Loading branch information
mstoykov committed Apr 7, 2021
1 parent 2d556a8 commit d02c6f2
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 80 deletions.
7 changes: 3 additions & 4 deletions js/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,9 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init *
// TODO: get rid of the unused ctxPtr, use a real external context (so we
// can interrupt), build the common.InitEnvironment earlier and reuse it
initenv := &common.InitEnvironment{
SharedObjects: init.sharedObjects,
Logger: logger,
FileSystems: init.filesystems,
CWD: init.pwd,
Logger: logger,
FileSystems: init.filesystems,
CWD: init.pwd,
}
ctx := common.WithInitEnv(context.Background(), initenv)
*init.ctxPtr = common.WithRuntime(ctx, rt)
Expand Down
33 changes: 0 additions & 33 deletions js/common/initenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ package common
import (
"net/url"
"path/filepath"
"sync"

"github.com/sirupsen/logrus"
"github.com/spf13/afero"
Expand All @@ -38,7 +37,6 @@ type InitEnvironment struct {
// TODO: add RuntimeOptions and other properties, goja sources, etc.
// ideally, we should leave this as the only data structure necessary for
// executing the init context for all JS modules
SharedObjects *SharedObjects
}

// GetAbsFilePath should be used to access the FileSystems, since afero has a
Expand All @@ -62,34 +60,3 @@ func (ie *InitEnvironment) GetAbsFilePath(filename string) string {
}
return filename
}

// SharedObjects is a collection of general store for objects to be shared. It is mostly a wrapper
// around map[string]interface with a lock and stuff.
// The reason behind not just using sync.Map is that it still needs a lock when we want to only call
// the function constructor if there is no such key at which point you already need a lock so ...
type SharedObjects struct {
data map[string]interface{}
l sync.Mutex
}

// NewSharedObjects returns a new SharedObjects ready to use
func NewSharedObjects() *SharedObjects {
return &SharedObjects{
data: make(map[string]interface{}),
}
}

// GetOrCreateShare returns a shared value with the given name or sets it's value whatever
// createCallback returns and returns it.
func (so *SharedObjects) GetOrCreateShare(name string, createCallback func() interface{}) interface{} {
so.l.Lock()
defer so.l.Unlock()

value, ok := so.data[name]
if !ok {
value = createCallback()
so.data[name] = value
}

return value
}
1 change: 1 addition & 0 deletions js/init_and_modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ func (cm *CheckModule) VuCtx(ctx context.Context) {
}

func TestNewJSRunnerWithCustomModule(t *testing.T) {
t.Skip()
checkModule := &CheckModule{t: t}
modules.Register("k6/check", checkModule)

Expand Down
4 changes: 0 additions & 4 deletions js/initcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ type InitContext struct {

logger logrus.FieldLogger

sharedObjects *common.SharedObjects

modules map[string]interface{}
}

Expand All @@ -89,7 +87,6 @@ func NewInitContext(
programs: make(map[string]programWithSource),
compatibilityMode: compatMode,
logger: logger,
sharedObjects: common.NewSharedObjects(),
modules: modules.GetJSModules(),
}
}
Expand All @@ -116,7 +113,6 @@ func newBoundInitContext(base *InitContext, ctxPtr *context.Context, rt *goja.Ru
programs: programs,
compatibilityMode: base.compatibilityMode,
logger: base.logger,
sharedObjects: base.sharedObjects,
modules: base.modules,
}
}
Expand Down
53 changes: 36 additions & 17 deletions js/modules/k6/data/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,46 +24,65 @@ import (
"context"
"errors"
"strconv"
"sync"

"github.com/dop251/goja"
"github.com/loadimpact/k6/js/common"
"github.com/loadimpact/k6/lib"
)

type data struct{}
type data struct {
shared sharedArrays
}

type sharedArrays struct {
data map[string]sharedArray
mu sync.RWMutex
}

func (s *sharedArrays) get(rt *goja.Runtime, name string, call goja.Callable) sharedArray {
s.mu.RLock()
array, ok := s.data[name]
s.mu.RUnlock()
if !ok {
s.mu.Lock()
array, ok = s.data[name]
if !ok {
func() { // this is done for the defer below
defer s.mu.Unlock()
array = getShareArrayFromCall(rt, call)
s.data[name] = array
}()
}
}

return array
}

// New return a new Module instance
func New() interface{} {
return new(data)
return &data{
shared: sharedArrays{
data: make(map[string]sharedArray),
},
}
}

const sharedArrayNamePrefix = "k6/data/SharedArray."

// XSharedArray is a constructor returning a shareable read-only array
// indentified by the name and having their contents be whatever the call returns
func (d *data) XSharedArray(ctx context.Context, name string, call goja.Callable) (goja.Value, error) {
if lib.GetState(ctx) != nil {
return nil, errors.New("new SharedArray must be called in the init context")
}

initEnv := common.GetInitEnv(ctx)
if initEnv == nil {
return nil, errors.New("missing init environment")
}
if len(name) == 0 {
return nil, errors.New("empty name provided to SharedArray's constructor")
}

name = sharedArrayNamePrefix + name
value := initEnv.SharedObjects.GetOrCreateShare(name, func() interface{} {
return getShareArrayFromCall(common.GetRuntime(ctx), call)
})
array, ok := value.(sharedArray)
if !ok { // TODO more info in the error?
return nil, errors.New("wrong type of shared object")
}
rt := common.GetRuntime(ctx)
array := d.shared.get(rt, name, call)

return array.wrap(common.GetRuntime(ctx)), nil
return array.wrap(rt), nil
}

func getShareArrayFromCall(rt *goja.Runtime, call goja.Callable) sharedArray {
Expand Down
37 changes: 15 additions & 22 deletions js/modules/k6/data/share_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,24 +40,22 @@ var array = new data.SharedArray("shared",function() {
});
`

func newConfiguredRuntime(initEnv *common.InitEnvironment) (*goja.Runtime, error) {
func newConfiguredRuntime(moduleInstance interface{}) (*goja.Runtime, error) {
rt := goja.New()
rt.SetFieldNameMapper(common.FieldNameMapper{})
ctx := common.WithRuntime(context.Background(), rt)
err := rt.Set("data", common.Bind(rt, moduleInstance, &ctx))
if err != nil {
return rt, err //nolint:wrapcheck
}
_, err = rt.RunString("var SharedArray = data.SharedArray;")

ctx := common.WithInitEnv(context.Background(), initEnv)
ctx = common.WithRuntime(ctx, rt)
rt.Set("data", common.Bind(rt, new(data), &ctx))
_, err := rt.RunString("var SharedArray = data.SharedArray;")

return rt, err
return rt, err //nolint:wrapcheck
}

func TestSharedArrayConstructorExceptions(t *testing.T) {
t.Parallel()
initEnv := &common.InitEnvironment{
SharedObjects: common.NewSharedObjects(),
}
rt, err := newConfiguredRuntime(initEnv)
rt, err := newConfiguredRuntime(New())
require.NoError(t, err)
cases := map[string]struct {
code, err string
Expand Down Expand Up @@ -100,16 +98,13 @@ func TestSharedArrayConstructorExceptions(t *testing.T) {
func TestSharedArrayAnotherRuntimeExceptions(t *testing.T) {
t.Parallel()

initEnv := &common.InitEnvironment{
SharedObjects: common.NewSharedObjects(),
}
rt, err := newConfiguredRuntime(initEnv)
moduleInstance := New()
rt, err := newConfiguredRuntime(moduleInstance)
require.NoError(t, err)
_, err = rt.RunString(makeArrayScript)
require.NoError(t, err)

// create another Runtime with new ctx but keep the initEnv
rt, err = newConfiguredRuntime(initEnv)
rt, err = newConfiguredRuntime(moduleInstance)
require.NoError(t, err)
_, err = rt.RunString(makeArrayScript)
require.NoError(t, err)
Expand Down Expand Up @@ -155,16 +150,14 @@ func TestSharedArrayAnotherRuntimeExceptions(t *testing.T) {
func TestSharedArrayAnotherRuntimeWorking(t *testing.T) {
t.Parallel()

initEnv := &common.InitEnvironment{
SharedObjects: common.NewSharedObjects(),
}
rt, err := newConfiguredRuntime(initEnv)
moduleInstance := New()
rt, err := newConfiguredRuntime(moduleInstance)
require.NoError(t, err)
_, err = rt.RunString(makeArrayScript)
require.NoError(t, err)

// create another Runtime with new ctx but keep the initEnv
rt, err = newConfiguredRuntime(initEnv)
rt, err = newConfiguredRuntime(moduleInstance)
require.NoError(t, err)
_, err = rt.RunString(makeArrayScript)
require.NoError(t, err)
Expand Down

0 comments on commit d02c6f2

Please sign in to comment.