Increase in memory utilization on Mac OSX 10.7+ due to history swipe animations

[Stephen Pohl]

Hi,

I wanted to give a heads up that we're in the process of finalizing the
patch for bug 678392 which will give us history swipe animations on Mac
OSX 10.7+. Since we will be taking snapshots of the 20 most-recently
visited pages, this will undoubtedly lead to an increase in memory
utilization on these platforms.

There is a discussion about experimental memory measurements in comment
305 and 309 in the bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c305
https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c309

We attempted to land the patch late last week but ran into test failures
caused (most-likely) by not forcing a clobber. The upside of it is that
it gave us feedback from Talos and an opportunity to email the list for
feedback before making another attempt at landing the patch. The "Trace
Malloc MaxHeap" regression is documented in this graph:
http://graphs.mozilla.org/graph.html#tests=[[29,63,22]]&sel=1360274365000,1360447165000&displayrange=7&datatype=running

We'd like to make sure that this increase is acceptable before landing
the patch. If you have any feedback, please let me know.

Thanks,
Stephen

[Ed Morley]

On 12 February 2013 22:11:12, Stephen Pohl wrote:
> I wanted to give a heads up that we're in the process of finalizing
> the patch for bug 678392 which will give us history swipe animations
> on Mac OSX 10.7+. Since we will be taking snapshots of the 20
> most-recently visited pages, this will undoubtedly lead to an increase
> in memory utilization on these platforms.

To save everyone having to look at the graph - the initial landing
showed a consistent 20% regression in trace malloc maxheap. If this
were a 1-5% regression, then I think it would be worth discussing the
trade-off. At 20%, I really don't see how we can take this, sorry! :-(

Ed

[Jet Villegas]

Stephen:

We talked about extracting this snapshot from the GFx Layers. Have you explored that possibility? Taking the screencap from the raster data we already paint should help here. BTW, 20 seems to be a very high number to cache per tab. The biggest problem I'd like to solve is that I often accidentally swipe back, and that's almost always just 1 page back.

--Jet

----- Original Message -----
From: "Stephen Pohl" <sp...@mozilla.com>
To: dev-pl...@lists.mozilla.org
Sent: Tuesday, February 12, 2013 2:11:12 PM
Subject: Increase in memory utilization on Mac OSX 10.7+ due to history swipe animations

Hi,

I wanted to give a heads up that we're in the process of finalizing the
patch for bug 678392 which will give us history swipe animations on Mac
OSX 10.7+. Since we will be taking snapshots of the 20 most-recently
visited pages, this will undoubtedly lead to an increase in memory
utilization on these platforms.

There is a discussion about experimental memory measurements in comment
305 and 309 in the bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c305
https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c309

We attempted to land the patch late last week but ran into test failures
caused (most-likely) by not forcing a clobber. The upside of it is that
it gave us feedback from Talos and an opportunity to email the list for
feedback before making another attempt at landing the patch. The "Trace
Malloc MaxHeap" regression is documented in this graph:
http://graphs.mozilla.org/graph.html#tests=[[29,63,22]]&sel=1360274365000,1360447165000&displayrange=7&datatype=running

We'd like to make sure that this increase is acceptable before landing
the patch. If you have any feedback, please let me know.

Thanks,
Stephen

_______________________________________________
dev-platform mailing list
dev-pl...@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

[Justin Lebar]

> To save everyone having to look at the graph - the initial landing showed a
> consistent 20% regression in trace malloc maxheap. If this were a 1-5%
> regression, then I think it would be worth discussing the trade-off. At 20%,
> I really don't see how we can take this, sorry! :-(

I hope it's not adding noise to say, "what Ed said."

-Justin

[Stephen Pohl]

Hi Jet,

Extracting the snapshot from the GFx layer has not been explored yet. I
think we discussed this in the context of the time it takes to create
the snapshots. Since this will be handled off the main thread (bug
817700) we expect a pretty good performance there. I believe we would
still incur the memory cost however to store the snapshots. Do I recall
this conversation correctly?

The limit of 20 snapshots is currently per window (not per tab). In bug
839549, I'd like to switch to a WeakMap to store the snapshots and make
the limit of 20 global to all windows. I'm not very familiar with the
Trace Malloc MaxHeap tests, but if they use lots of windows, it may
explain a jump of 20% in memory utilization and I may have to address
bug 839549 sooner than later.

I hope this clarifies things.

-Stephen

[Nicholas Nethercote]

On Tue, Feb 12, 2013 at 3:29 PM, Jet Villegas <j...@mozilla.com> wrote:
>
> BTW, 20 seems to be a very high number to cache per tab. The biggest problem I'd like to solve is that I often accidentally swipe back, and that's almost always just 1 page back.

That was my first thought, too.

Nick

[Robert O'Callahan]

Can we compress these screenshots to JPEG or something?

Rob
--
Wrfhf pnyyrq gurz gbtrgure naq fnvq, “Lbh xabj gung gur ehyref bs gur
Tragvyrf ybeq vg bire gurz, naq gurve uvtu bssvpvnyf rkrepvfr nhgubevgl
bire gurz. Abg fb jvgu lbh. Vafgrnq, jubrire jnagf gb orpbzr terng nzbat
lbh zhfg or lbhe freinag, naq jubrire jnagf gb or svefg zhfg or lbhe fynir
— whfg nf gur Fba bs Zna qvq abg pbzr gb or freirq, ohg gb freir, naq gb
tvir uvf yvsr nf n enafbz sbe znal.” [Znggurj 20:25-28]

[Stephen Pohl]

We use <canvas>.toBlob() to compress the snapshots to PNG. I ran a few
experiments with JPG, but the results seemed to even out between PNG and
JPG (with 100% quality). Choosing a lower quality for JPG compression
quickly started to look bad, especially on retina displays. Comment 176
had a few details around this:
https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c176

-Stephen

-------- Original Message --------
Subject: Re: Increase in memory utilization on Mac OSX 10.7+ due to
history swipe animations
From: Robert O'Callahan <rob...@ocallahan.org>
To: Stephen Pohl <sp...@mozilla.com>
Cc: dev-pl...@lists.mozilla.org

[Asa Dotzler]

I don't see how we can *not* take this. Of course it's going to mean
using more memory. If it doesn't leak, and it doesn't put us over some
magic limit where a significant portion of our users end up paging or
something like that, then I don't see how we can reject it.

Without context, 1-5% or 20% growth are just meaningless numbers. The
context here is not some accidental regression or a feature doing
something horribly wrong with memory. This is simply a memory-expensive
feature and it's a feature we *must* land.

I'm all for smart people looking for ways to get this memory usage as
low as it can be without undermining the value of the feature, but if we
cannot find those wins, we should land this as it is.


- A

[L. David Baron]

On Tuesday 2013-02-12 18:40 -0800, Asa Dotzler wrote:
> doing something horribly wrong with memory. This is simply a
> memory-expensive feature and it's a feature we *must* land.

Why is it simply a memory-expensive feature? Why does it require
any additional memory overhead at all, other than while an animation
is happening?

-David

--
𝄞 L. David Baron http://dbaron.org/ 𝄂
𝄢 Mozilla http://www.mozilla.org/ 𝄂

[Andreas Gal]

Hey Asa,

where does the magic 20 pages deep history number come from? Why not 1? Or 999?

Andreas

[Stephen Pohl]

L. David Baron wrote:
> On Tuesday 2013-02-12 18:40 -0800, Asa Dotzler wrote:

>> doing something horribly wrong with memory. This is simply a
>> memory-expensive feature and it's a feature we *must* land.

> Why is it simply a memory-expensive feature? Why does it require
> any additional memory overhead at all, other than while an animation
> is happening?
>
> -David
>

Currently, when navigating back/forward in the browser history, we don't
keep track of the rendered pages. So, without storing snapshots of pages
in history, we only have the current page to animate. The previous/next
pages would appear blank.

[Stephen Pohl]

There is some context to the magic number 20 in comment 305 in the bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c305

In summary, it seems to match what other browsers are doing and the
memory consumption during experiments remained below 10mb.

-Stephen

Andreas Gal wrote:
> Hey Asa,
>
> where does the magic 20 pages deep history number come from? Why not 1? Or 999?
>
> Andreas
>
> On Feb 12, 2013, at 9:40 PM, Asa Dotzler <a...@mozilla.com> wrote:
>

[L. David Baron]

On Tuesday 2013-02-12 20:17 -0800, Stephen Pohl wrote:
> L. David Baron wrote:
> >On Tuesday 2013-02-12 18:40 -0800, Asa Dotzler wrote:

> >>doing something horribly wrong with memory. This is simply a
> >>memory-expensive feature and it's a feature we *must* land.

> >Why is it simply a memory-expensive feature? Why does it require
> >any additional memory overhead at all, other than while an animation
> >is happening?

> Currently, when navigating back/forward in the browser history, we
> don't keep track of the rendered pages. So, without storing
> snapshots of pages in history, we only have the current page to
> animate. The previous/next pages would appear blank.

But we store the pages themselves in the bfcache, and it seems like
we ought to be able to paint them into the needed image (or other
lower-level graphical buffer) right before we do the animation.

[Asa Dotzler]

On 2/12/2013 8:05 PM, Andreas Gal wrote:
> Hey Asa,
>
> where does the magic 20 pages deep history number come from? Why not 1? Or 999?
>
> Andreas

The goal of the feature is to work when ever the user engages it and not
just "for the first couple of pages".

Just have one and too many users would fall off of their back stack very
quickly. Nine hundred ninety nine and we'd be wasting memory for all but
a very small minority of users who accumulate and navigate through that
much history.

We have Test Pilot data we could use to fine tune this, maybe we only
need 15, or maybe we actually need 25.

My point isn't to quarrel about the depth of that back stack, but to say
that it is not OK to simply dismiss a new feature because it increases
memory footprint. Features vs footprint is a balancing act. Both sides
must be weighed and that wasn't what I saw happening here. What I saw
happening was the out of hand dismissal of a feature based on no
consideration other than increased memory footprint. That cannot be how
we roll.

- A

[Andreas Gal]

I think people are legitimately concerned about the memory use of this feature. I don't think anyone is trying to dismiss anything here. I still don't fully understand why we need a full size screenshot of the last N pages. Is the last page sufficient and we redraw the rest while we animate? I am sure you guys considered this, so I am curious why this was excluded.

Thanks,

Andreas

[Justin Lebar]

Now that I've looked more closely, I take back my earlier "What Ed
said." The issue is more nuanced than I originally thought.

> To save everyone having to look at the graph - the initial landing showed a
> consistent 20% regression in trace malloc maxheap. If this were a 1-5%
> regression, then I think it would be worth discussing the trade-off. At 20%,
> I really don't see how we can take this, sorry! :-(

As I read the graph, the 20% max-heap regression corresponds to 10mb,
which I understand is the expected memory-usage increase here. So it
looks like this feature is behaving as intended.

To be clear, this regression isn't a 20% regression of memory usage
overall; it's pushing the maximum amount of memory allocated on the
C++ heap up by 10mb, which corresponds to a 20% regression of maximum
heap size in our particular test.

I'd like to put the 10mb in perspective based on our telemetry data.

Unfortunately we don't have good RSS telemetry numbers on Mac (a
direct effect of fixing bug 789975, with no known work-around), so I
can't say precisely how much memory the average Mac user uses. I also
don't have a CDF in the telemetry front-end (bug 723604), so I can't
interpret the data I do have except by eyeballing it.

But very roughly I'd guess that the bottom 10% of users are somewhere
around 300mb of memory used. A 10mb regression represents a ~1.05x
increase in memory usage for these users, which doesn't seem like the
end of the world to me.

Still my preference would be to be conservative and decrease this down
to 5 or 10 screenshots (2.5 - 5mb), which I'd bet most of us could
agree is acceptable in terms of memory usage. I'd prefer going down
to 5 or 10 screenshots largely because of the precedent it sets:
Multiply 10mb of memory times five must-have features and you're
talking serious money.

After configuring this feature to keep fewer screenshots, we could do
the following in parallel:

a) Collect data to determine how many screenshots we "need to" keep
around, as Asa suggested. Offhand, I'd prefer that we used telemetry
instead of test pilot for this, since presumably the usage of this
feature will change over time, as people discover it.

b) Try to do clever things such as re-rendering bfcached pages, as
Andreas suggested.

c) Consider adaptive techniques so that users who use this feature
heavily will store more screenshots (at the cost of more memory),
while those who don't use it won't pay a price.

I prefer this "start small" approach of limiting us to 5 or 10
screenshots for four reasons.

0) It lets us land this feature now, rather than after we've tried to
reduce its footprint by being clever. Perfect is the enemy of the
good and all that.

1) It minimizes the impact on users, particularly those who don't use
the feature.

2) It gives the team in charge of this feature an incentive to do
(a)-(c) above, all of which would be beneficial even if we were
storing 50 screenshots.

3) Most importantly, it sets a precedent that we will configure
features to use as little memory as feasible until we have data
demonstrating that additional memory would be beneficial, and that if
you want to use more than the bare minimum necessary, you need to be
clever and avoid making users who don't use the feature pay.

-Justin

On Tue, Feb 12, 2013 at 6:08 PM, Ed Morley <emo...@mozilla.com> wrote:
> On 12 February 2013 22:11:12, Stephen Pohl wrote:
>>
>> I wanted to give a heads up that we're in the process of finalizing
>> the patch for bug 678392 which will give us history swipe animations
>> on Mac OSX 10.7+. Since we will be taking snapshots of the 20
>> most-recently visited pages, this will undoubtedly lead to an increase
>> in memory utilization on these platforms.
>
>
> To save everyone having to look at the graph - the initial landing showed a
> consistent 20% regression in trace malloc maxheap. If this were a 1-5%
> regression, then I think it would be worth discussing the trade-off. At 20%,
> I really don't see how we can take this, sorry! :-(
>
> Ed
>

[Benjamin Smedberg]

On 2/13/2013 3:12 AM, Justin Lebar wrote:
> c) Consider adaptive techniques so that users who use this feature
> heavily will store more screenshots (at the cost of more memory),
> while those who don't use it won't pay a price.

Apart from the other solution of re-rendering directly from bfcache,
this seems like the obvious longer-term solution, and it ties in with
Josh's proposal about "ddd". I don't think there's any way to make
absolute judgements about whether 2.5MB or 5MB or 10MB is "reasonable"
for a feature, and there's a lot of room for adaptive data-driven
behavior here. If the computer is memory-constrained, then we should be
evicting all sorts of memory-based caches pretty aggressively, and this
is one of them. OTOH, if the computer has memory available, we should be
using *all* of it any place we can trade memory for speed.

Other possible technical tradeoffs that might be possible:

* render from bfcache if its present
* when we evict from bfcache, take a high-res snapshot
* when things go back far enough, compress or resample the image to a
low-res snapshot.

Is there a video/demo/feature page which explains what this feature
actually is? I've seen status updates about a new cool swipe thing, but
I don't actually know what problem its solving. Is this feature
mac-only, or is it starting out on mac and we'll be porting it back to
other platforms? Or is it touchscreen-only?

--BDS

[Justin Lebar]

This is kind of OT, but

> OTOH, if the computer has memory available, we should be using *all* of it any place we can trade
> memory for speed.

We considered this idea in MemShrink some months ago, and we mostly
dropped it. There are two essential problems that we weren't able to
overcome.

1) It's not always simple to determine how much memory the system has
available. This is particularly true on Mac, as it turns out; bug
789975 says we can't even accurately measure our own memory usage on
Mac.

The issue is that the OS keeps some types of free'd pages in a
process's address space and only releases them when the system is very
low on memory, but it doesn't give us any way (that I'm aware of) to
measure how many of these pages there are.

2) More importantly, using as much RAM is available but no more makes
Firefox the nicest process on the system. We can get into a situation
where, because some other program is hogging memory, the user's swipe
animations stop working as she expects. That may look like a bug in
Firefox, and in any case it may not be what the user wants.

Doing this makes Firefox change its behavior based upon what other
processes on the system are doing, and I'm not convinced that's a safe
thing to do in general.

But this is a better discussion to have in the context of DDD than in
the context of this bug.

-Justin

[Neil]

L. David Baron wrote:

>On Tuesday 2013-02-12 20:17 -0800, Stephen Pohl wrote:
>
>
>>L. David Baron wrote:
>>
>>>On Tuesday 2013-02-12 18:40 -0800, Asa Dotzler wrote:
>>>
>>>>doing something horribly wrong with memory. This is simply a memory-expensive feature and it's a feature we *must* land.
>>>
>>> Why is it simply a memory-expensive feature? Why does it require any
>>> additional memory overhead at all, other than while an animation is
>>> happening?
>>
>>Currently, when navigating back/forward in the browser history, we don't keep track of the rendered pages. So, without storing snapshots of pages in history, we only have the current page to animate. The previous/next pages would appear blank.
>>
>>
>But we store the pages themselves in the bfcache, and it seems like we ought to be able to paint them into the needed image (or other lower-level graphical buffer) right before we do the animation.
>
>

Or better still, paint the *current* page into a temporary image, start
the browser loading, then animate the image away appropriately. (If the
bfcache applies, the new page will load very quickly, particularly if
the user is distracted by the animation.)

--
Warning: May contain traces of nuts.

[Nicholas Nethercote]

On Tue, Feb 12, 2013 at 8:29 PM, Asa Dotzler <a...@mozilla.com> wrote:
>
> My point isn't to quarrel about the depth of that back stack, but to say
> that it is not OK to simply dismiss a new feature because it increases
> memory footprint. Features vs footprint is a balancing act. Both sides must
> be weighed and that wasn't what I saw happening here. What I saw happening
> was the out of hand dismissal of a feature based on no consideration other
> than increased memory footprint. That cannot be how we roll.

Nobody dismissed it out-of-hand. Reading back through the thread, I

guess you're objecting to what Ed Morley said:

> If this were a 1-5% regression, then I think it would be worth discussing the
> trade-off. At 20%, I really don't see how we can take this, sorry!

But that's hardly a direct dismissal -- he's given a range that he
feels would be acceptable, which feeds directly into the kind of
"weighing out" that you have advocated (and that much of the rest of
the thread has been about).

Nick

[Stephen Pohl]

Hi,

We are now (finally) getting ready to turn on history swipe animations
(bug 860493). There have been two major changes since sending out the
email below earlier in the year:
1. We will only store snapshots for the 5 most recent pages, instead of 20.
2. Bug 817700 has landed, which gives us async <canvas>.toBlob(). Since
this is used every time a snapshot of the page is taken, this greatly
helps in terms of performance.

I'm expecting to turn on history swipe animations either today or
tomorrow by landing the patch in bug 860493 on inbound. This will also
enable the page bounce behavior on OSX (bug 673875).

If you have any questions, concerns or feedback, please let me know.

Thanks,
Stephen

-------- Original Message --------
Subject: Increase in memory utilization on Mac OSX 10.7+ due to history
swipe animations

From: Stephen Pohl <sp...@mozilla.com>
To: dev-pl...@lists.mozilla.org

Date: 2/12/13 5:11 PM
> Hi,
>
> I wanted to give a heads up that we're in the process of finalizing
> the patch for bug 678392 which will give us history swipe animations
> on Mac OSX 10.7+. Since we will be taking snapshots of the 20
> most-recently visited pages, this will undoubtedly lead to an increase
> in memory utilization on these platforms.
>

> There is a discussion about experimental memory measurements in

> comment 305 and 309 in the bug:
> https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c305

> https://bugzilla.mozilla.org/show_bug.cgi?id=678392#c309
>
> We attempted to land the patch late last week but ran into test
> failures caused (most-likely) by not forcing a clobber. The upside of
> it is that it gave us feedback from Talos and an opportunity to email
> the list for feedback before making another attempt at landing the
> patch. The "Trace Malloc MaxHeap" regression is documented in this graph:
> http://graphs.mozilla.org/graph.html#tests=[[29,63,22]]&sel=1360274365000,1360447165000&displayrange=7&datatype=running
>
>
> We'd like to make sure that this increase is acceptable before landing
> the patch. If you have any feedback, please let me know.
>
> Thanks,
> Stephen

[Nicholas Nethercote]

On Mon, Oct 21, 2013 at 12:00 PM, Stephen Pohl <sp...@mozilla.com> wrote:
>
> We are now (finally) getting ready to turn on history swipe animations
> (bug 860493). There have been two major changes since sending out the
> email below earlier in the year:
> 1. We will only store snapshots for the 5 most recent pages, instead of 20.
> 2. Bug 817700 has landed, which gives us async <canvas>.toBlob(). Since
> this is used every time a snapshot of the page is taken, this greatly
> helps in terms of performance.
>
> I'm expecting to turn on history swipe animations either today or
> tomorrow by landing the patch in bug 860493 on inbound. This will also
> enable the page bounce behavior on OSX (bug 673875).

Five pages sounds like a reasonable compromise. Do you have numbers
on the expected trace malloc maxheap regression? It was 20% for 20
pages, so presumably it'll be around 5% now?

Nick

[Stephen Pohl]

You're right, Nick. The assumption here is that the regression will be
around 5%. Unfortunately, I don't have performance numbers from inbound
to confirm yet, since we didn't land yet. The most recent try run with
the patch in bug 860493 applied can be found here:
https://tbpl.mozilla.org/?tree=Try&rev=b33b98df38de

Datazilla (perf-o-matic?) somehow doesn't like to display performance
numbers for me on try. Maybe more experienced minds can make this work?

Thank you,
Stephen