936062 - slight tp5 regression on os x due to back/forward screenshot capture

Status: RESOLVED INVALID

(Core :: Widget: Cocoa, defect, P2)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

I was looking at osx 10.8 on tp5 and noticed a handful of pages have a noticeable bump in load times after these two commits: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=07ea7b11adef and https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=2ea21428f70c
were landed.

This included patches for:
bug 596723
bug 545156
bug 860493

I have looked at the pushlog and the pushes prior had tp5o summary values of ~215 and for this revision and future revisions it is ~223.  This is very visible on datazilla when looking at a per page timing:
https://datazilla.mozilla.org/?start=1383239792&stop=1383844592&product=Firefox&repository=Mozilla-Inbound&os=mac&os_version=OS%20X%2010.8&test=tp5o&graph_search=07ea7b11adef&tr_id=3445678&graph=chinaz.com&x86=false&error_bars=false&project=talos

:neil, :spohl- can you guys take a look at your patches and see if they would affect page load times in general or on osx specifically?

Comment 1

[Stephen A Pohl [:spohl]]

This is most definitely due to bug 860493, since we're now taking snapshots of pages when navigating away. Bug 817700 handles the encoding of the snapshot asynchronously, but there is inevitably some overhead that still happens on the main thread.

Comment 2

[Joel Maher ( :jmaher ) (UTC -8)]

thanks Stephen- is there work to reduce the impact this has, or has there been discussion around accepting this as a performance regression?

Comment 3

[Stephen A Pohl [:spohl]]

I think it's important to point out that this feature remains turned off on the release channel, at least for now. It's turned on in nightly and aurora to gather this type of feedback.

There is a general effort to reduce the performance regressions of history swipe animations (see bug 860493 comment 39 for example).

I've previously notified dev-platform [1] about the expected performance regressions due to this feature, although I have to admit that it was mostly focused on regressions in terms of memory consumption.

I've inquired about the proper channel to discuss performance regressions in dev-planning [2]. :-) I'd be more than happy to start a conversation or add regressions to a wiki page once we know where this should happen.

I hope this helps.

[1] https://groups.google.com/d/msg/mozilla.dev.platform/NOCX_tDfFCA/vLsIWBAW7VIJ
[2] https://groups.google.com/d/msg/mozilla.dev.planning/IBdpNWhWU94/iiGk8mxNfEgJ

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

checking in on this bug, is anybody looking into this?

Comment 5

[Stephen A Pohl [:spohl]]

(In reply to Joel Maher (:jmaher) from comment #4)
> checking in on this bug, is anybody looking into this?

This regression shouldn't be present at the moment. History swipe animations should currently be turned off on all channels (see bug 860493 comment 60). This bug will be looked into at the same time as other performance bugs before turning the feature back on. I'm hoping to return to this soon.

Comment 6

[Stephen A Pohl [:spohl]]

We have switched to arrows instead of page snapshots for history swiping (bug 860493). Once we go back to page snapshot animations, this will most likely be built on top of work such as bug 933389, which will require its own performance measurements. It does not make sense to keep this bug around, since it measured the old drawWindow way of taking snapshots.