Closed Bug 686345 Opened 10 years ago Closed 9 years ago
Negative value for canvas-2d-pixel-bytes in about:memory
Other Measurements -7,859,276 B -- canvas-2d-pixel-bytes 0 B -- gfx-d2d-surfacecache 0 B -- gfx-d2d-surfacevram 5,424 B -- gfx-surface-image 37,351,417 B -- gfx-surface-win32 434,671,722 B -- heap-allocated 552,775,680 B -- heap-committed 2,793,472 B -- heap-dirty 412,568,708 B -- heap-unallocated 43 -- js-compartments-system 321 -- js-compartments-user 250,609,664 B -- js-gc-heap 39,413,696 B -- js-gc-heap-arena-unused 0 B -- js-gc-heap-chunk-clean-unused 82,848,516 B -- js-gc-heap-chunk-dirty-unused 48.78% -- js-gc-heap-unused-fraction 951,693,312 B -- private 854,806,528 B -- resident 970,752 B -- shmem-allocated 970,752 B -- shmem-mapped 1,394,135,040 B -- vsize Note the negative value for canvas-2d-pixel-bytes. According to the explanation in the tooltip it's (width * height * 4) bytes, so how can it be negative? Steps to reproduce ? (*) 1) Open deviantart.com 2) Open a lot of images in new tabs 3) Close about half of the tabs again (*) I didn't try to reproduce yet, but this is what I was doing when I noticed the bug.
Mozilla/5.0 (X11; Linux i686; rv:9.0a1) Gecko/20110913 Firefox/9.0a1 Confirming. I also see a negative value for canvas-2d-pixel-bytes. I think, but I can't be sure, that the value wasn't negative until I clicked "Minimize memory usage". I'll attach the verbose output. Bug 572274 comment 5 also mentions this.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
This is the verbose output of about:memory showing the negative value for canvas-2d-pixel-bytes. There are also a couple of zombie compartments, but I'll look into that separately. This is with no other tabs open and after clicking "Minimize memory usage", leaving it for a few minutes, and clicking it again. I think that before I clicked it, the value wasn't negative and it was only when I clicked it that I noticed it change, but I can't be sure. If there's anything else suspicious in there then let me know and I can file a new bug. This was with my usual profile which has a few extensions. I haven't been able to reproduce it with this profile or a clean one.
Could you please attach a concrete STR? I've tried "open a bunch of deviantart images", but the ones I'm selecting apparently aren't creating canvas objects.
Well, like I said I didn't try to reproduce and trying now I can't reproduce the issue with these steps. Probably it was coincidence that I noticed the issue while being on deviantart and the site is not related to the issue. Though the reason I opened about:memory was that I tried to investigate an issue with compartments staying around for too long. I therefore clicked on the GC and CC buttons. I didn't really pay attention if the value was negative before or only after clicking the GC and CC buttons. Comment 2 mentions "Minimize memory usage" which triggers GC and CC as well, so maybe this issue is related to GC and/or CC...
(In response to comment 1) > Confirming. Daniel, do you have a STR for this?
Oh, apparently just loading a random canvas page (e.g. ) and closing it was enough to trigger this bug for me.  http://aerotwist.com/lab/creating-particles-with-three-js/demo/
Assignee: nobody → justin.lebar+bug
The site in comment 6 doesn't actually trigger this -- it's a webgl page. I must have gone somewhere else right beforehand which triggered it, but I haven't been able to reproduce. In any case, the change here makes the code obviously correct. I think. :)
I found steps to reproduce the bug reliably in a clean profile: Open http://www.atopon.org/dissolve/dissolve.html in a new tab => 3.17 MB -- canvas-2d-pixel-bytes Close the tab and wait a few seconds => 0.90 MB -- canvas-2d-pixel-bytes Undo Close Tab => 2.27 MB -- canvas-2d-pixel-bytes Close the tab again and wait a few seconds => -1.80 MB -- canvas-2d-pixel-bytes You can repeat steps 3 and 4 to get an even bigger negative value.
(In reply to Justin Lebar [:jlebar] from comment #5) > (In response to comment 1) > > Confirming. > > Daniel, do you have a STR for this? I just saw this bug in the middle of a browsing session, opened about:memory and went "oh yeah, same here". Comment 9 works reliably for me though.
(In reply to Andreas Jung from comment #9) > I found steps to reproduce the bug reliably in a clean profile: Thank you, Andreas! Just tested with my patch, and it works properly.
Comment on attachment 560509 [details] [diff] [review] Patch v1 Can you give a description of what this patch is doing?
So there's a global count of how much memory is being used by canvases. Previously, nsCanvasRenderingContext2D::Reset() would subtract out this canvas's size from the global count if |mValid && !mDocShell|. But this was wrong somehow in an edge case; either we subtracted outselves twice, or once subtracted without adding ourselves first. Rather than try to figure this out and potentially miss another edge case, I just added a member to the canvas object so it remembers if it's added itself to the global count.
(In reply to Justin Lebar [:jlebar] from comment #13) > So there's a global count of how much memory is being used by canvases. > > Previously, nsCanvasRenderingContext2D::Reset() would subtract out this > canvas's size from the global count if |mValid && !mDocShell|. > > But this was wrong somehow in an edge case; either we subtracted outselves > twice, or once subtracted without adding ourselves first. I'd like to know more about this edge case. Can we simplify the code so that the correspondence between when we increment and decrement is obvious?
Attachment #560509 - Flags: review?(jmuizelaar) → review-
This is a great example of why I prefer "crawl over a data structure" memory reporters to "maintain a global counter" memory reporters :) Can we make this memory reporter a crawler?
Hm, the current code is pretty broken. When I load  in a instrumented build, I get this=0x4710010 - gCanvasMemoryUsed -= 180000, new value -180000 this=0x4710010 - gCanvasMemoryUsed -= 805200, new value -985200 this=0x4710010 - gCanvasMemoryUsed += 4310504, new value 3325304 That is, we start subtracting for a canvas before we ever add anything!  http://www.atopon.org/dissolve/dissolve.html
So the problem seems to be that we're calling nsCanvasRenderingContext2D::Reset() before calling nsCanvasRenderingContext2D::EnsureSurface(). I'll post a patch which seems to fix the reporting in a moment...
Comment on attachment 563116 [details] [diff] [review] Patch v2 Actually, I just tested on my Windows 7 machine, and Azure works properly.
Comment on attachment 563116 [details] [diff] [review] Patch v2 It seems like 'if (mSurface)' should be a sufficient test here no?
I don't think so; I think someone can call InitializeWithSurface and pass a surface in without incrementing gCanvasMemoryUsed. mSurface && !mDocShell might be sufficient, but checking mValid seems semantically right. Honestly, it feels pretty fragile to do it this way rather than to explicitly remember whether we incremented gCanvasMemoryUsed, precisely because figuring out the right condition is nontrivial.
Attachment #560509 - Attachment is obsolete: true
Attachment #563116 - Flags: review?(jmuizelaar) → review+
Whiteboard: [MemShrink:P3] → [MemShrink:P3][inbound]
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink:P3][inbound] → [MemShrink:P3]
Target Milestone: --- → mozilla10
I wonder whether the bug should be re-opened: After my Firefox 20.0 had used 1.3GB of virtual memory, I had a look in about:memory, and I found: -311,966,480 B ── canvas-2d-pixel-bytes [?!]
You need to log in before you can comment on or make changes to this bug.