Avoid calling nsDisplayTransform::GetFrameBoundsForTransform

RESOLVED FIXED in Firefox 27

Status

()

Core
SVG
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: roc, Assigned: roc)

Tracking

({perf, regression})

Trunk
mozilla27
x86_64
Linux
perf, regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox26 unaffected, firefox27+ fixed)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment)

Bug 923193 regressed tsvg performance.

We should fix it by avoiding calling GetFrameBoundsForTransform if possible. I think the easiest and best thing to do is to avoid calling GetDeltaToMozTransformOrigin for translation-only transforms, since it doesn't affect them.
Actually the thing to do is to avoid calling GetFrameBoundsForTransform when we have absolute coordinates for transform-orign --- which is the default for SVG, so we end up avoiding the cost of bug 923193 except where transform-origin is explicitly used.
Created attachment 820213 [details] [diff] [review]
fix
Attachment #820213 - Flags: review?(cam)
Attachment #820213 - Flags: review?(cam) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/a61d898ea4fa

Comment 4

4 years ago
Whole push backed out since not clear what caused the reftest failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=ffd9c7bbd2fe&jobname=reftest

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/69d9f136cf26
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/b71812b077de
status-firefox27: --- → affected
tracking-firefox27: --- → ?
Keywords: perf, regression

Updated

4 years ago
status-firefox26: --- → unaffected
tracking-firefox27: ? → +
http://hg.mozilla.org/integration/mozilla-inbound/rev/75ee2a0bc5d3

Comment 6

4 years ago
Push backed out for reftest failures:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=reftest&rev=75ee2a0bc5d3

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/efe4b4053304
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/e33a8cab1096

These are the same reftest failures that caused the backout in comment 4.
https://tbpl.mozilla.org/?tree=Try&rev=c52e80b74d53
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1578a4fc86d

Comment 8

4 years ago
https://hg.mozilla.org/mozilla-central/rev/c1578a4fc86d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox27: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
This only partially fixed the tsvg regression from bug 923193.  The original regression was about +80ms, and the change from this patch was about -30ms.
Where is data for this regression this bug is tracking?
Keywords: verifyme
(In reply to Matt Brubeck (:mbrubeck) from comment #9)
> This only partially fixed the tsvg regression from bug 923193.  The original
> regression was about +80ms, and the change from this patch was about -30ms.

That's probably because there are a lot more places that call GetFrameBoundsForTransform than the caller that the patch for this bug guarded. Unfortunately at least some of those other callers don't look like they can be guarded since we don't know at the call point whether we will want the bounds later or not.
(In reply to [:tracy] Tracy Walker - QA Mentor from comment #10)
> Where is data for this regression this bug is tracking?

http://graphs.mozilla.org/graph.html#tests=[[281,63,29]]&sel=none&displayrange=30&datatype=running

For example. As far as verifying the regression fixed, no need though. It's not.

I'm not sure if roc would prefer to reopen this bug, or have another opened for the rest. Or maybe bug 923193 should be backed out until we implement caching of bboxes. roc?
Blocks: 933354
I filed bug 933354 to track the remaining regression.
Backed out everything involving this bug:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3fb151446ec5

Leaving FIXED since there is no more work to do here.
Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/3fb151446ec5

Comment 16

4 years ago
Removing keyword per comment 12.
Keywords: verifyme

Updated

4 years ago
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.