Closed Bug 1082745 Opened 11 years ago Closed 11 years ago

Avoid reoptimizing optimized SourceSurfaceCairos

Categories

(Core :: Graphics, defect)

All
Linux
defect
Not set
normal

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)

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)
Whiteboard: [MemShrink]
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.
Whiteboard: [MemShrink] → [MemShrink:P2]
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+
The check is now generic.
Attachment #8504923 - Attachment is obsolete: true
Hardware: x86_64 → All
[Tracking Requested - why for this release]: This fixes severe leaks for certain canvas users (like freeciv) on GTK/X11.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
(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)
Goes back to Gecko 32 when bug 994081 landed.
Flags: needinfo?(mwu)
Thanks for the bug ref. Once this has been verified on m-c, let's get it uplifted to Beta and Aurora.
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
(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.
Xrender needs to be enabled/supported to trigger this bug.
Verified as fixed by user on bug 1081926 comment 22 .
Status: RESOLVED → VERIFIED
Keywords: verifyme
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 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+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: