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.

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

Jump to bottom

syscall/js: implement GC of refs #35111

Closed
hajimehoshi opened this issue Oct 23, 2019 · 8 comments
Closed

syscall/js: implement GC of refs #35111

hajimehoshi opened this issue Oct 23, 2019 · 8 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.14

Comments

@hajimehoshi
Copy link
Member

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

$ go version
go version go1.13.1 darwin/amd64

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=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/hajimehoshi/Library/Caches/go-build"
GOENV="/Users/hajimehoshi/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/hajimehoshi/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/ht/ky_bwgzs4bd5z1hh02k34x_h0000gn/T/go-build424384589=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Build this program with GOOS=js GOARCH=wasm and see the browser (with wasm_exec.js and so on). I tested this with Chrome Version 77.0.3865.120 (Official Build) (64-bit)

package main

import (
        "syscall/js"
)

var (
        requestAnimationFrame = js.Global().Get("requestAnimationFrame")
)

func main() {
        var cf js.Func
        f := func(this js.Value, args []js.Value) interface{} {
                requestAnimationFrame.Invoke(cf)
                return nil
        }
        cf = js.FuncOf(f)
        f(js.Value{}, nil)
        select {}
}

What did you expect to see?

No memory leak

What did you see instead?

The memory usage keeps increasing in the long run.

The memory usage rises and falls in a cycle fashion, but it seems that the peak memory usage keeps increasing.

graph

crbug.com/120186 might be related.

CC @agnivade
@hajimehoshi
Copy link
Member Author

hajimehoshi commented Oct 23, 2019 •
edited

I could not reproduce this issue on Firefox (69.0.1 (64-bit)) Reproduced also on Firefox

@hajimehoshi
Copy link
Member Author

hajimehoshi commented Oct 23, 2019 •
edited

I inserted a debug printing like this:

@@ -182,6 +182,7 @@
                                if (ref === undefined) {
                                        ref = this._values.length;
                                        this._values.push(v);
+                                        console.log("value", v, "length", this._values.length);
                                        this._refs.set(v, ref);
                                }
                                let typeFlag = 0;

And it looks like _values in wasm_exec.js is bloating.

...
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 716
wasm_exec.js:185 value Arguments [6393.862, callee: (...), Symbol(Symbol.iterator): ƒ] length 717
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 718
wasm_exec.js:185 value Arguments [6410.528, callee: (...), Symbol(Symbol.iterator): ƒ] length 719
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 720
wasm_exec.js:185 value Arguments [6427.194, callee: (...), Symbol(Symbol.iterator): ƒ] length 721
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 722
wasm_exec.js:185 value Arguments [6443.86, callee: (...), Symbol(Symbol.iterator): ƒ] length 723
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 724
wasm_exec.js:185 value Arguments [6460.526, callee: (...), Symbol(Symbol.iterator): ƒ] length 725
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 726
wasm_exec.js:185 value Arguments [6477.192, callee: (...), Symbol(Symbol.iterator): ƒ] length 727
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 728
wasm_exec.js:185 value Arguments [6493.858, callee: (...), Symbol(Symbol.iterator): ƒ] length 729
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 730
wasm_exec.js:185 value Arguments [6510.524, callee: (...), Symbol(Symbol.iterator): ƒ] length 731
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 732
wasm_exec.js:185 value Arguments [6527.19, callee: (...), Symbol(Symbol.iterator): ƒ] length 733
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 734
wasm_exec.js:185 value Arguments [6543.856, callee: (...), Symbol(Symbol.iterator): ƒ] length 735
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 736
wasm_exec.js:185 value Arguments [6560.522, callee: (...), Symbol(Symbol.iterator): ƒ] length 737
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 738
wasm_exec.js:185 value Arguments [6577.188, callee: (...), Symbol(Symbol.iterator): ƒ] length 739
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 740
wasm_exec.js:185 value Arguments [6593.854, callee: (...), Symbol(Symbol.iterator): ƒ] length 741
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 742
wasm_exec.js:185 value Arguments [6610.52, callee: (...), Symbol(Symbol.iterator): ƒ] length 743
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 744
wasm_exec.js:185 value Arguments [6627.186, callee: (...), Symbol(Symbol.iterator): ƒ] length 745
wasm_exec.js:185 value {id: 1, this: undefined, args: Arguments(1)} length 746
...

@hajimehoshi hajimehoshi mentioned this issue Oct 23, 2019
Closed
@dmitshur dmitshur added arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 23, 2019
@dmitshur dmitshur added this to the Backlog milestone Oct 23, 2019
@dmitshur
Copy link
Contributor

/cc @neelance

@hajimehoshi
Copy link
Member Author

What is 'Backlog'?

@dmitshur
Copy link
Contributor

It's a recently introduced new milestone. Read about it in #34376.

@agnivade
Copy link
Contributor

Yeah, it's a known issue and a marked TODO item in wasm_exec.js. From my investigation, it does not appear to be trivial. The stuff at wasm_exec side is simple though. Finalizing the ref from syscall/js side is where the problem lies.

Re-titling the issue to reflect that.

@agnivade agnivade changed the title syscall/js: suspicious memory leak with requestAnimationFrame syscall/js: implement GC of refs Oct 24, 2019
@hajimehoshi
Copy link
Member Author

Thanks.

I understand GC is a TODO item, but besides, I was wondering what are the accumulated objects every frame (Arguments and objects including them).

@gopherbot
Copy link

Change https://golang.org/cl/203600 mentions this issue: syscall/js: garbage collect references to JavaScript values

@neelance neelance modified the milestones: Backlog, Go1.14 Nov 3, 2019
@tmthrgd tmthrgd mentioned this issue Nov 6, 2019
Closed
hajimehoshi added a commit to hajimehoshi/ebiten that referenced this issue Jan 7, 2020
@hajimehoshi
Use Go 1.14 for Wasm
7f4a82d 
Go 1.13 has memory issue on Wasm (golang/go#35111). Use Go 1.14
instead.
@fabioberger fabioberger mentioned this issue Jan 29, 2020
Closed
@golang golang locked and limited conversation to collaborators Nov 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.14
Development

No branches or pull requests
5 participants
@neelance @hajimehoshi @agnivade @dmitshur @gopherbot