700514 - Tab Icon has a whole copy of uncompressed image/icon in heap-unclassified for each tab

Status: NEW

(Firefox :: General, defect)

Description

[Hugh Nougher [:Hughman]]

Attachment: html page with large favicon

When viewing an image directly in a tab, and it is small enough to create a favicon for the tab, an entire copy of the uncompressed image seems to be stored in heap-unclassified. It also happens when the image is set as the icon for an html page.

I have added a url to an image which creates a favicon which i quickly found through google which can be used for testing.

The following is steps to reproduce in
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0a1) Gecko/20111107 Firefox/10.0a1

Just about:memory open in new profile:
25.10 MB (100.0%) -- explicit
├───8.61 MB (34.31%) -- heap-unclassified
├───0.18 MB (00.72%) -- images

Open image directly in new tab (ie: paste uri of image into address bar)
33.21 MB (100.0%) -- explicit
├──11.53 MB (34.52%) -- heap-unclassified
├───3.78 MB (11.32%) -- images

For further visibility since a single tab is a little insignificant, Duplicate image tab (middle click on reload button) until there is 10 copies open.
69.82 MB (100.0%) -- explicit
├──43.44 MB (62.21%) -- heap-unclassified
├───3.78 MB (05.41%) -- images

For a proof when not viewing an image in a tab but just having a favicon for the page I have attached a simple html page using the same image as its icon. With 10 tabs of this page open i get the same measurements as the 10 tabs with the image in it.

Comment 1

[Hugh Nougher [:Hughman]]

If I disable hardware acceleration, heap-unclassified does not rise as much (only a few MB) but gfx-surface-win32 seems to take up most of the slack causing a similar amount of memory usage.

With HW Acceleration:
  3.11 MB -- gfx-d2d-surfacecache
 11.72 MB -- gfx-d2d-surfacevram
 33.19 MB -- gfx-surface-image
  0.00 MB -- gfx-surface-win32
(Interesting note: although some of this is in gpu, the 33MB is not and its about the same size as the extra usage in the heap-unclassified)

Without HW Acceleration:
  0.00 MB -- gfx-d2d-surfacecache
  0.00 MB -- gfx-d2d-surfacevram
  0.00 MB -- gfx-surface-image
 25.13 MB -- gfx-surface-win32

Comment 2

[Hugh Nougher [:Hughman]]

It seems that this is exactly the same as the problem I found in bug 699150 comment 31 except in that case it was a 240MB APNG. The difference is that the rise happens shortly after the entire animation has been played once. To test, simply use attachment 572433 [details] in place of my suggested jpeg.

As another note from observation, I think icon images are most of the explicit/images/content which is left over when all other images would have been discarded, which in my case is ~3MB. However that is the non-duplicated value so I may have upto 8MB of heap-unclassified used by these icons.

Comment 3

[Nicholas Nethercote [inactive]]

FWIW, I don't seem to be able to replicate these results on Linux -- the heap-unclassified isn't going up in any notable way.

Comment 4

[DB Cooper]

Opening the attachment URL 10x increased "images" in about:memory by ~ 4MB for me.

(Win7 64bit, 32bit nightly (latest) build, D2D/D3D10 on)

Comment 5

[Jeff Muizelaar [:jrmuizel]]

This will be easier when we switch to Azure.

Comment 6

[Justin Dolske [:Dolske]]

It sounds like there are 2 bugs here:

1) heap-unclassified memory use goes up, gfx/images should be handling the accounting

2) giant favicons use more memory than their 16x16 size would suggest.

Can we split one of these off to a separate bug (#1, might I suggest?).

Also, as a shorter (?) term solution than Azure, we could explicitly scale favicons down to 16x16 and use the resulting image as the favicon. Hmm, actually I thought that's what I did a long time ago back in bug 389273.

Also^2, is this an issue for ImageDocuments (loading giantimage.jpg in a tab)? Ah, that's exactly what comment 0 says. We've talked about possibly just not using a tiny scaled icon for those. I'd suspect that's _far_ more commonly encountered by users than sites with giant favicons. This would be a quick and easy fix, I think.

Comment 7

[Marco Bonardo [:mak]]

(In reply to Justin Dolske [:Dolske] from comment #6)
> Also, as a shorter (?) term solution than Azure, we could explicitly scale
> favicons down to 16x16 and use the resulting image as the favicon. Hmm,
> actually I thought that's what I did a long time ago back in bug 389273.

Actually, iirc mobile uses 32px favicons, and there is requirement to have larger favicons for new tab page and such UI features... Still I suspect scaling down to something between 64-256px may cover most needs.

Comment 8

[Radek 'sysKin' Czyz]

This is particularly problematic for animated gifs, because they have small dimensions (therefore, almost always have a favicon) but otherwise unbounded decoded size.