Martin von Gagern Reporter
	Description • 8 years ago

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160925055522

Steps to reproduce:

Write code which is now available at http://gagern.github.io/CindyJS/bugs/2016/10/22/ifs-memleak.html (RUN AT YOUR OWN RISK!).

It contains a web worker running the script http://gagern.github.io/CindyJS/dist/v0.7.3-109-ge24c6dc/ifs.js. The counterpart to that is in http://gagern.github.io/CindyJS/dist/v0.7.3-109-ge24c6dc/Cindy.js starting at line 21777 where it says “new Worker”. The worker generates images and sends them to the main thread. Each image is only generated if the one before has been transferred successfully, so they don't pile up in a message queue. Each image is closed when the next one was received. So I don't see any resource leak here on my part.


Actual results:

Firefox starts consuming memory in large quantities. Memory is usually exhaustet quickly, and swap space soon afterwards. The system becomes unusable until Firefox gets killed. Observed with Firefox compiled from source (Gentoo) and binary releases, on Linux and on OS X. about:memory lists this as Web Content / explicit / heap-unclassified which tells me nothing.


Expected results:

At most two ImageBitmaps of moderate size should not amount to several gigabytes of data, so the memory consumption of the whole process should remain moderate.

	Tooru Fujisawa [:arai]
	Comment 1 • 8 years ago

here's reduced testcase that fills up memory.
looks like createImageBitmap is causing the issue.

	Tooru Fujisawa [:arai]
	Comment 2 • 8 years ago

maybe better one.
this waits for createImageBitmap.

(actually we could do everything in Worker without message passing tho)

	Tooru Fujisawa [:arai]
	Comment 3 • 8 years ago

both with the original testcase and reduced one, if I click "GC" in about:memory frequently (once per second or more) while running the testcase,
the memory usage keeps at almost same amount.

sounds like GC isn't collecting enough, compared to the amount of new allocation.

	Tooru Fujisawa [:arai]
	Comment 4 • 8 years ago

moving to GC component for now.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 5 • 8 years ago

This is mostly likely not a GC issue.

Does the issue happen in the main thread?

	Martin von Gagern Reporter
	Comment 6 • 8 years ago

(In reply to Olli Pettay [:smaug] from comment #5)
> Does the issue happen in the main thread?

It does. I've adapted the test case to single-threaded operations and see the same kind of memory drain.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 7 • 8 years ago

aha, that testcase creates new image bitmaps in a fast pace.
We probably need to inform DOM about the created objects so that it gets re-triggered sooner
( JS_updateMallocCounter)
(I assume here that about:memory GC doesn't help here at least to some extant)

kaku, could you take a look here?

	Tzuhao Kuo [:kaku] Assignee
	Comment 8 • 8 years ago

Sure, will check it soon.

	Tzuhao Kuo [:kaku] Assignee
	Comment 9 • 8 years ago

Thanks smaug, reporting memory allocation does solve the issue. A patch is following.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 11 • 8 years ago
mozreview-review

Comment on attachment 8805508 [details]
Bug 1312148 - report memory allocation while creating ImageBitmap;

https://reviewboard.mozilla.org/r/89256/#review88472

	Tzuhao Kuo [:kaku] Assignee
	Comment 12 • 8 years ago

Try looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2053c3b2b728

	Morris Tseng [:mtseng] [:Morris] (Inactive)
	Comment 13 • 8 years ago
mozreview-review

Comment on attachment 8805508 [details]
Bug 1312148 - report memory allocation while creating ImageBitmap;

https://reviewboard.mozilla.org/r/89256/#review89100

	Tzuhao Kuo [:kaku] Assignee
	Comment 14 • 8 years ago

Thanks for the review!

	Pulsebot
	Comment 15 • 8 years ago

Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/021cef553ece
report memory allocation while creating ImageBitmap; r=mtseng,smaug

	Pulsebot
	Comment 16 • 8 years ago

Backout by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/82f4ae765247
Backed out changeset 021cef553ece for failing test_bitmaprenderer.html on Windows 8 x64 opt. r=backout

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 17 • 8 years ago

Backed out for failing test_bitmaprenderer.html on Windows 8 x64 opt:

https://hg.mozilla.org/integration/autoland/rev/82f4ae7652477629b7bc64d7935b5ae36feebfce

First push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=3bee4763126ec434e91b70b3883c19e1b33272ce
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=6020657&repo=autoland

07:34:36     INFO - TEST-START | dom/canvas/test/test_bitmaprenderer.html
07:34:36     INFO - [GFX1]: Failed to map bitmap (M).
07:34:36     INFO - TEST-INFO | started process screenshot
07:34:36     INFO - TEST-INFO | screenshot: exit 0
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | toDataURL should return same result. 
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | Screenshots should be the same 
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | toDataURL should return same result. 
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | Screenshots should be the same 
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | Screenshots should be the same 
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | Screenshots should be the same 
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | Get the ImageBitmap from the main script. 
07:34:36     INFO - TEST-PASS | dom/canvas/test/test_bitmaprenderer.html | Get bitmaprenderer context on worker. 
07:34:36     INFO - TEST-UNEXPECTED-FAIL | dom/canvas/test/test_bitmaprenderer.html | uncaught exception - NS_ERROR_FAILURE:  at runTestOnWorker/</worker.onmessage@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:159:35
07:34:36     INFO - EventHandlerNonNull*runTestOnWorker/<@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:152:5
07:34:36     INFO - promise callback*runTestOnWorker@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:150:3
07:34:36     INFO - promise callback*scaleTest@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:105:3
07:34:36     INFO - promise callback*runTest@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:55:3
07:34:36     INFO - runTest/<@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:88:5
07:34:36     INFO - promise callback*runTest@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:55:3
07:34:36     INFO - Async*@http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html:166:1
07:34:36     INFO - 
07:34:36     INFO -     simpletestOnerror@SimpleTest/SimpleTest.js:1582:11
07:34:36     INFO -     OnErrorEventHandlerNonNull*@SimpleTest/SimpleTest.js:1562:1
07:34:36     INFO - JavaScript error: http://mochi.test:8888/tests/dom/canvas/test/test_bitmaprenderer.html, line 159: NS_ERROR_FAILURE:
07:34:36     INFO - [GFX1]: Failed to map bitmap (M).
07:34:36     INFO - [GFX1]: Failed to map source surface for D3D11TextureData::Create
07:34:36     INFO - [GFX1]: Failed to map bitmap (M).
07:34:36     INFO - [GFX1]: Failed to map source surface for UpdateFromSurface (BT).
07:34:36     INFO - [GFX1]: Failed to map bitmap (M).

	Tzuhao Kuo [:kaku] Assignee
	Comment 18 • 8 years ago

So, in the test_bitmaprenderer.html, the test case creates a DataSourceSurfaceD2D1 object in a worker thread and transfer it back to the main thread. In my patch, I invoke the DataSourceSurfaceD2D1 object's Stride() method which tries to map its backend data. The mapping operation fails if the DataSourceSurfaceD2D1 object is transferred between threads. :Morris suggests to disable the test case for now, a patch follows.

	Tzuhao Kuo [:kaku] Assignee
	Comment 20 • 8 years ago

Comment on attachment 8805508 [details]
Bug 1312148 - report memory allocation while creating ImageBitmap;

@smaug and Morris,

As comment 18 mentioned, the original implementation tries to map DataSourceSurfaceD2D1's backend data into CPU memory which doesn't work if the surface is transferred crossing threads. Alos, mapping data out is not efficient at all. So, I updated the patch which now calculates the used memory without mapping data. :smaug is not accepting review request now and since the changes belong to graphics, only ask for Morris' review again.

@Morris, 
The issue we found test_bitmaprenderer.html won't be triggered anymore, however, we could still disable the test case through a separated following bug.

	Tzuhao Kuo [:kaku] Assignee
	Comment 21 • 8 years ago

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ae034bf1674c50e39e40beb0b208eeb8ce6c9042
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=873c8c9ae0bc13ea9f4416474db4992811a64721

	Tzuhao Kuo [:kaku] Assignee
	Comment 22 • 8 years ago

Thanks for the review!

	Pulsebot
	Comment 23 • 8 years ago

Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/edfe5715c0a3
report memory allocation while creating ImageBitmap; r=mtseng,smaug

	Carsten Book [:Tomcat]
	Comment 24 • 8 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/edfe5715c0a3