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.

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

wasm: does not return memory to the OS #59061

Closed
achille-roussel opened this issue Mar 15, 2023 · 8 comments
Closed

wasm: does not return memory to the OS #59061

achille-roussel opened this issue Mar 15, 2023 · 8 comments
Labels
arch-wasm WebAssembly issues FeatureRequest FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Plan9
Milestone

Comments

@achille-roussel
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
go version go1.20 darwin/arm64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="arm64"
GOBIN=""
GOCACHE="..."
GOENV="..."
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="..."
GONOPROXY="github.com/..."
GONOSUMDB="github.com/..."
GOOS="darwin"
GOPATH="..."
GOPRIVATE="github.com/..."
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/qc/zv1mt0j11sscgjygfx47nxlm0000gn/T/go-build1776626115=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

As part of the work on #58141, I came across the runtime code that takes care of interfacing between the Go and system memory allocators, and I believe that Go programs built to WebAssembly are leaking all memory they attempt to release to the OS.

The relevant code is in https://github.com/golang/go/blob/master/src/runtime/mem_js.go

The code comment suggest that the initial plan was either to leverage virtual memory capabilities of the host application if the design discussions recorded at https://github.com/WebAssembly/design/blob/master/FutureFeatures.md#finer-grained-control-over-memory had materialized, or use the same allocation strategy used on Plan9 which also uses a linear memory space (via brk/sbrk) https://github.com/golang/go/blob/master/src/runtime/mem_plan9.go

Since neither of these alternatives were implemented, the current implementation leaks all memory blocks that the GC attempts to release to the OS (via calls to runtime.sysFreeOS).

I have verified this hypothesis by adding print statements in runtime.sysReserveOS and runtime.sysFreeOS which show that the runtime.reserveEnd pointer tracking the current size of the WASM memory, is always increasing (verified when running tests for the standard bytes package). It also shows that the size of the WASM memory is ~7x the size of the Go heap in those tests, which further supports the hypothesis of a leak.

sysReserveOS: 0x0 262144
init reserve end to 2293760
update reserve end to 2555904
sysReserveOS: 0x0 65536
update reserve end to 2621440
sysReserveOS: 0x0 8388608
update reserve end to 11010048
sysFreeOS: 0x280000 1572864
sysFreeOS: 0x800000 2621440
sysReserveOS: 0x0 8192
update reserve end to 11075584
sysReserveOS: 0x0 70864
update reserve end to 11206656
sysReserveOS: 0x0 65536
update reserve end to 11272192
sysReserveOS: 0x0 131072
update reserve end to 11403264
sysReserveOS: 0x0 65536
update reserve end to 11468800
sysReserveOS: 0x0 65536
update reserve end to 11534336
sysReserveOS: 0x0 262144
update reserve end to 11796480
sysReserveOS: 0x800000 4194304
sysReserveOS: 0x0 4194304
update reserve end to 15990784
sysFreeOS: 0xb40000 4194304
sysReserveOS: 0x0 8388608
update reserve end to 24379392
sysFreeOS: 0xf40000 786432
sysFreeOS: 0x1400000 3407872
sysReserveOS: 0x0 70864
update reserve end to 24510464
===================================================================
gcController.heapInUse:    3866624
gcController.heapFree:     32768
===================================================================

I looked for open issues related to WebAssembly and memory usage and it is possible that these might be tied back to this memory leak (if confirmed):

What did you expect to see?

I was expecting the size of the WASM memory to be closer to the size of the Go heap.

I was also expecting runtime.sysFreeOS to retain the freed memory blocks in a freelist in order to reuse them when memory is needed again.

What did you see instead?

The size of the WASM memory is ~7x larger than the size of the Go heap.

When the program needs memory, it always increases the size of the WebAssembly linear memory and never reuses memory blocks that it freed.

@Pryz Pryz added the arch-wasm WebAssembly issues label Mar 15, 2023
@johanbrandhorst
Copy link
Member

CC @neelance

@cherrymui cherrymui changed the title GOARCH=wasm: possible memory leak wasm: does not return memory to the OS Mar 15, 2023
@cherrymui
Copy link
Member

Yes, this is a limitation of the current Go Wasm implementation. I also recall there was some discussion about using a strategy to reuse memory similar to what we do on Plan 9, but I don't think it completed.

At the point when the Wasm port is implemented (and may be still true?), there was no way to shrink the Wasm linear memory. So the best strategy is probably reuse it. If you (or anyone) would like to implement the Plan 9 strategy, you're welcome to do so. Thanks.

cc @golang/wasm

@cherrymui cherrymui added help wanted NeedsFix The path to resolution is known, but the work has not been done. FeatureRequest labels Mar 15, 2023
@cherrymui cherrymui added this to the Unplanned milestone Mar 15, 2023
@achille-roussel
Copy link
Contributor Author

I unified the plan9 and wasm memory allocation in https://github.com/achille-roussel/go/compare/master..fix-wasm-memory-leak to confirm the hypothesis.

When TestGrow of the bytes package from master and measuring the size of the WASM memory:

grow wasm memory size 2621440
grow wasm memory size 2686976
grow wasm memory size 11075584
grow wasm memory size 11141120
grow wasm memory size 11272192
grow wasm memory size 11337728
grow wasm memory size 11468800
grow wasm memory size 11534336
grow wasm memory size 11599872
grow wasm memory size 11862016
grow wasm memory size 16056320
grow wasm memory size 24444928
grow wasm memory size 24576000
=== RUN   TestGrow
grow wasm memory size 24838144
grow wasm memory size 24903680
grow wasm memory size 25165824
grow wasm memory size 33554432
grow wasm memory size 33685504
grow wasm memory size 33816576
grow wasm memory size 33882112
grow wasm memory size 35323904
grow wasm memory size 43712512
grow wasm memory size 43843584
grow wasm memory size 56426496
grow wasm memory size 56557568
grow wasm memory size 56688640
grow wasm memory size 77660160
grow wasm memory size 77791232
grow wasm memory size 77922304
grow wasm memory size 78053376
grow wasm memory size 78184448
grow wasm memory size 90767360
grow wasm memory size 90898432
grow wasm memory size 91029504
grow wasm memory size 112001024
grow wasm memory size 112132096
grow wasm memory size 112263168
grow wasm memory size 112394240
grow wasm memory size 112525312
--- PASS: TestGrow (0.02s)
=== RUN   TestGrowOverflow
--- PASS: TestGrowOverflow (0.00s)
PASS
ok  	bytes	0.300s

When running the same test with the change I made:

grow wasm memory size to 18939904
grow wasm memory size to 19202048
grow wasm memory size to 19267584
grow wasm memory size to 27656192
=== RUN   TestGrow
grow wasm memory size to 36044800
grow wasm memory size to 44433408
grow wasm memory size to 52822016
grow wasm memory size to 61210624
grow wasm memory size to 73793536
grow wasm memory size to 94765056
grow wasm memory size to 107347968
--- PASS: TestGrow (0.02s)
=== RUN   TestGrowOverflow
--- PASS: TestGrowOverflow (0.00s)
PASS
ok  	bytes	0.238s

To reproduce:

$ GOOS=js GOARCH=wasm GOROOT=$PWD PATH=$PWD/misc/wasm/:$PATH ./bin/go test -run TestGrow -v ./src/bytes

@achille-roussel
Copy link
Contributor Author

Happy to send a CL with the fix to unify memory allocation with plan9 👍

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/476717 mentions this issue: wasm: reuse freed memory blocks

achille-roussel added a commit to achille-roussel/go that referenced this issue Mar 15, 2023
When compiling Go programs to WebAssembly, the memory allocation strategy
was neither releasing memory to the OS nor reusing blocks freed by calls to
`runtime.sysFreeOS`.

This CL unifies the plan9 and wasm memory management strategy since both
platforms use a linear memory space and do not have a mechanism for
returning memory blocks to the OS.

Fixes golang#59061
achille-roussel added a commit to achille-roussel/go that referenced this issue Mar 17, 2023
When compiling Go programs to WebAssembly, the memory allocation strategy
was neither releasing memory to the OS nor reusing blocks freed by calls to
`runtime.sysFreeOS`.

This CL unifies the plan9 and wasm memory management strategy since both
platforms use a linear memory space and do not have a mechanism for
returning memory blocks to the OS.

Fixes golang#59061
achille-roussel added a commit to achille-roussel/go that referenced this issue Mar 20, 2023
When compiling Go programs to WebAssembly, the memory allocation
strategy was neither releasing memory to the OS nor reusing blocks
freed by calls to runtime.sysFreeOS.

This CL unifies the plan9 and wasm memory management strategy since
both platforms use a linear memory space and do not have a mechanism
for returning memory blocks to the OS.

Fixes golang#59061
achille-roussel added a commit to achille-roussel/go that referenced this issue Mar 20, 2023
When compiling Go programs to WebAssembly, the memory allocation
strategy was neither releasing memory to the OS nor reusing blocks
freed by calls to runtime.sysFreeOS.

This CL unifies the plan9 and wasm memory management strategy since
both platforms use a linear memory space and do not have a mechanism
for returning memory blocks to the OS.

Fixes golang#59061
@dmitshur dmitshur modified the milestones: Unplanned, Go1.21 Mar 21, 2023
@rickieNodereal
Copy link

May I ask when would this feature be landed? Like Go 1.21? Please have a look, thanks. @achille-roussel

@achille-roussel
Copy link
Contributor Author

Hello @rickieNodereal

I believe this change should be released in Go 1.21 since it has been merged, you could use it on tip already as well.

@rickieNodereal
Copy link

rickieNodereal commented Apr 10, 2023

Thanks a lot! @achille-roussel But may I ask, what is "use it on tip already"? Sorry I'm new to Golang, could you explain a bit more detailed, thanks.

@golang golang locked and limited conversation to collaborators Apr 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly issues FeatureRequest FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done. OS-Plan9
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants