1395908 - chrome://browser/skin/tabbrowser/loading-burst.svg consume much memory than other UI images

Status: NEW

(Firefox :: Tabbed Browser, defect, P4)

Description

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: memory-report.json.gz

## ENVIRONMENT

- https://hg.mozilla.org/mozilla-central/rev/04b6be50a2526c7a26a63715f441c47e1aa1f9be
- Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:57.0) Gecko/20100101 Firefox/57.0

## STEPS TO REPRODUCE


1. open https://www.google.com
2. open about:memory
3. Click "measure"

## ACTUAL RESULT

I see chrome://browser/skin/tabbrowser/loading-burst.svg consumes more memory than other svg files (see the below or attachmented "memory-report.json.gz" file). 

I feel this count is too large for the single UI image compared with others.


-------------------

│   │  │  ├───7.94 MB (02.68%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)
│   │  │  │   ├──7.91 MB (02.67%) -- locked/surface(1440x1440)
│   │  │  │   │  ├──7.91 MB (02.67%) ── decoded-nonheap
│   │  │  │   │  └──0.00 MB (00.00%) ── decoded-heap
│   │  │  │   └──0.03 MB (00.01%) ── source
│   │  │  └───3.82 MB (01.29%) -- (29 tiny)
│   │  │      ├──1.06 MB (00.36%) ++ image(468x20, chrome://browser/skin/stop-to-reload.svg)
│   │  │      ├──1.06 MB (00.36%) ++ image(468x20, chrome://browser/skin/reload-to-stop.svg)
│   │  │      ├──0.52 MB (00.18%) ++ image(960x16, chrome://browser/skin/tabbrowser/loading.svg)
│   │  │      ├──0.11 MB (00.04%) ++ image(16x16, chrome://mozapps/skin/places/defaultFavicon.svg)
│   │  │      ├──0.08 MB (00.03%) ++ image(0x0, chrome://global/skin/icons/autocomplete-search.svg)
│   │  │      ├──0.06 MB (00.02%) ── image(16x16, chrome://browser/skin/tracking-protection-16.svg#enabled)/source
│   │  │      ├──0.05 MB (00.02%) ── image(16x16, chrome://browser/skin/downloads/download-icons.svg#arrow-with-bar)/source
│   │  │      ├──0.05 MB (00.02%) ── image(0x0, chrome://browser/skin/fxa/sync-illustration.svg)/source
│   │  │      ├──0.05 MB (00.02%) ++ image(20x20, chrome://global/skin/icons/close.svg)
│   │  │      ├──0.04 MB (00.02%) ++ image(16x16, chrome://browser/skin/back.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/identity-icon.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/reload.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/stop.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/gear.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/sidebars.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://global/skin/icons/arrow-dropdown-16.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/forget.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/bookmark.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/urlbar-tab.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/bookmark-hollow.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/library.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/chevron.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/developer.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/menu.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/page-action.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/forward.svg)
│   │  │      ├──0.04 MB (00.01%) ++ image(16x16, chrome://browser/skin/tabbrowser/newtab.svg)
│   │  │      ├──0.03 MB (00.01%) ── image(16x16, chrome://browser/skin/home.svg)/source
│   │  │      └──0.03 MB (00.01%) ── image(16x16, chrome://browser/skin/arrow-left.svg)/source

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

loading-burst.svg is very simple:

<!-- This Source Code Form is subject to the terms of the Mozilla Public
   - License, v. 2.0. If a copy of the MPL was not distributed with this
   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
<svg viewBox="0 0 10 10" xmlns="http://www.w3.org/2000/svg">
  <circle cx="5" cy="5" r="5" fill="context-fill"/>
</svg>


The animation for this does a `transform: scale(40);` and `opacity: 0;`.

Could the large transform be the cause of this higher memory?

Comment 2

[Sam Foster [:sfoster] (he/him)]

Maybe explicit width/height attributes on the SVG element would help here? The viewBox should allow it to scale to whatever size is needed when used.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> Could the large transform be the cause of this higher memory?

Yeah, it might. jwatt or dholbert might need to confirm, but I suspect that the layer is rendered and held at the highest scaling to avoid pixelation.

Comment 4

[Jonathan Watt [:jwatt]]

(In reply to Mike Conley (:mconley) (:⚙️) from comment #3)
> Yeah, it might. jwatt or dholbert might need to confirm, but I suspect that
> the layer is rendered and held at the highest scaling to avoid pixelation.

Right, exactly. To make sure that the rendering at the extremes of the transform animation is acceptable, we render at the max scale. That's why about:memory shows the surface size is 1440 x 1440, and we have 4 bytes per pixel.

Comment 5

[Jonathan Watt [:jwatt]]

(In reply to Sam Foster [:sfoster] from comment #2)
> Maybe explicit width/height attributes on the SVG element would help here?
> The viewBox should allow it to scale to whatever size is needed when used.

That won't help, I'm afraid. The dimensions of the surface that we draw the SVG to will be unchanged, as you'd expect if the dimensions of that surface is to be consistent with the pixel density of the target surface that it will be drawn into at the full 40x scale.

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Can we at least do anything to release the memory after the animation has completed?

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

ni for comment #6 ^

Comment 8

[:Gijs (he/him)]

(In reply to Jonathan Watt [:jwatt] from comment #4)
> (In reply to Mike Conley (:mconley) (:⚙️) from comment #3)
> > Yeah, it might. jwatt or dholbert might need to confirm, but I suspect that
> > the layer is rendered and held at the highest scaling to avoid pixelation.
> 
> Right, exactly. To make sure that the rendering at the extremes of the
> transform animation is acceptable, we render at the max scale. That's why
> about:memory shows the surface size is 1440 x 1440, and we have 4 bytes per
> pixel.

Hm, but tabs will never be 1440 x 1440, right? Even on a hidpi screen, I wouldn't expect the tabs to be more than about 250px wide and thus you'd need 500 or maaaaybe on some super-hidpi screens 750 or 1000 device pixels. On top of that, normally the tab won't be more than 40px tall, maybe up to 100 for huge system fonts.

Can we get a similar effect of the burst "dissipating" with a lower scaling than 40x? Seems like that would quadratically reduce the amount of memory used. If we could bound the vertical size of the surface separately somehow that would improve things still further.

Comment 9

[Jonathan Watt [:jwatt]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> Can we at least do anything to release the memory after the animation has
> completed?

Since we'll be using this scaled image every time a tab loads, it doesn't seem to make much sense to release the surface, I think.

Besides that, feeding information back from CSS through the layers system to imagelib whenever a large animation is removed from an element containing images would be highly non-trivial.

Comment 10

[Jonathan Watt [:jwatt]]

(In reply to :Gijs from comment #8)
> Hm, but tabs will never be 1440 x 1440, right? Even on a hidpi screen, I
> wouldn't expect the tabs to be more than about 250px wide and thus you'd
> need 500 or maaaaybe on some super-hidpi screens 750 or 1000 device pixels.
> On top of that, normally the tab won't be more than 40px tall, maybe up to
> 100 for huge system fonts.

Sure, but the code that decides on layer scaling factors or, more particularly, the code that decides imagelib surface sizes based on that scale, doesn't know anything about the clip areas that will be applied by ancestor elements (the tab in this case). (Nor would it make sense for it to do so since that could change and if the image surface was clipped it would have to be re-decoded.) That code simply knows that a 40x scale is to be applied at some point, and it will need 40x the surface size to be able to display without bad anti aliasing.

> Can we get a similar effect of the burst "dissipating" with a lower scaling
> than 40x? Seems like that would quadratically reduce the amount of memory
> used. If we could bound the vertical size of the surface separately somehow
> that would improve things still further.

It seems that 40x, as you say, will create a circle that extends far outside the bounds of the tab. One thing you could do in the Firefox code would be to figure out what scale factor is actually needed, and use that instead. Of course you may then need two separate animations (one for the scale, one for the opacity) if you don't want the opacity to reach zero at the same time as the edge of the circle reaches the far side of the tab.

Note also that the scale() CSS transforms scale() function can be given both X and Y scales. It looks like nsLayoutUtils::ComputeSuitableScaleForAnimation takes those into account individually, so it seems you could restrict the Y scale somewhat more than the X scale and it probably wouldn't be visually noticeable (given the speed of the burst).

I (and this may be just my lack of an eye for these details) hadn't actually noticed that the bust effect was an expanding circle. I had thought it was more like a rapidly expanding progress bar-like thing, maybe with a curved leading edge. (It happens so quickly.) Perhaps it could be implemented that way if that is visually acceptable and other tweaks don't improve things enough?

Other than that maybe we could implement a CSS property that indicates "I don't care about the resolution of that this element is drawn at when an animation scales it - it will happen fast enough that it can be fuzzy".

Comment 11

[Jonathan Watt [:jwatt]]

For my own reference as much as anything else, the scaling is initiated here:

http://searchfox.org/mozilla-central/rev/67f38de2443e6b613d874fcf4d2cd1f2fc3d5e97/layout/painting/FrameLayerBuilder.cpp#5477

Comment 12

[Jared Wein [:jaws] (please needinfo? me)]

Is this only reproducible on OSX? I've tried measuring three different times, including while a page is loading.

On Windows I get the following on about:memory,
53,680 B (00.02%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)

On Ubuntu I get the following,
53,672 B (00.02%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

From Windows,
│    │  │  └─────53,680 B (00.02%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)
│    │  │        ├──53,600 B (00.02%) ── source
│    │  │        └──────80 B (00.00%) ── locked/cannot_substitute/surface(1440x1440)/decoded-heap

From Ubuntu,
│    │  │  └─────53,672 B (00.02%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)
│    │  │        ├──53,592 B (00.02%) ── source
│    │  │        └──────80 B (00.00%) ── locked/cannot_substitute/surface(1080x1080)/decoded-heap

Comment 14

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #12)
> Is this only reproducible on OSX? I've tried measuring three different
> times, including while a page is loading.

I tested again in my envs:

From macOS v10.12.6 on MacBook Pro 15 (HiDPI) with https://hg.mozilla.org/mozilla-central/rev/d8e238b811d3dc74515065ae8cab6c74baf0295f:
```
│   │  │  │  ├───7.96 MB (03.35%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)
│   │  │  │  │   ├──7.91 MB (03.33%) -- locked/cannot_substitute/surface(1440x1440)
│   │  │  │  │   │  ├──7.91 MB (03.33%) ── decoded-nonheap
│   │  │  │  │   │  └──0.00 MB (00.00%) ── decoded-heap
│   │  │  │  │   └──0.05 MB (00.02%) ── source
```


From macOS v10.13 beta on MacBook Air (not-HiDPI) with: 
```
│    │  │  └──2.03 MB (01.22%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)
│    │  │     ├──1.98 MB (01.19%) -- locked/cannot_substitute/surface(720x720)
│    │  │     │  ├──1.98 MB (01.19%) ── decoded-nonheap
│    │  │     │  └──0.00 MB (00.00%) ── decoded-heap
│    │  │     └──0.05 MB (00.03%) ── source
```


From Windows 10 ver.1703 (build15063.540) on Dell XPS 13 (HiDPI) with:
```
│    │  │  └──0.05 MB (00.04%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)
│    │  │     ├──0.05 MB (00.04%) ── source
│    │  │     └──0.00 MB (00.00%) ── locked/cannot_substitute/surface(1440x720)/decoded-heap
```

Sorry. I don't have a suitable Linux Desktop for this testing.

By these results, I also seem that this is macOS specific problem (especially for HiDPI).

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

jwatt, given comment 12 - 14, it seems that this is actually a macOS-specific bug. Can you take a look in to this?

Comment 16

[Jonathan Watt [:jwatt]]

(In reply to (Away until Sept 13th) Jared Wein [:jaws] (please needinfo? me) from comment #13)
> From Windows,
> │    │  │  └─────53,680 B (00.02%) -- image(0x0,
> chrome://browser/skin/tabbrowser/loading-burst.svg)
> │    │  │        ├──53,600 B (00.02%) ── source
> │    │  │        └──────80 B (00.00%) ── locked/cannot_substitute/surface(1440x1440)/decoded-heap

A 1440x1440 decoded image cannot be stored in 80 B of memory. about:memory must be misreporting on Windows. (I would assume that that memory is reported under another section of about:memory.)

> From Ubuntu,
> │    │  │  └─────53,672 B (00.02%) -- image(0x0,
> chrome://browser/skin/tabbrowser/loading-burst.svg)
> │    │  │        ├──53,592 B (00.02%) ── source
> │    │  │        └──────80 B (00.00%) ── locked/cannot_substitute/surface(1080x1080)/decoded-heap

Again, A 1080x1080 decoded image cannot be stored in 80 B of memory.

Comment 17

[Jonathan Watt [:jwatt]]

needinfo'ing tn and aosmond to see if they have any insights into this memory reporting.

Comment 18

[Andrew Osmond [:aosmond] (he/him)]

(In reply to Jonathan Watt [:jwatt] from comment #17)
> needinfo'ing tn and aosmond to see if they have any insights into this
> memory reporting.

Hmm, I suspect imgFrame::Optimize [1] is getting in the way. Can you reproduce with the environment variable MOZ_DISABLE_IMAGE_OPTIMIZE=1 set and see what the memory reporting says? Reviewing the code, we only appear to support giving the true result if we didn't optimize the surface [2], and I hazard that's what is misrepresenting the memory usage.

[1] http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/image/imgFrame.cpp#428
[2] http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/image/imgFrame.cpp#954

Comment 19

[Timothy Nikkel (:tnikkel)]

I'd also guess it's a mistake in reporting the memory, so probably a separate issue from the main issue of this bug.

Comment 20

[Marco Bonardo [:mak]]

This is my measurement on Windows 10, it's the largest memory usage, maybe measurement has been fixed in the meanwhile.

├────8.97 MB (03.86%) -- images
│    ├──7.38 MB (03.17%) -- chrome
│    │  ├──7.37 MB (03.17%) -- vector/used
│    │  │  ├──4.40 MB (01.89%) -- image(0x0, chrome://browser/skin/tabbrowser/loading-burst.svg)
│    │  │  │  ├──3.06 MB (01.32%) -- locked/cannot_substitute/surface(896x896, svgContext:[ viewport=(11x11) contextPaint=( fill=ffff840a fillOpa=1 strokeOpa=1 ) ])
│    │  │  │  │  ├──3.06 MB (01.32%) ── decoded-nonheap
│    │  │  │  │  └──0.00 MB (00.00%) ── decoded-heap
│    │  │  │  └──1.34 MB (00.57%) ++ (2 tiny)
│    │  │  └──2.97 MB (01.28%) -- (38 tiny)
│    │  │     ├──0.60 MB (00.26%) ── image(660x33, chrome://browser/skin/bookmark-animation.svg)/source
│    │  │     ├──0.43 MB (00.18%) ++ image(960x16, chrome://browser/skin/tabbrowser/loading.svg)