Closed
Bug 748649
Opened 13 years ago
Closed 13 years ago
Add PLayers async NoSwapUpdate
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla15
Tracking | Status | |
---|---|---|
blocking-fennec1.0 | --- | - |
People
(Reporter: BenWa, Assigned: BenWa)
References
Details
Attachments
(2 files, 2 obsolete files)
(deleted),
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
BenWa
:
review+
|
Details | Diff | Splinter Review |
With the introduction of TiledThebesLayer we no longer have to swap for that layer type. This patch introduce an async NoSwapUpdate when we can guarantee that no EditReply will be generated for the transaction.
I haven't test this on mobile but I will early tomorrow. It work well on Mac. FYI we block content on average 5-15ms per layers update and most update on sites without a changing canvas will benefit from this change.
This may also be an option to move towards async layers update one layer type/back end a time without a sweeping refactor.
Assignee | ||
Comment 1•13 years ago
|
||
Assignee | ||
Comment 2•13 years ago
|
||
The risk is moderate and the performance win is significant over many paints (esp. smaller paints).
blocking-fennec1.0: --- → ?
Assignee | ||
Comment 3•13 years ago
|
||
This should make you happy Chris, to support this properly we need to fix bug 747811 otherwise a get a race. Patch is already underway. Let's get this reviewed in the meantime since it wont effect this patch in the slightest.
Depends on: 747811
Updated•13 years ago
|
blocking-fennec1.0: ? → -
Comment 4•13 years ago
|
||
We don't require this at the moment, but we may come back to it if we still aren't good enough and it shakes out on m-c first.
Assignee | ||
Comment 5•13 years ago
|
||
If you don't have time to look at the patch review Chris do you think Ali could review it? He knows the shadow layers code very well.
I'll be able to look tomorrow. Something came up today.
Assignee | ||
Comment 7•13 years ago
|
||
ping
Comment on attachment 618162 [details] [diff] [review]
patch
>diff --git a/gfx/layers/ipc/PLayers.ipdl b/gfx/layers/ipc/PLayers.ipdl
>+ // We don't need to send a sync transaction if
>+ // no transaction operate require a swap.
>+ async NoSwapUpdate(Edit[] cset, bool isFirstPaint);
Nit: |UpdateNoSwap()|.
Yuck ... I thought the isFirstPaint turd went away :/.
r=me with naming change. Looks good!
Attachment #618162 -
Flags: review?(jones.chris.g) → review+
Assignee | ||
Comment 9•13 years ago
|
||
Attachment #618162 -
Attachment is obsolete: true
Attachment #620015 -
Flags: review+
Assignee | ||
Comment 10•13 years ago
|
||
Attachment #620028 -
Flags: review?(ajuma)
Comment 11•13 years ago
|
||
Comment on attachment 620028 [details] [diff] [review]
bug 747811 work-around
Review of attachment 620028 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/layers/opengl/TiledThebesLayerOGL.cpp
@@ +131,5 @@
> void
> TiledThebesLayerOGL::PaintedTiledLayerBuffer(const BasicTiledLayerBuffer* mTiledBuffer)
> {
> mMainMemoryTiledBuffer = *mTiledBuffer;
> + delete mTiledBuffer;
Assuming this is also temporary, add a comment here too.
Attachment #620028 -
Flags: review?(ajuma) → review+
Assignee | ||
Comment 12•13 years ago
|
||
Attachment #620028 -
Attachment is obsolete: true
Attachment #620034 -
Flags: review+
Assignee | ||
Comment 13•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b2b64e68a410
https://hg.mozilla.org/integration/mozilla-inbound/rev/25c7d09bf7a9
Target Milestone: --- → mozilla15
Comment 14•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b2b64e68a410
http://hg.mozilla.org/mozilla-central/rev/25c7d09bf7a9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Comment 15•13 years ago
|
||
It looks like this caused a 53% improvement in the DHTML NoChrome benchmark on Android Native. Should we consider taking this on Aurora?
Assignee | ||
Comment 16•13 years ago
|
||
It seems to be it:
http://graphs.mozilla.org/graph.html#tests=[[19,63,20]]&sel=none&displayrange=7&datatype=running
I'm guessing this benchmark does a lot of small-ish painting? I'd be willing to uplift this if it bakes well on trunk.
You need to log in
before you can comment on or make changes to this bug.
Description
•