From 8c31a0ad9650d918bfc589d18e7bf5a9ac313d8a Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Thu, 10 Apr 2025 10:37:28 +0300 Subject: [PATCH 01/12] Allow builtin modules replacement in compiled scripts --- bytecode.go | 23 ++++++++++++++ compiler_test.go | 83 ++++++++++++++++++++++++++++++++++++++++++++++++ example_test.go | 16 ++++++++++ script.go | 32 +++++++++++++++++++ 4 files changed, 154 insertions(+) diff --git a/bytecode.go b/bytecode.go index f3049cee..419fd72e 100644 --- a/bytecode.go +++ b/bytecode.go @@ -16,6 +16,14 @@ type Bytecode struct { Constants []Object } +func (b *Bytecode) Clone() *Bytecode { + return &Bytecode{ + FileSet: b.FileSet, + MainFunction: b.MainFunction, + Constants: append([]Object{}, b.Constants...), + } +} + // Encode writes Bytecode data to the writer. func (b *Bytecode) Encode(w io.Writer) error { enc := gob.NewEncoder(w) @@ -62,6 +70,21 @@ func (b *Bytecode) FormatConstants() (output []string) { return } +// ReplaceBuiltinModule replaces a builtin module with a new one. +// This is helpful for concurrent script execution, when builtin module does not support +// concurrency and you need to provide custom module instance for each script clone. +func (b *Bytecode) ReplaceBuiltinModule(name string, attrs map[string]Object) { + for i, c := range b.Constants { + switch c := c.(type) { + case *ImmutableMap: + modName := inferModuleName(c) + if modName == name { + b.Constants[i] = (&BuiltinModule{Attrs: attrs}).AsImmutableMap(name) + } + } + } +} + // Decode reads Bytecode data from the reader. func (b *Bytecode) Decode(r io.Reader, modules *ModuleMap) error { if modules == nil { diff --git a/compiler_test.go b/compiler_test.go index 5caf2b03..67760e94 100644 --- a/compiler_test.go +++ b/compiler_test.go @@ -1336,6 +1336,89 @@ func TestCompilerSetImportExt_extension_name_validation(t *testing.T) { } } +func TestCompiler_ReplaceBuiltinModule(t *testing.T) { + sharedValues1 := map[string]int{} + sharedValues2 := map[string]int{} + + createSetter := func(vals map[string]int) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + } + } + + checkValues := func(vals map[string]int, name string, base int) { + require.Equal(t, base, vals["direct"], fmt.Sprintf("unexpected value in %s['direct']", name)) + require.Equal(t, base+1, vals["srcM1"], fmt.Sprintf("unexpected value in %s['srcM1']", name)) + require.Equal(t, base+2, vals["srcM2"], fmt.Sprintf("unexpected value in %s['srcM2']", name)) + } + + srcM1 := ` + m := import("setter") + export { set: m.set } + ` + + srcM2 := ` + m := import("m1") + export { set: m.set } + ` + + code := ` + ss := import("setter") + m1 := import("m1") + m2 := import("m2") + + ss.set("direct", value) + m1.set("srcM1", value+1) // should update shared value under once again (src module shares builtin module instance) + m2.set("srcM2", value+2) // should again update the same value (nested src modules share the same builtin module instance) + ` + + script := tengo.NewScript([]byte(code)) + + // set values + err := script.Add("value", 0) + require.NoError(t, err, "failed to set value in script") + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("setter", createSetter(sharedValues1)) + modules.AddSourceModule("m1", []byte(srcM1)) + modules.AddSourceModule("m2", []byte(srcM2)) + script.SetImports(modules) + + compiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + // Check original script uses builtin module it got at start + compiled.Set("value", 10) + err = compiled.Run() + + require.NoError(t, err, "failed to run original compiled script") + checkValues(sharedValues1, "sharedValues1", 10) + + // Check cloned script uses new builtin module after replacement + clone := compiled.Clone() + clone.Set("value", 20) + clone.ReplaceBuiltinModule("setter", createSetter(sharedValues2)) + err = clone.Run() + + require.NoError(t, err, "failed to run cloned compiled script") + checkValues(sharedValues2, "sharedValues2", 20) + + // Check original script still uses old builtin module + compiled.Set("value", 30) + err = compiled.Run() + + require.NoError(t, err, "failed to run original compiled script") + checkValues(sharedValues1, "sharedValues1", 30) + checkValues(sharedValues2, "sharedValues2", 20) +} + func concatInsts(instructions ...[]byte) []byte { var concat []byte for _, i := range instructions { diff --git a/example_test.go b/example_test.go index 5f074d87..b7358f6e 100644 --- a/example_test.go +++ b/example_test.go @@ -44,3 +44,19 @@ each([a, b, c, d], func(x) { // Output: // 22 288 } + +type TestModule struct { + value int +} + +func (t *TestModule) module() *tengo.BuiltinModule { + return &tengo.BuiltinModule{ + Attrs: map[string]tengo.Object{ + "set": &tengo.UserFunction{Value: func(args ...tengo.Object) (tengo.Object, error) { + i, _ := tengo.ToInt64(args[0]) + t.value = int(i) + return nil, nil + }}, + }, + } +} diff --git a/script.go b/script.go index 16f2944b..be6606f7 100644 --- a/script.go +++ b/script.go @@ -138,6 +138,7 @@ func (s *Script) Compile() (*Compiled, error) { bytecode: bytecode, globals: globals, maxAllocs: s.maxAllocs, + fullClone: true, // we do not share bytecode or global indexes with other clones }, nil } @@ -200,6 +201,7 @@ type Compiled struct { globals []Object maxAllocs int64 lock sync.RWMutex + fullClone bool } // Run executes the compiled script in the virtual machine. @@ -255,6 +257,7 @@ func (c *Compiled) Clone() *Compiled { bytecode: c.bytecode, globals: make([]Object, len(c.globals)), maxAllocs: c.maxAllocs, + fullClone: false, // this clone shares bytecode and global indexes with the 'original' } // copy global objects for idx, g := range c.globals { @@ -265,6 +268,14 @@ func (c *Compiled) Clone() *Compiled { return clone } +func (c *Compiled) ReplaceBuiltinModule(name string, attrs map[string]Object) { + c.lock.Lock() + defer c.lock.Unlock() + + c.prepareForModulesUpdate() + c.bytecode.ReplaceBuiltinModule(name, attrs) +} + // IsDefined returns true if the variable name is defined (has value) before or // after the execution. func (c *Compiled) IsDefined(name string) bool { @@ -336,3 +347,24 @@ func (c *Compiled) Set(name string, value interface{}) error { c.globals[idx] = obj return nil } + +func (c *Compiled) prepareForModulesUpdate() { + if c.fullClone { + return + } + + // To safely modify compiled script internals we need to be sure noone shares + // the same bytecode or global indexes. + // We do not do full copy during Clone() call to skip additional memory allocations + // when they are not needed, leaving Clone() call as optional as it was + // before the 'ReplaceBuiltinModule' feature was added. + + indexes := make(map[string]int, len(c.globalIndexes)) + for name, idx := range c.globalIndexes { + indexes[name] = idx + } + c.globalIndexes = indexes + c.bytecode = c.bytecode.Clone() + + c.fullClone = true +} From a02ad27c41c61d9c1eaf70c82de457e1048a8537 Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Thu, 10 Apr 2025 21:10:05 +0300 Subject: [PATCH 02/12] calculate compiled script size as good as we can in current interfaces --- bytecode.go | 4 ++++ objects.go | 4 ++++ parser/source_file.go | 8 +++++++- script.go | 7 +++++++ 4 files changed, 22 insertions(+), 1 deletion(-) diff --git a/bytecode.go b/bytecode.go index f3049cee..70b4299c 100644 --- a/bytecode.go +++ b/bytecode.go @@ -16,6 +16,10 @@ type Bytecode struct { Constants []Object } +func (b *Bytecode) Size() int64 { + return b.MainFunction.Size() + b.FileSet.Size() + int64(len(b.Constants)) +} + // Encode writes Bytecode data to the writer. func (b *Bytecode) Encode(w io.Writer) error { enc := gob.NewEncoder(w) diff --git a/objects.go b/objects.go index 25745586..c52dbabb 100644 --- a/objects.go +++ b/objects.go @@ -587,6 +587,10 @@ func (o *CompiledFunction) String() string { return "" } +func (o *CompiledFunction) Size() int64 { + return int64(len(o.Instructions) + len(o.SourceMap) + len(o.Free)) +} + // Copy returns a copy of the type. func (o *CompiledFunction) Copy() Object { return &CompiledFunction{ diff --git a/parser/source_file.go b/parser/source_file.go index e9f4b0f5..71e2e8d4 100644 --- a/parser/source_file.go +++ b/parser/source_file.go @@ -26,7 +26,6 @@ func (p SourceFilePos) IsValid() bool { // line valid position without file name and no column (column == 0) // file invalid position with file name // - invalid position without file name -// func (p SourceFilePos) String() string { s := p.Filename if p.IsValid() { @@ -58,6 +57,13 @@ func NewFileSet() *SourceFileSet { } } +func (s *SourceFileSet) Size() (size int64) { + for _, f := range s.Files { + size += int64(f.Size) + } + return size +} + // AddFile adds a new file in the file set. func (s *SourceFileSet) AddFile(filename string, base, size int) *SourceFile { if base < 0 { diff --git a/script.go b/script.go index 16f2944b..6f7283b9 100644 --- a/script.go +++ b/script.go @@ -244,6 +244,13 @@ func (c *Compiled) RunContext(ctx context.Context) (err error) { return } +func (c *Compiled) Size() int64 { + c.lock.RLock() + defer c.lock.RUnlock() + + return c.bytecode.Size() + int64(len(c.globalIndexes)+len(c.globals)) +} + // Clone creates a new copy of Compiled. Cloned copies are safe for concurrent // use by multiple goroutines. func (c *Compiled) Clone() *Compiled { From 9a19ce5ac955170961de3b73c5bbbb33ad04e524 Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Thu, 10 Apr 2025 21:19:27 +0300 Subject: [PATCH 03/12] completely disable release CI --- .github/workflows/release.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index cb363d99..9f0a002e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -2,7 +2,8 @@ name: release on: push: tags: - - '*' + - 'never' + jobs: goreleaser: runs-on: ubuntu-latest From ba11d946501092b8341f1e91272258d5ac28e803 Mon Sep 17 00:00:00 2001 From: Vitalii Popov Date: Fri, 11 Apr 2025 16:48:42 +0300 Subject: [PATCH 04/12] test: concurrent test for ReplaceBuiltinModule --- compiler_test.go | 179 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 165 insertions(+), 14 deletions(-) diff --git a/compiler_test.go b/compiler_test.go index 67760e94..c8561c2b 100644 --- a/compiler_test.go +++ b/compiler_test.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "path/filepath" "strings" + "sync" "testing" "github.com/d5/tengo/v2" @@ -1310,20 +1311,48 @@ func TestCompilerSetImportExt_extension_name_validation(t *testing.T) { requireErr bool msgFail string }{ - {[]string{".tengo"}, []string{".tengo"}, false, - "well-formed extension should not return an error"}, - {[]string{""}, []string{".tengo"}, true, - "empty extension name should return an error"}, - {[]string{"foo"}, []string{".tengo"}, true, - "name without dot prefix should return an error"}, - {[]string{"foo.bar"}, []string{".tengo"}, true, - "malformed extension should return an error"}, - {[]string{"foo."}, []string{".tengo"}, true, - "malformed extension should return an error"}, - {[]string{".mshk"}, []string{".mshk"}, false, - "name with dot prefix should be added"}, - {[]string{".foo", ".bar"}, []string{".foo", ".bar"}, false, - "it should replace instead of appending"}, + { + []string{".tengo"}, + []string{".tengo"}, + false, + "well-formed extension should not return an error", + }, + { + []string{""}, + []string{".tengo"}, + true, + "empty extension name should return an error", + }, + { + []string{"foo"}, + []string{".tengo"}, + true, + "name without dot prefix should return an error", + }, + { + []string{"foo.bar"}, + []string{".tengo"}, + true, + "malformed extension should return an error", + }, + { + []string{"foo."}, + []string{".tengo"}, + true, + "malformed extension should return an error", + }, + { + []string{".mshk"}, + []string{".mshk"}, + false, + "name with dot prefix should be added", + }, + { + []string{".foo", ".bar"}, + []string{".foo", ".bar"}, + false, + "it should replace instead of appending", + }, } { err := c.SetImportFileExt(test.extensions...) if test.requireErr { @@ -1419,6 +1448,128 @@ func TestCompiler_ReplaceBuiltinModule(t *testing.T) { checkValues(sharedValues2, "sharedValues2", 20) } +func TestCompiler_ConcurrentParallelExecution(t *testing.T) { + sharedValues := map[string]int{} + + createBuiltin := func(vals map[string]int, tengoMapValue int64) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + "get": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + return &tengo.Map{Value: map[string]tengo.Object{ + "Value": &tengo.Int{Value: tengoMapValue}, + }}, nil + }, + }, + } + } + + srcM1 := ` + m := import("builtin") + export { set: m.set } + ` + + srcM2 := ` + m := import("m1") + export { set: m.set } + ` + + srcWithConstructor := ` + m := import("builtin") + + globalModuleVariable := m.get().Value + 20 // should update after replace builtin module + export { + get: func() { + return globalModuleVariable + }, + calculate: func(v) { + return v + globalModuleVariable + }, + global: globalModuleVariable + } + ` + + transitModule := ` + m := import("constructor") + global := m.get() + + export { + transitGlobal: func() { + return global + } + } + ` + + code := ` + ss := import("builtin") + m1 := import("m1") + m2 := import("m2") + m3 := import("constructor") + m4 := import("transit") + + ss.set("direct", value1) + m1.set("out", value1+1) // should update shared value under once again (src module shares builtin module instance) + m2.set("out", value1+value2) // should again update the same value (nested src modules share the same builtin module instance) + m2.set("outFromConstructor", m3.get()) + m2.set("outFromClosure", m3.calculate(value2)) + m2.set("outGlobal", m3.global) + m2.set("outGlobalTransit", m4.transitGlobal()) + ` + + script := tengo.NewScript([]byte(code)) + + // set values + err := script.Add("value1", 0) + require.NoError(t, err, "failed to set value in script") + err = script.Add("value2", 0) + require.NoError(t, err, "failed to set value in script") + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("builtin", createBuiltin(sharedValues, 10)) + modules.AddSourceModule("m1", []byte(srcM1)) + modules.AddSourceModule("m2", []byte(srcM2)) + modules.AddSourceModule("transit", []byte(transitModule)) + modules.AddSourceModule("constructor", []byte(srcWithConstructor)) + script.SetImports(modules) + + precompiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + const goroutineCount = 1_000 + wg := sync.WaitGroup{} + for i := 0; i < goroutineCount; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + localMap := make(map[string]int) + const base = 20 + script := precompiled.Clone() + script.ReplaceBuiltinModule("builtin", createBuiltin(localMap, int64(i))) + script.Set("value1", base) + script.Set("value2", i) + err := script.Run() + require.NoError(t, err) + require.Equal(t, base+i, localMap["out"], fmt.Sprintf("i: %d", i)) + require.Equal(t, 20+i, localMap["outFromConstructor"], fmt.Sprintf("constructor i: %d", i)) + require.Equal(t, 2*i+20, localMap["outFromClosure"], fmt.Sprintf("closure i: %d", i)) + require.Equal(t, i+20, localMap["outGlobal"], fmt.Sprintf("global i: %d", i)) + require.Equal(t, i+20, localMap["outGlobalTransit"], fmt.Sprintf("transit i: %d", i)) + require.Equal(t, base, localMap["direct"]) + }() + } + + wg.Wait() +} + func concatInsts(instructions ...[]byte) []byte { var concat []byte for _, i := range instructions { From 577c903333fbe7fbc53b47334e7ef4f5c4afd8c6 Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Fri, 11 Apr 2025 16:58:00 +0300 Subject: [PATCH 05/12] update cache action --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5edb1a3a..197bdcd3 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -15,7 +15,7 @@ jobs: go-version: 1.18 id: go - name: set up Go module cache - uses: actions/cache@v1 + uses: actions/cache@v4 with: path: ~/go/pkg/mod key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} From f200f0fa5d376315a36cd248a053fed8f212455c Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Fri, 11 Apr 2025 17:14:00 +0300 Subject: [PATCH 06/12] doc comments --- bytecode.go | 7 +++++++ objects.go | 2 ++ parser/source_file.go | 1 + script.go | 7 +++++++ 4 files changed, 17 insertions(+) diff --git a/bytecode.go b/bytecode.go index 9cef3eb5..364dfbfe 100644 --- a/bytecode.go +++ b/bytecode.go @@ -16,10 +16,17 @@ type Bytecode struct { Constants []Object } +// Size of the bytecode in bytes +// (as much as we can calculate it without reflection and black magic) func (b *Bytecode) Size() int64 { return b.MainFunction.Size() + b.FileSet.Size() + int64(len(b.Constants)) } +// Clone of the bytecode suitable for modification without affecting the original. +// New Bytecode itself is independent, but all the contents of it are still shared +// with the original. +// The only thing that is not shared with the original is Constants slice, as it might be updated +// by ReplaceBuiltinModule(), which should be safe for clone. func (b *Bytecode) Clone() *Bytecode { return &Bytecode{ FileSet: b.FileSet, diff --git a/objects.go b/objects.go index c52dbabb..ef9185f8 100644 --- a/objects.go +++ b/objects.go @@ -587,6 +587,8 @@ func (o *CompiledFunction) String() string { return "" } +// Size of the compiled function in bytes +// (as much as we can calculate it without reflection and black magic) func (o *CompiledFunction) Size() int64 { return int64(len(o.Instructions) + len(o.SourceMap) + len(o.Free)) } diff --git a/parser/source_file.go b/parser/source_file.go index 71e2e8d4..bb617e95 100644 --- a/parser/source_file.go +++ b/parser/source_file.go @@ -57,6 +57,7 @@ func NewFileSet() *SourceFileSet { } } +// Size of all files in this set in bytes. func (s *SourceFileSet) Size() (size int64) { for _, f := range s.Files { size += int64(f.Size) diff --git a/script.go b/script.go index 6c909455..3adffc32 100644 --- a/script.go +++ b/script.go @@ -246,6 +246,8 @@ func (c *Compiled) RunContext(ctx context.Context) (err error) { return } +// Size of compiled script in bytes +// (as much as we can calculate it without reflection and black magic) func (c *Compiled) Size() int64 { c.lock.RLock() defer c.lock.RUnlock() @@ -275,6 +277,11 @@ func (c *Compiled) Clone() *Compiled { return clone } +// ReplaceBuiltinModule replaces a builtin module with a new one. +// This is helpful for concurrent script execution, when builtin module does not support +// concurrency and you need to provide custom module instance for each script clone. +// +// Remember to call .Clone() to get an instance of the script safe for concurrent use. func (c *Compiled) ReplaceBuiltinModule(name string, attrs map[string]Object) { c.lock.Lock() defer c.lock.Unlock() From 9cbe3ff014ee94eb5d63dd93509171655cc2041b Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Fri, 11 Apr 2025 17:14:00 +0300 Subject: [PATCH 07/12] doc comments --- bytecode.go | 5 +++++ parser/source_file.go | 1 - script.go | 5 +++++ 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/bytecode.go b/bytecode.go index 419fd72e..9d4ba457 100644 --- a/bytecode.go +++ b/bytecode.go @@ -16,6 +16,11 @@ type Bytecode struct { Constants []Object } +// Clone of the bytecode suitable for modification without affecting the original. +// New Bytecode itself is independent, but all the contents of it are still shared +// with the original. +// The only thing that is not shared with the original is Constants slice, as it might be updated +// by ReplaceBuiltinModule(), which should be safe for clone. func (b *Bytecode) Clone() *Bytecode { return &Bytecode{ FileSet: b.FileSet, diff --git a/parser/source_file.go b/parser/source_file.go index e9f4b0f5..2f4eea7e 100644 --- a/parser/source_file.go +++ b/parser/source_file.go @@ -26,7 +26,6 @@ func (p SourceFilePos) IsValid() bool { // line valid position without file name and no column (column == 0) // file invalid position with file name // - invalid position without file name -// func (p SourceFilePos) String() string { s := p.Filename if p.IsValid() { diff --git a/script.go b/script.go index be6606f7..5cff62bb 100644 --- a/script.go +++ b/script.go @@ -268,6 +268,11 @@ func (c *Compiled) Clone() *Compiled { return clone } +// ReplaceBuiltinModule replaces a builtin module with a new one. +// This is helpful for concurrent script execution, when builtin module does not support +// concurrency and you need to provide custom module instance for each script clone. +// +// Remember to call .Clone() to get an instance of the script safe for concurrent use. func (c *Compiled) ReplaceBuiltinModule(name string, attrs map[string]Object) { c.lock.Lock() defer c.lock.Unlock() From 7903cd3b18e725dfa6c97b4752850226ce871105 Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Fri, 11 Apr 2025 17:14:00 +0300 Subject: [PATCH 08/12] doc comments --- bytecode.go | 2 ++ objects.go | 2 ++ parser/source_file.go | 1 + script.go | 2 ++ 4 files changed, 7 insertions(+) diff --git a/bytecode.go b/bytecode.go index 70b4299c..4510210c 100644 --- a/bytecode.go +++ b/bytecode.go @@ -16,6 +16,8 @@ type Bytecode struct { Constants []Object } +// Size of the bytecode in bytes +// (as much as we can calculate it without reflection and black magic) func (b *Bytecode) Size() int64 { return b.MainFunction.Size() + b.FileSet.Size() + int64(len(b.Constants)) } diff --git a/objects.go b/objects.go index c52dbabb..ef9185f8 100644 --- a/objects.go +++ b/objects.go @@ -587,6 +587,8 @@ func (o *CompiledFunction) String() string { return "" } +// Size of the compiled function in bytes +// (as much as we can calculate it without reflection and black magic) func (o *CompiledFunction) Size() int64 { return int64(len(o.Instructions) + len(o.SourceMap) + len(o.Free)) } diff --git a/parser/source_file.go b/parser/source_file.go index 71e2e8d4..bb617e95 100644 --- a/parser/source_file.go +++ b/parser/source_file.go @@ -57,6 +57,7 @@ func NewFileSet() *SourceFileSet { } } +// Size of all files in this set in bytes. func (s *SourceFileSet) Size() (size int64) { for _, f := range s.Files { size += int64(f.Size) diff --git a/script.go b/script.go index 6f7283b9..d2023c4f 100644 --- a/script.go +++ b/script.go @@ -244,6 +244,8 @@ func (c *Compiled) RunContext(ctx context.Context) (err error) { return } +// Size of compiled script in bytes +// (as much as we can calculate it without reflection and black magic) func (c *Compiled) Size() int64 { c.lock.RLock() defer c.lock.RUnlock() From f17ec4803a2465539b07d13d1b564e527a3ab6f9 Mon Sep 17 00:00:00 2001 From: Vitalii Popov Date: Fri, 11 Apr 2025 18:10:49 +0300 Subject: [PATCH 09/12] test: add singleton test --- compiler_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/compiler_test.go b/compiler_test.go index c8561c2b..ac94144c 100644 --- a/compiler_test.go +++ b/compiler_test.go @@ -1484,7 +1484,7 @@ func TestCompiler_ConcurrentParallelExecution(t *testing.T) { srcWithConstructor := ` m := import("builtin") - globalModuleVariable := m.get().Value + 20 // should update after replace builtin module + globalModuleVariable := m.get().Value + 20 export { get: func() { return globalModuleVariable @@ -1507,20 +1507,60 @@ func TestCompiler_ConcurrentParallelExecution(t *testing.T) { } ` + singleton := ` + localState := 0 + export { + get: func() { + return localState + }, + set: func(v) { + localState = v + } + } + ` + object := ` + export { + newObject: func() { + localState := 0 + return { + get: func() { + return localState + }, + set: func(v) { + localState = v + } + } + } + } + ` + code := ` ss := import("builtin") m1 := import("m1") m2 := import("m2") m3 := import("constructor") m4 := import("transit") + singleton := import("singleton") + objectBuilder := import("object") ss.set("direct", value1) m1.set("out", value1+1) // should update shared value under once again (src module shares builtin module instance) m2.set("out", value1+value2) // should again update the same value (nested src modules share the same builtin module instance) - m2.set("outFromConstructor", m3.get()) - m2.set("outFromClosure", m3.calculate(value2)) - m2.set("outGlobal", m3.global) - m2.set("outGlobalTransit", m4.transitGlobal()) + m2.set("outFromConstructor", m3.get()) // should update closure after replace builtin module + m2.set("outFromClosure", m3.calculate(value2)) // should update global in closure module variable after change builtin module + m2.set("outGlobal", m3.global) // should update global variable after change builtin module + m2.set("outGlobalTransit", m4.transitGlobal()) // should update global variable in module after rebuild builtin module + m2.set("singleton", func() { + state := singleton.get() + state = state + 1 + singleton.set(state) + return singleton.get() + }()) // should update global variable in module after rebuild builtin module + m2.set("object", func() { + obj := objectBuilder.newObject() + obj.set(1) + return obj.get() + }()) // should reset after update builtin module ` script := tengo.NewScript([]byte(code)) @@ -1537,6 +1577,8 @@ func TestCompiler_ConcurrentParallelExecution(t *testing.T) { modules.AddSourceModule("m2", []byte(srcM2)) modules.AddSourceModule("transit", []byte(transitModule)) modules.AddSourceModule("constructor", []byte(srcWithConstructor)) + modules.AddSourceModule("singleton", []byte(singleton)) + modules.AddSourceModule("object", []byte(object)) script.SetImports(modules) precompiled, err := script.Compile() @@ -1563,6 +1605,8 @@ func TestCompiler_ConcurrentParallelExecution(t *testing.T) { require.Equal(t, 2*i+20, localMap["outFromClosure"], fmt.Sprintf("closure i: %d", i)) require.Equal(t, i+20, localMap["outGlobal"], fmt.Sprintf("global i: %d", i)) require.Equal(t, i+20, localMap["outGlobalTransit"], fmt.Sprintf("transit i: %d", i)) + require.Equal(t, 1, localMap["singleton"], fmt.Sprintf("singleton i: %d", i)) + require.Equal(t, 1, localMap["object"], fmt.Sprintf("object i: %d", i)) require.Equal(t, base, localMap["direct"]) }() } From c49e6bfa8ed3e4350a63414180e7f49bcf0d60f6 Mon Sep 17 00:00:00 2001 From: Vitalii Popov Date: Mon, 14 Apr 2025 11:13:26 +0300 Subject: [PATCH 10/12] test: split concurrent tests --- compiler_test.go | 434 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 428 insertions(+), 6 deletions(-) diff --git a/compiler_test.go b/compiler_test.go index ac94144c..3402f73c 100644 --- a/compiler_test.go +++ b/compiler_test.go @@ -1400,12 +1400,7 @@ func TestCompiler_ReplaceBuiltinModule(t *testing.T) { code := ` ss := import("setter") - m1 := import("m1") - m2 := import("m2") - - ss.set("direct", value) - m1.set("srcM1", value+1) // should update shared value under once again (src module shares builtin module instance) - m2.set("srcM2", value+2) // should again update the same value (nested src modules share the same builtin module instance) + export { set: m.set } ` script := tengo.NewScript([]byte(code)) @@ -1614,6 +1609,433 @@ func TestCompiler_ConcurrentParallelExecution(t *testing.T) { wg.Wait() } +func TestCompiler_ConcurrentParallelExecution_Out(t *testing.T) { + sharedValues := map[string]int{} + + createBuiltin := func(vals map[string]int, tengoMapValue int64) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + } + } + + srcM1 := ` + m := import("builtin") + export { set: m.set } + ` + + srcM2 := ` + m := import("m1") + export { set: m.set } + ` + + code := ` + ss := import("builtin") + m1 := import("m1") + m2 := import("m2") + + m1.set("out", value1+1) + ` + + script := tengo.NewScript([]byte(code)) + + err := script.Add("value1", 0) + require.NoError(t, err, "failed to set value in script") + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("builtin", createBuiltin(sharedValues, 10)) + modules.AddSourceModule("m1", []byte(srcM1)) + modules.AddSourceModule("m2", []byte(srcM2)) + script.SetImports(modules) + + precompiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + const goroutineCount = 1_000 + wg := sync.WaitGroup{} + for i := 0; i < goroutineCount; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + localMap := make(map[string]int) + const base = 20 + script := precompiled.Clone() + script.ReplaceBuiltinModule("builtin", createBuiltin(localMap, int64(i))) + script.Set("value1", base) + err := script.Run() + require.NoError(t, err) + require.Equal(t, base+1, localMap["out"], fmt.Sprintf("i: %d", i)) + }() + } + + wg.Wait() +} + +func TestCompiler_ConcurrentParallelExecution_OutFromConstructor(t *testing.T) { + sharedValues := map[string]int{} + + createBuiltin := func(vals map[string]int, tengoMapValue int64) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + "get": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + return &tengo.Map{Value: map[string]tengo.Object{ + "Value": &tengo.Int{Value: tengoMapValue}, + }}, nil + }, + }, + } + } + + srcWithConstructor := ` + m := import("builtin") + export { + get: func() { + return m.get().Value + } + } + ` + + code := ` + m := import("constructor") + ss := import("builtin") + ss.set("outFromConstructor", m.get()) + ` + + script := tengo.NewScript([]byte(code)) + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("builtin", createBuiltin(sharedValues, 10)) + modules.AddSourceModule("constructor", []byte(srcWithConstructor)) + script.SetImports(modules) + + precompiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + const goroutineCount = 1_000 + wg := sync.WaitGroup{} + for i := 0; i < goroutineCount; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + localMap := make(map[string]int) + script := precompiled.Clone() + script.ReplaceBuiltinModule("builtin", createBuiltin(localMap, int64(i))) + err := script.Run() + require.NoError(t, err) + require.Equal(t, i, localMap["outFromConstructor"], fmt.Sprintf("constructor i: %d", i)) + }() + } + + wg.Wait() +} + +func TestCompiler_ConcurrentParallelExecution_OutGlobal(t *testing.T) { + sharedValues := map[string]int{} + + createBuiltin := func(vals map[string]int, tengoMapValue int64) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + "get": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + return &tengo.Map{Value: map[string]tengo.Object{ + "Value": &tengo.Int{Value: tengoMapValue}, + }}, nil + }, + }, + } + } + + srcWithConstructor := ` + m := import("builtin") + globalModuleVariable := m.get().Value + 20 + export { + global: globalModuleVariable + } + ` + + code := ` + m := import("constructor") + ss := import("builtin") + ss.set("outGlobal", m.global) + ` + + script := tengo.NewScript([]byte(code)) + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("builtin", createBuiltin(sharedValues, 10)) + modules.AddSourceModule("constructor", []byte(srcWithConstructor)) + script.SetImports(modules) + + precompiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + const goroutineCount = 1_000 + wg := sync.WaitGroup{} + for i := 0; i < goroutineCount; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + localMap := make(map[string]int) + script := precompiled.Clone() + script.ReplaceBuiltinModule("builtin", createBuiltin(localMap, int64(i))) + err := script.Run() + require.NoError(t, err) + require.Equal(t, i+20, localMap["outGlobal"], fmt.Sprintf("global i: %d", i)) + }() + } + + wg.Wait() +} + +func TestCompiler_ConcurrentParallelExecution_OutGlobalTransit(t *testing.T) { + sharedValues := map[string]int{} + + createBuiltin := func(vals map[string]int, tengoMapValue int64) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + "get": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + return &tengo.Map{Value: map[string]tengo.Object{ + "Value": &tengo.Int{Value: tengoMapValue}, + }}, nil + }, + }, + } + } + + srcWithConstructor := ` + m := import("builtin") + globalModuleVariable := m.get().Value + 20 + export { + get: func() { + return globalModuleVariable + } + } + ` + + transitModule := ` + m := import("constructor") + global := m.get() + + export { + transitGlobal: func() { + return global + } + } + ` + + code := ` + m := import("transit") + ss := import("builtin") + ss.set("outGlobalTransit", m.transitGlobal()) + ` + + script := tengo.NewScript([]byte(code)) + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("builtin", createBuiltin(sharedValues, 10)) + modules.AddSourceModule("constructor", []byte(srcWithConstructor)) + modules.AddSourceModule("transit", []byte(transitModule)) + script.SetImports(modules) + + precompiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + const goroutineCount = 1_000 + wg := sync.WaitGroup{} + for i := 0; i < goroutineCount; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + localMap := make(map[string]int) + script := precompiled.Clone() + script.ReplaceBuiltinModule("builtin", createBuiltin(localMap, int64(i))) + err := script.Run() + require.NoError(t, err) + require.Equal(t, i+20, localMap["outGlobalTransit"], fmt.Sprintf("transit i: %d", i)) + }() + } + + wg.Wait() +} + +func TestCompiler_ConcurrentParallelExecution_Singleton(t *testing.T) { + sharedValues := map[string]int{} + + createBuiltin := func(vals map[string]int, tengoMapValue int64) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + } + } + + singleton := ` + localState := 0 + export { + get: func() { + return localState + }, + set: func(v) { + localState = v + } + } + ` + + code := ` + singleton := import("singleton") + ss := import("builtin") + ss.set("singleton", func() { + state := singleton.get() + state = state + 1 + singleton.set(state) + return singleton.get() + }()) + ` + + script := tengo.NewScript([]byte(code)) + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("builtin", createBuiltin(sharedValues, 10)) + modules.AddSourceModule("singleton", []byte(singleton)) + script.SetImports(modules) + + precompiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + const goroutineCount = 1_000 + wg := sync.WaitGroup{} + for i := 0; i < goroutineCount; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + localMap := make(map[string]int) + script := precompiled.Clone() + script.ReplaceBuiltinModule("builtin", createBuiltin(localMap, int64(i))) + err := script.Run() + require.NoError(t, err) + require.Equal(t, 1, localMap["singleton"], fmt.Sprintf("singleton i: %d", i)) + }() + } + + wg.Wait() +} + +func TestCompiler_ConcurrentParallelExecution_Object(t *testing.T) { + sharedValues := map[string]int{} + + createBuiltin := func(vals map[string]int, tengoMapValue int64) map[string]tengo.Object { + return map[string]tengo.Object{ + "set": &tengo.UserFunction{ + Value: func(args ...tengo.Object) (tengo.Object, error) { + n, _ := tengo.ToString(args[0]) + i, _ := tengo.ToInt64(args[1]) + vals[n] = int(i) + return nil, nil + }, + }, + } + } + + object := ` + export { + newObject: func() { + localState := 0 + return { + get: func() { + return localState + }, + set: func(v) { + localState = v + } + } + } + } + ` + + code := ` + objectBuilder := import("object") + ss := import("builtin") + ss.set("object", func() { + obj := objectBuilder.newObject() + obj.set(1) + return obj.get() + }()) + ` + + script := tengo.NewScript([]byte(code)) + + modules := stdlib.GetModuleMap() + modules.AddBuiltinModule("builtin", createBuiltin(sharedValues, 10)) + modules.AddSourceModule("object", []byte(object)) + script.SetImports(modules) + + precompiled, err := script.Compile() + require.NoError(t, err, "failed to compile script") + + const goroutineCount = 1_000 + wg := sync.WaitGroup{} + for i := 0; i < goroutineCount; i++ { + i := i + wg.Add(1) + go func() { + defer wg.Done() + + localMap := make(map[string]int) + script := precompiled.Clone() + script.ReplaceBuiltinModule("builtin", createBuiltin(localMap, int64(i))) + err := script.Run() + require.NoError(t, err) + require.Equal(t, 1, localMap["object"], fmt.Sprintf("object i: %d", i)) + }() + } + + wg.Wait() +} + func concatInsts(instructions ...[]byte) []byte { var concat []byte for _, i := range instructions { From 815b81598608866fd7793b14522d2940783f6f86 Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Thu, 11 Dec 2025 16:43:39 +0300 Subject: [PATCH 11/12] bring formatting back to normal --- .github/workflows/release.yml | 2 +- compiler_test.go | 56 +++++++++-------------------------- 2 files changed, 15 insertions(+), 43 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9f0a002e..7dac3c13 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -2,7 +2,7 @@ name: release on: push: tags: - - 'never' + - '*' jobs: goreleaser: diff --git a/compiler_test.go b/compiler_test.go index dedfb72f..e47d08c8 100644 --- a/compiler_test.go +++ b/compiler_test.go @@ -1311,48 +1311,20 @@ func TestCompilerSetImportExt_extension_name_validation(t *testing.T) { requireErr bool msgFail string }{ - { - []string{".tengo"}, - []string{".tengo"}, - false, - "well-formed extension should not return an error", - }, - { - []string{""}, - []string{".tengo"}, - true, - "empty extension name should return an error", - }, - { - []string{"foo"}, - []string{".tengo"}, - true, - "name without dot prefix should return an error", - }, - { - []string{"foo.bar"}, - []string{".tengo"}, - true, - "malformed extension should return an error", - }, - { - []string{"foo."}, - []string{".tengo"}, - true, - "malformed extension should return an error", - }, - { - []string{".mshk"}, - []string{".mshk"}, - false, - "name with dot prefix should be added", - }, - { - []string{".foo", ".bar"}, - []string{".foo", ".bar"}, - false, - "it should replace instead of appending", - }, + {[]string{".tengo"}, []string{".tengo"}, false, + "well-formed extension should not return an error"}, + {[]string{""}, []string{".tengo"}, true, + "empty extension name should return an error"}, + {[]string{"foo"}, []string{".tengo"}, true, + "name without dot prefix should return an error"}, + {[]string{"foo.bar"}, []string{".tengo"}, true, + "malformed extension should return an error"}, + {[]string{"foo."}, []string{".tengo"}, true, + "malformed extension should return an error"}, + {[]string{".mshk"}, []string{".mshk"}, false, + "name with dot prefix should be added"}, + {[]string{".foo", ".bar"}, []string{".foo", ".bar"}, false, + "it should replace instead of appending"}, } { err := c.SetImportFileExt(test.extensions...) if test.requireErr { From 51e5dbaa8b46828775e65436aa5ddd9669061c65 Mon Sep 17 00:00:00 2001 From: Korenevskiy Denis Date: Wed, 4 Mar 2026 14:46:04 +0300 Subject: [PATCH 12/12] review fixes: inline function for further development safety --- script.go | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/script.go b/script.go index 3adffc32..067e98de 100644 --- a/script.go +++ b/script.go @@ -286,7 +286,23 @@ func (c *Compiled) ReplaceBuiltinModule(name string, attrs map[string]Object) { c.lock.Lock() defer c.lock.Unlock() - c.prepareForModulesUpdate() + if !c.fullClone { + // To safely modify compiled script internals we need to be sure noone shares + // the same bytecode or global indexes. + // We do not do full copy during Clone() call to skip additional memory allocations + // when they are not needed, leaving Clone() call as optional as it was + // before the 'ReplaceBuiltinModule' feature was added. + + indexes := make(map[string]int, len(c.globalIndexes)) + for name, idx := range c.globalIndexes { + indexes[name] = idx + } + c.globalIndexes = indexes + c.bytecode = c.bytecode.Clone() + + c.fullClone = true + } + c.bytecode.ReplaceBuiltinModule(name, attrs) } @@ -361,24 +377,3 @@ func (c *Compiled) Set(name string, value interface{}) error { c.globals[idx] = obj return nil } - -func (c *Compiled) prepareForModulesUpdate() { - if c.fullClone { - return - } - - // To safely modify compiled script internals we need to be sure noone shares - // the same bytecode or global indexes. - // We do not do full copy during Clone() call to skip additional memory allocations - // when they are not needed, leaving Clone() call as optional as it was - // before the 'ReplaceBuiltinModule' feature was added. - - indexes := make(map[string]int, len(c.globalIndexes)) - for name, idx := range c.globalIndexes { - indexes[name] = idx - } - c.globalIndexes = indexes - c.bytecode = c.bytecode.Clone() - - c.fullClone = true -}