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

Jump to bottom

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
Labels
arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-JS
Milestone
Backlog

Comments

@esimov
Copy link

esimov commented Aug 25, 2021 •
edited

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.
@esimov esimov mentioned this issue Aug 25, 2021
Closed
@toothrot toothrot changed the title syscall/js - CopyBytesToGo increased memory allocation syscall/js: CopyBytesToGo increased memory allocation Aug 25, 2021
@toothrot
Copy link
Contributor

@cherrymui @neelance

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

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 25, 2021
@toothrot toothrot added this to the Backlog milestone Aug 25, 2021
@dmitshur dmitshur added arch-wasm WebAssembly issues OS-JS labels Aug 25, 2021
@esimov
Copy link
Author

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).

@EtoDemerzel0427
Copy link

EtoDemerzel0427 commented Aug 30, 2021 •
edited

@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.

@achille-roussel achille-roussel mentioned this issue Mar 15, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-JS
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@toothrot @esimov @dmitshur @EtoDemerzel0427