Skip to content

BAAS-35403 track object with largest memory in vm value stack#130

Merged
kpatel71716 merged 4 commits into
mongodb-forks:realmfrom
kpatel71716:BAAS-35403
Apr 8, 2025
Merged

BAAS-35403 track object with largest memory in vm value stack#130
kpatel71716 merged 4 commits into
mongodb-forks:realmfrom
kpatel71716:BAAS-35403

Conversation

@kpatel71716
Copy link
Copy Markdown

@kpatel71716 kpatel71716 commented Apr 3, 2025

This PR does the following things:

  • Add a flag to the runtime so we can feature flag the memory tracking change below

  • Track the object with the largest memory used on the goja value stack. Theoretically the Function objects should contain references to other objects we can check memory for. However the largest memory I see is from an Arguments object which might be from some sub function or call happening under the hood when the program finishes. Instead of skipping these types, we will just check member all types since I noticed some incorrect tracked sizes in the BenchmarkVmMemTracking benchmark

  • Added benchmarks to test both an entire program execution via the VM and test the vm stack push independently to isolate the mem tracking differences. The benchmarks have some varying results so pasting 5 runs of each below for some reference data. While the first benchmark shows that tracking memory this way does have some overhead. But the second benchmark shows that it's negligible with objects 10k+ bytes in size

go test -benchmem -run=^$ -bench ^BenchmarkStackPushMemTracking$ -count=5            
goos: darwin
goarch: arm64
pkg: github.com/dop251/goja
BenchmarkStackPushMemTracking/with_stack_mem_tracking-10                   74139             17696 ns/op             998 B/op          3 allocs/op
BenchmarkStackPushMemTracking/with_stack_mem_tracking-10                   74660             16418 ns/op            2322 B/op          3 allocs/op
BenchmarkStackPushMemTracking/with_stack_mem_tracking-10                   72896             17230 ns/op            3662 B/op          3 allocs/op
BenchmarkStackPushMemTracking/with_stack_mem_tracking-10                   72631             17569 ns/op            4916 B/op          3 allocs/op
BenchmarkStackPushMemTracking/with_stack_mem_tracking-10                   70515             16899 ns/op            6203 B/op          3 allocs/op
BenchmarkStackPushMemTracking/without_stack_mem_tracking-10              1000000              2890 ns/op            7987 B/op          0 allocs/op
BenchmarkStackPushMemTracking/without_stack_mem_tracking-10               169609              7321 ns/op           17213 B/op          0 allocs/op
BenchmarkStackPushMemTracking/without_stack_mem_tracking-10               198897              6969 ns/op           20239 B/op          0 allocs/op
BenchmarkStackPushMemTracking/without_stack_mem_tracking-10               203395              6893 ns/op           23612 B/op          0 allocs/op
BenchmarkStackPushMemTracking/without_stack_mem_tracking-10               146028              7590 ns/op           26343 B/op          0 allocs/op
go test -benchmem -run=^$ -bench ^BenchmarkVmMemTracking$ -count=5                   
goos: darwin
goarch: arm64
pkg: github.com/dop251/goja
BenchmarkVmMemTracking/with_stack_mem_tracking-10                   5256            229754 ns/op          323156 B/op       1293 allocs/op
BenchmarkVmMemTracking/with_stack_mem_tracking-10                   5337            236230 ns/op          323175 B/op       1294 allocs/op
BenchmarkVmMemTracking/with_stack_mem_tracking-10                   5148            233559 ns/op          323162 B/op       1293 allocs/op
BenchmarkVmMemTracking/with_stack_mem_tracking-10                   5247            222142 ns/op          323187 B/op       1294 allocs/op
BenchmarkVmMemTracking/with_stack_mem_tracking-10                   5116            246885 ns/op          323181 B/op       1294 allocs/op
BenchmarkVmMemTracking/without_stack_mem_tracking-10                5840            203693 ns/op          308787 B/op       1114 allocs/op
BenchmarkVmMemTracking/without_stack_mem_tracking-10                5737            200969 ns/op          308750 B/op       1114 allocs/op
BenchmarkVmMemTracking/without_stack_mem_tracking-10                5820            203425 ns/op          308786 B/op       1114 allocs/op
BenchmarkVmMemTracking/without_stack_mem_tracking-10                5662            226229 ns/op          308799 B/op       1114 allocs/op
BenchmarkVmMemTracking/without_stack_mem_tracking-10                5665            201886 ns/op          308787 B/op       1114 allocs/op

Comment thread vm.go
return
}

valueMemUsage, _ := v.MemUsage(vm.r.stackMemUsageContext)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[minor] Although it rarely happens, what should we expect if v.MemUsage returns an error?

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.

Ah yeah, I'll add a comment saying that we will swallow the error. I didn't want to thread the logger through here for now since I don't see anywhere that err would ever be set to non-nil. Though I could be missing it

Comment thread runtime.go Outdated
Copy link
Copy Markdown

@jwongmongodb jwongmongodb left a comment

Choose a reason for hiding this comment

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

looks good for a first step. Do we want to create a test case in DEV with some fixed data in our atlas clusters from a range of documents totaling more than 300mb? We can have a scheduled event for every 1 minute, if that's not frequent enough, you can make it a database trigger with multiple scheduled triggers to insert more actions on the main trigger to do the same range scan (using a bunch of memory). We can compare the before and after memory usage etc.

@kpatel71716
Copy link
Copy Markdown
Author

looks good for a first step. Do we want to create a test case in DEV with some fixed data in our atlas clusters from a range of documents totaling more than 300mb? We can have a scheduled event for every 1 minute, if that's not frequent enough, you can make it a database trigger with multiple scheduled triggers to insert more actions on the main trigger to do the same range scan (using a bunch of memory). We can compare the before and after memory usage etc.

Imo that would be overkill for now since we just log the value and take no action. But when we start enforcing this value, we can add a unit test that will specifically capture the corrected memory total in our function executions. Once we turn on the flag for candidate apps, we can use the logging to determine how effective this change is

@kpatel71716
Copy link
Copy Markdown
Author

Made a change to clear the visitTracker each time we check memory since I noticed that we don't revise objects that may have changed between the last track. This is a bit of a performance hit, and we could try to optimize this, but looking at the BenchmarkVmMemTracking results, I don't expect this to be too big of a hit. We will test this out incrementally as well, so fine to merge as is

@kpatel71716 kpatel71716 merged commit a444ce5 into mongodb-forks:realm Apr 8, 2025
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants