809587 - canvas.toBlob() doesn't increment the JS malloc counter

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Justin Lebar (not reading bugmail)]

There's a JS malloc counter which we should increment every time we do a big allocation; it affects how often the GC gets run.

Are we incrementing this counter when we take a blob screenshot (i.e., when we do canvas.toBlob)?  If not, we should be.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

They do not.

Comment 2

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 3

[Justin Lebar (not reading bugmail)]

Comment on attachment 679397 [details] [diff] [review]
Patch, v1

Heh, I didn't actually compile this

> +  if (context) {

if (cx)

Comment 4

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v2

Increment the malloc counter in one other place.  This covers all instances of nsDOMMemoryFile, which I guess is what we want?

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 679400 [details] [diff] [review]
Patch, v2

Review of attachment 679400 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/html/content/src/nsHTMLCanvasElement.cpp
@@ +682,5 @@
>  
> +  JSContext* cx = nsContentUtils::GetCurrentJSContext();
> +  if (cx) {
> +    JS_updateMallocCounter(cx, imgSize);
> +  }

Why are you doing this twice here?

Comment 6

[Justin Lebar (not reading bugmail)]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #5)
> Comment on attachment 679400 [details] [diff] [review]
> Patch, v2
> 
> Review of attachment 679400 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/html/content/src/nsHTMLCanvasElement.cpp
> @@ +682,5 @@
> >  
> > +  JSContext* cx = nsContentUtils::GetCurrentJSContext();
> > +  if (cx) {
> > +    JS_updateMallocCounter(cx, imgSize);
> > +  }
> 
> Why are you doing this twice here?

Because I was writing a code during a meeting.

Comment 7

[Justin Lebar (not reading bugmail)]

Attachment: Patch v3

Now with less less repetition.

Comment 8

[Justin Lebar (not reading bugmail)]

Soft basecamp blocker because this should help us purge screenshots out of the heap more quickly and because it's a low-risk change.

Comment 9

[Justin Lebar (not reading bugmail)]

Thanks, Kyle.

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/284ad66e8bda

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/284ad66e8bda

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bbdbb8b0cef8