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: large memory usage with hard-coded map/array #42979

Open
tbhartman opened this issue Dec 3, 2020 · 13 comments
Open

wasm: large memory usage with hard-coded map/array #42979

tbhartman opened this issue Dec 3, 2020 · 13 comments
Labels
arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@tbhartman
Copy link

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

$ go version
go version go1.13.4 windows/amd64

Does this issue reproduce with the latest release?

Unknown; due to my employer's security policy, I cannot install latest on my machine.

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

go env Output
*Some output redacted*
$ go env
set GO111MODULE=
set GOARCH=wasm
set GOBIN=
set GOCACHE=C:\Users\redacted\AppData\Local\go-build
set GOENV=C:\Users\redacted\AppData\Roaming\go\env
set GOEXE=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GONOPROXY=
set GONOSUMDB=
set GOOS=js
set GOPATH=C:\Users\redacted\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set GOWASM=
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=0
set GOMOD=C:\Users\redacted\wasm\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-fPIC -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\redacted\AppData\Local\Temp\1\go-build312734978=/tmp/go-build -gno-record-gcc-switches

What did you do?

Files available at: https://gist.github.com/tbhartman/c496b9cfa9eb3ec1c736fd0849a30e98

I built a Wasm module. Initially, I was trying to use the Goldmark markdown parser, but upon loading the module in the browser, the browser page quickly crashed with a memory error. I identified that the crash was reproduced by only importing "github.com/yuin/goldmark/util", which defines a couple of const maps by hard-coding several thousand elements.

I have created a minimum working example that reproduces the issue (see the gist) by declaring an array (in my example, a string array []string) and hard-coding the filling of that map/array:

ss := make([]string, 3000)
ss[0] = "hi"
ss[1] = "hi"
ss[2] = "hi"
// many lines omitted...
ss[2997] = "hi"
ss[2998] = "hi"
ss[2999] = "hi"

When run in the browser, even after the main() function returned, the browser page memory usage would increase greatly (in my case, over 12 GB). If the page did not crash, once the memory peaked, it would gradually reduce back to normal usage (<100MB).

Performing the same operation in a loop, I never saw the page memory usage increase above 100MB:

ss := make([]string, 3000)
for i := 0; i < 3000; i++ {
    s[i] = "hi"
}

In another test, I broke up the 3000 hard-coded lines into 300 goroutines operating asynchronously, each of which handled 10 of the assignments. In this case, the page memory never increased above 100 MB. (See the example in the gist.)

In an intermediate case, the 3000 hard-coded lines were broken into 6 asynchronous goroutines. In this case, the memory peaked around 2 GB.

What did you expect to see?

A lower memory usage (or, in my original case, no crash).

What did you see instead?

A very high memory usage or a crash.

@dmitshur dmitshur added arch-wasm WebAssembly issues binary-size NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed binary-size labels Dec 3, 2020
@dmitshur dmitshur added this to the Backlog milestone Dec 3, 2020
@dmitshur dmitshur changed the title Wasm large memory usage with hard-coded map/array wasm: large memory usage with hard-coded map/array Dec 3, 2020
@dmitshur
Copy link
Contributor

dmitshur commented Dec 3, 2020

CC @neelance, @cherrymui.

@ratchetfreak
Copy link

From another article https://bvisness.me/chrome-wasm-crash/ this seems related to the structure of the go assembly that jumps to the end of the function and back instead of using a standard if/else.

You don't need arbitrary labels and gotos to support this. Instead having an optimizer stage for go assembly in the wasm pipeline that converts the GC branch to end blocks back into standard if/else should fix this. The pattern looks fairly simple to detect and fix.

@inkeliz
Copy link

inkeliz commented Oct 14, 2021

I hit the same issue, I'm using the text/message to translate texts, and I have one map as the follow:

func texts() map[string]map[language.Tag]catalog.Message {
	return map[string]map[language.Tag]catalog.Message{
		"Copy": {
			language.English:    catalog.String("Copy"),
			language.Portuguese: catalog.String("Copiar"),
			language.Spanish:    catalog.String("Copiar"),
		},
		"Paste": {
			language.English:    catalog.String("Paste"),
			language.Portuguese: catalog.String("Colar"),
			language.Spanish:    catalog.String("Pegar"),
		},
		// ... Continue

On Windows, Android and Linux (on ARM-32, ARM-64 and x64), the entire program uses less than ~30MB. However, on WebAssembly it takes more than 2GB. Then, after some minutes (when it survives) it drops to ~100MB, which is acceptable.


I could mitigate this issue using:

func texts() [334][4]string {
	return [334][4]string{
		{
			"Copy",
			"Copy",
			"Copiar",
			"Copiar",
		},
		{
			"Paste",
			"Paste",
			"Colar",
			"Pegar",
		},

In that case, the index of the [4]string is the language and the first item is the "key". Also, it's a fixed-size array.


Currently, it drops from 2GB to ~500MB, then drops to ~200MB after some seconds. It's now running on Chrome Android just fine.

That issue is very annoying. It's also a undocumented behavior, so I waste a big time trying to find what is "leaking memory". The current solution doesn't match with the memory usage from native executable.

@neelance
Copy link
Member

The root cause of this issue is again that WebAssembly has no goto instruction or some similar solution like funclets.

We can not generally turn this into a flat structure. It could work for the simple example above, but simply changing ss[0] = "hi" to ss[0] = someFunction("hi") would force the compiler to assume that the call to someFunction could switch goroutines and then there is no way to avoid the huge br_table. See this huge issue for more details.

The only somewhat general workaround I can see here is to split the single br_table into multiple br_table and hope that the WebAssembly runtime chokes less on this. But theoretically this makes the CFG even more harder to reconstruct, so it would just mitigate a specific limitation of some specific WebAssembly runtime. My opinion is that this is not a workaround that Go should do, but rather the WebAssembly runtimes should be implemented in a way that does not have a bad complexity class when processing a WebAssembly binary with this structure.

@ratchetfreak
Copy link

The core issue is that the GC allocations for datastructure init turn into this kind of structure: (C-like syntax)

var foo;
if(GC_active)goto GC_register;
//init foo
after_GC:


//other code
return;

GC_register:{
    //alloc and init foo using GC regestiring functions
    goto after_GC;
}

You can transform each each conditional goto pair to an if-else

var foo;
if(GC_active){
    //alloc and init foo using GC regestiring functions
}else{
    //init foo
}

//other code
return;

This will make targets that need structured code very happy, it's a bog standard if-else. At the cost of some branches being less predictable. (does moving the code to the end this even help with that?). If those targets have a likely annotation for branches you can apply those to help alleviate that. Those targets have a pre-process anyway before executing which is where the main issue arises. Adding support for arbitrary gotos to all potential targets is an overreach and unlikely to happen, this can be solved purely on the GO side by obeying the structured code requirements which GO can do..

The other way is duplicating the function and having a branch at the top that checks once but that means you cannot activate the GC in the middle of a function so the logic to move from the non-GC side to the other would still be annoying.

@bvisness
Copy link

bvisness commented Oct 16, 2021

Yeah, I agree with @ratchetfreak on this. While I understand that something like ss[0] = someFunction("hi") may cause problems, that's not the situation the OP and I are in. I see no reason yet why large literals, or a long hardcoded series of initializations, should result in worse code structure. This is a structure entirely provided by the language, not by users, so better codegen should be possible.

It doesn't seem reasonable to me to hold out for the acceptance (and implementation, and release) of a goto or funclet proposal, when the majority of the issue in practice could be solved by changing the generated WASM for core language structures.

@neelance
Copy link
Member

You still need to consider goroutine switching, panics, etc. It is not as trivial as you make it sound like.

On Windows, Android and Linux (on ARM-32, ARM-64 and x64), the entire program uses less than ~30MB. However, on WebAssembly it takes more than 2GB.

Think about it: The Go compiler uses roughly the same approach for all platforms. What does that say about WebAssembly?

I am most likely not going to spend time on this. If you want to see progress, someone else needs to experiment with implementing the suggestion.

@ratchetfreak
Copy link

it says that wasm needs a slightly different approach than platforms which do support arbitrary gotos as one would expect from a target that disallows arbitrary goto and requires structured code.

@inkeliz
Copy link

inkeliz commented Oct 17, 2021

Think about it: The Go compiler uses roughly the same approach for all platforms. What does that say about WebAssembly?

Maybe the "roughly the same approach" isn't working for WebAssembly? Otherwise, all other platform will also use 2GB of RAM to build the map. I'm just saying that the observed behaviour is different between WebAssembly, ARM and amd64. For example, only the WASM crashes under the same memory constraints.

I compile the same code for Windows/amd64 and Android/arm. Only on WebAssembly crashes with "out-of-memory". The same device runs the native Android app, and works fine, but it crashes using the Chrome browser. Most important: removing the mentioned map makes the wasm work again, dropping the memory usage from 2GB to almost 100MB.

As you said, it seems to be a limitation on WebAssembly itself, the lack of goto. But there's no alternative to mitigate that situation? I never expected that one hard-coded map could use more than 2GB of ram, and it was painful to find out the cause of this memory consumption. Also, no other platform uses 2GB while running the same exactly code.

@bvisness
Copy link

@inkeliz The problem is that Go's assembly assumes the existence of arbitrary jumps, which WASM doesn't have. This particular case ends up really pathological because of the transformation to WASM's structured control flow.

One way or another, the issue here is a mismatch between Go's assembly and WASM's control flow. Neelance apparently would rather change WASM to match Go instead of changing Go to match WASM. I don't agree with that approach, since the decision was already made to target WASM as a platform, and none of these alternative control flow proposals have gone anywhere in the proposal process.

I understand that those proposals might solve Go's problems more completely, but I'd much prefer an 80% solution in the near future instead of a 100% solution in the hypothetical far future.

@neelance
Copy link
Member

Neelance apparently would rather change WASM to match Go

I believe a more fair way to phrase this is that I would rather have WASM to match every other major assembly target that already existed before WASM.

@bvisness
Copy link

I really just think that ship has sailed, unless one of those proposals actually moves on to at least phase 1 of the proposal process. Until then I don't think it's reasonable to make support / bug decisions based on changes to WASM.

For what it's worth, I'd be happy to learn my way around the compilation pipeline and take a look at it. Are there resources I can read about the Go compiler in general, and the WASM backend specifically?

@neelance
Copy link
Member

I really just think that ship has sailed

Probably true.

For what it's worth, I'd be happy to learn my way around the compilation pipeline and take a look at it. Are there resources I can read about the Go compiler in general, and the WASM backend specifically?

For the Go compiler there are some README files and long documentation comments that are really good. You have to find the right ones, though.

For the wasm backend specifically there is the design document that I wrote for getting the Go team to accept the new backend. Some parts may be out of date.

I would recommend starting with "Playing with SSA" because you can see what is happening with some example code that you provide yourself.

I am also available on the Gophers Slack for the occasional question.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants