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

syscall/js: CopyBytesToGo increased memory allocation #47956

Open
esimov opened this issue Aug 25, 2021 · 3 comments
Open

syscall/js: CopyBytesToGo increased memory allocation #47956

esimov opened this issue Aug 25, 2021 · 3 comments

Comments

@esimov
Copy link

@esimov esimov commented Aug 25, 2021

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

$ go version
go version go1.16.6 darwin/amd64

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

go env Output
$ go env

What did you do?

For the Webassembly version of Pigo face detection library I needed to transform the results returned by the getImageData Javascript method into a Uint8Array, which is the only value type accepted by the js.CopyBytesToGo function. For seamless user interaction it's requested the frame rate to be updated at a high frequency (ideally 60FPS) which cause the memory allocation to increase considerably and in the end the web browser crashes because of Out of memory error.

image

This is my code:

if err := det.UnpackCascades(); err == nil {
		c.wg.Add(1)
		go func() {
			defer c.wg.Done()
			c.renderer = js.FuncOf(func(this js.Value, args []js.Value) interface{} {
				c.reqID = c.window.Call("requestAnimationFrame", c.renderer)
				// Draw the webcam frame to the canvas element
				c.ctx.Call("drawImage", c.video, 0, 0)
				rgba := c.ctx.Call("getImageData", 0, 0, width, height).Get("data")

				uint8Arr := js.Global().Get("Uint8Array").New(rgba)
				js.CopyBytesToGo(data, uint8Arr)
				// pixels := c.rgbaToGrayscale(data)
				// res := det.DetectFaces(pixels, height, width)
				// c.drawDetection(res)

				// if len(res) > 0 {
				// 	c.send(string(c.stringify(res[0])))
				// }
				data = data[:cap(data)]
				return nil
			})
			c.window.Call("requestAnimationFrame", c.renderer)
			c.detectKeyPress()
			<-c.done
			c.renderer.Release()
		}()
		c.wg.Wait()
	}

I commented out the above code section in the hope that memory consumption will decrease, but was not the case, so I'm quite confident that this issue is related to js.CopyBytesToGo method.

What did you expect to see?

To not crash the browser.

What did you see instead?

After a few minutes of continuous running the browser crashes.

@toothrot toothrot changed the title syscall/js - CopyBytesToGo increased memory allocation syscall/js: CopyBytesToGo increased memory allocation Aug 25, 2021
@toothrot
Copy link
Contributor

@toothrot toothrot commented Aug 25, 2021

@cherrymui @neelance

Is there a chance you can design a simpler reproducible case?

Loading

@esimov
Copy link
Author

@esimov esimov commented Aug 29, 2021

I've checked a few other projects of mine which in certain cases relies on the same code base, but having some extra operations and I found that this increased memory consumption issue is not happening there, which means that the GC somehow sweeps out effectively the the allocated memory address.

Here is one of the examples:

https://github.com/esimov/pigo-wasm-demos/blob/d4d7b00d654961bc4c3f7ddebb02862fc0815e96/faceblur/canvas.go#L257

// Substract the image under the detected face region.
imgData := make([]byte, scale*scale*4)
subimg := c.ctx.Call("getImageData", row-scale/2, col-scale/2, scale, scale).Get("data")
uint8Arr := js.Global().Get("Uint8Array").New(subimg)
js.CopyBytesToGo(imgData, uint8Arr)

This code (line 257-261) practically will "force" the GC to do it's job effectively and deallocate the memory address, which means that the js.CopyBytesToGo function for some reason allocates some memory address which are not garbage collected (maybe because internally are still referenced somewhere).

Loading

@EtoDemerzel0427
Copy link

@EtoDemerzel0427 EtoDemerzel0427 commented Aug 30, 2021

@esimov I spot the difference between these two versions: In https://github.com/esimov/ascii-fluid/blob/master/wasm/canvas/canvas.go#L78, the data is declared outside the js.FuncOf, while in the example of pigo-wasm it is inside the js.FuncOf. There might be some tricky parts here to use a global go variable for js and maybe this causes the problem.

And I've already tried to move https://github.com/esimov/ascii-fluid/blob/master/wasm/canvas/canvas.go#L78 inside the js.FuncOf, it turns out the problem went away. You can test on your side for confirmation, but on my local machine it is the case.

As for why this would happen internally and how to deal with similar situations ( I believe in certain cases you will somehow need a global go variable), I would like to know golang community's insights.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants