Skip to content

BAAS-35403 add lock to runtime maxStackObjectMemUsage #133

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

Merged
kpatel71716 merged 1 commit into from
Apr 14, 2025
Merged

BAAS-35403 add lock to runtime maxStackObjectMemUsage #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 14 additions & 3 deletions runtime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"reflect"
"runtime"
"strconv"
"sync"
"time"

"golang.org/x/text/collate"
Expand Down Expand Up @@ -214,6 +215,7 @@ type Runtime struct {
// context used for tracking object with largest memory in the value stack. Must be non-nil if shouldTrackMaxMemOnStack is defined
stackMemUsageContext *MemUsageContext
shouldTrackMaxMemOnStack func(funcName string) bool
maxStackObjectMemLock sync.RWMutex
maxStackObjectMem uint64
}

Expand Down Expand Up @@ -585,9 +587,18 @@ func (r *Runtime) MemUsage(ctx *MemUsageContext) (memUsage uint64, err error) {
return memUsage, nil
}

// MaxStackObjectMemUsage returns the memory used by the largest object seen in the vm value stack
func (r *Runtime) MaxStackObjectMemUsage() uint64 {
return r.maxStackObjectMem
// GetMaxStackObjectMemUsage returns the memory used by the largest object seen in the vm value stack
func (r *Runtime) GetMaxStackObjectMemUsage() uint64 {
r.maxStackObjectMemLock.RLock()
mem := r.maxStackObjectMem
r.maxStackObjectMemLock.RUnlock()
return mem
}

func (r *Runtime) setMaxStackObjectMemUsage(mem uint64) {
r.maxStackObjectMemLock.Lock()
r.maxStackObjectMem = mem

@Gabri3l Gabri3l Apr 14, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Logic here is a bit different, not sure it makes a difference (e.g. not using math.Max)

@kpatel71716 kpatel71716 Apr 14, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yep, just using this as a setter here! And used a logical check over Math.max elsewhere to reduce the number of times we call into this to get the lock

r.maxStackObjectMemLock.Unlock()
}

func (r *Runtime) newError(typ *Object, format string, args ...interface{}) Value {
Expand Down
4 changes: 3 additions & 1 deletion vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,9 @@ func (vm *vm) push(v Value) {
// Any error will be swallowed here, though this should never happen.
// If an error occurs, the poller will catch the error when the object is checked
valueMemUsage, _ := v.MemUsage(vm.r.stackMemUsageContext)
vm.r.maxStackObjectMem = uint64(math.Max(float64(vm.r.maxStackObjectMem), float64(valueMemUsage)))
if valueMemUsage > vm.r.GetMaxStackObjectMemUsage() {
vm.r.setMaxStackObjectMemUsage(valueMemUsage)
}
}

func (vm *vm) pop() Value {
Expand Down