Closed
Bug 1082745
Opened 8 years ago
Closed 8 years ago
Avoid reoptimizing optimized SourceSurfaceCairos
Categories
(Core :: Graphics, defect)
Tracking
()
VERIFIED
FIXED
mozilla36
Tracking | Status | |
---|---|---|
firefox33 | --- | wontfix |
firefox34 | + | fixed |
firefox35 | + | fixed |
firefox36 | --- | fixed |
firefox-esr31 | --- | unaffected |
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | unaffected |
b2g-v2.0M | --- | unaffected |
b2g-v2.1 | --- | unaffected |
b2g-v2.2 | --- | unaffected |
People
(Reporter: mwu, Assigned: mwu)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file, 1 obsolete file)
919 bytes,
patch
|
lmandel
:
approval-mozilla-aurora+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
In some cases, OptimizeSourceSurface will be called on a surface that's already been optimized. This patch makes the function return early in that case.
Attachment #8504923 -
Flags: review?(bas)
Updated•8 years ago
|
Whiteboard: [MemShrink]
Comment 1•8 years ago
|
||
Comment on attachment 8504923 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos Review of attachment 8504923 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/DrawTargetCairo.cpp @@ +1277,5 @@ > #ifdef CAIRO_HAS_XLIB_SURFACE > + if (aSurface->GetType() == SurfaceType::CAIRO && > + cairo_surface_get_type( > + static_cast<SourceSurfaceCairo*>(aSurface)->GetSurface()) == > + CAIRO_SURFACE_TYPE_XLIB) { We could probably make this generic and compare the type of aSurface to that of mSurface. That'd make it cleaner if we ever implement optimizing for other cairo backends.
![]() |
||
Updated•8 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 2•8 years ago
|
||
Comment on attachment 8504923 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos Review of attachment 8504923 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/2d/DrawTargetCairo.cpp @@ +1277,5 @@ > #ifdef CAIRO_HAS_XLIB_SURFACE > + if (aSurface->GetType() == SurfaceType::CAIRO && > + cairo_surface_get_type( > + static_cast<SourceSurfaceCairo*>(aSurface)->GetSurface()) == > + CAIRO_SURFACE_TYPE_XLIB) { Agreed.
Attachment #8504923 -
Flags: review?(bas) → review+
Assignee | ||
Comment 3•8 years ago
|
||
The check is now generic.
Attachment #8504923 -
Attachment is obsolete: true
Assignee | ||
Updated•8 years ago
|
Hardware: x86_64 → All
Assignee | ||
Comment 4•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/f013910bbead
Assignee | ||
Comment 5•8 years ago
|
||
[Tracking Requested - why for this release]: This fixes severe leaks for certain canvas users (like freeciv) on GTK/X11.
status-b2g-v1.4:
--- → unaffected
status-b2g-v2.0:
--- → unaffected
status-b2g-v2.0M:
--- → unaffected
status-b2g-v2.1:
--- → unaffected
status-b2g-v2.2:
--- → unaffected
status-firefox34:
--- → affected
status-firefox35:
--- → affected
status-firefox36:
--- → affected
status-firefox-esr31:
--- → unaffected
tracking-firefox34:
--- → ?
tracking-firefox35:
--- → ?
tracking-firefox36:
--- → ?
Comment 6•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f013910bbead
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 7•8 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #5) > [Tracking Requested - why for this release]: > This fixes severe leaks for certain canvas users (like freeciv) on GTK/X11. How far back does this issue go?
Flags: needinfo?(mwu)
Assignee | ||
Updated•8 years ago
|
Comment 9•8 years ago
|
||
Thanks for the bug ref. Once this has been verified on m-c, let's get it uplifted to Beta and Aurora.
status-firefox33:
--- → wontfix
tracking-firefox36:
? → ---
Assignee | ||
Comment 10•8 years ago
|
||
To verify, use a Linux system/VM, and load freeciv from the link in bug 1081926 - http://play.freeciv.org/webclient/?action=load&load=tutorial&scenario=true . Watch Xorg in top (or your favorite process monitor) and make sure the memory usage doesn't jump several gigabytes when freeciv loads.
Keywords: verifyme
![]() |
||
Comment 11•8 years ago
|
||
(In reply to Michael Wu [:mwu] from comment #10) > To verify, use a Linux system/VM, and load freeciv from the link in bug > 1081926 - > http://play.freeciv.org/webclient/?action=load&load=tutorial&scenario=true . > Watch Xorg in top (or your favorite process monitor) and make sure the > memory usage doesn't jump several gigabytes when freeciv loads. But note that it doesn't affect all systems. I was unable to reproduce on my Ubuntu 14.04 box.
Assignee | ||
Comment 12•8 years ago
|
||
Xrender needs to be enabled/supported to trigger this bug.
Assignee | ||
Comment 13•8 years ago
|
||
Verified as fixed by user on bug 1081926 comment 22 .
Status: RESOLVED → VERIFIED
Keywords: verifyme
Assignee | ||
Comment 14•8 years ago
|
||
Comment on attachment 8505586 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos, v2 Approval Request Comment [Feature/regressing bug #]: Regressed by bug 994081. [User impact if declined]: Using the canvas API in certain ways will cause massive memory usage in the X server on GTK/X11 systems. It can often cause an X server OOM crash. [Describe test coverage new/current, TBPL]: No tests, but testing this is somewhat difficult since it'll be made ineffective once bug 1081926 is fixed. [Risks and why]: Low. I can't think of any risks. [String/UUID change made/needed]: None.
Attachment #8505586 -
Flags: approval-mozilla-beta?
Attachment #8505586 -
Flags: approval-mozilla-aurora?
Comment 15•8 years ago
|
||
Comment on attachment 8505586 [details] [diff] [review] Avoid reoptimizing optimized SourceSurfaceCairos, v2 Beta+ Aurora+
Attachment #8505586 -
Flags: approval-mozilla-beta?
Attachment #8505586 -
Flags: approval-mozilla-beta+
Attachment #8505586 -
Flags: approval-mozilla-aurora?
Attachment #8505586 -
Flags: approval-mozilla-aurora+
Comment 16•8 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fdd0036a0ae5 https://hg.mozilla.org/releases/mozilla-beta/rev/5e3fc9d8a99b
You need to log in
before you can comment on or make changes to this bug.
Description
•