Closed
Bug 1082745
Opened 11 years ago
Closed 11 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•11 years ago
|
Whiteboard: [MemShrink]
Comment 1•11 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•11 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Comment 2•11 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•11 years ago
|
||
The check is now generic.
Attachment #8504923 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Hardware: x86_64 → All
Assignee | ||
Comment 4•11 years ago
|
||
Assignee | ||
Comment 5•11 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•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Comment 7•11 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 | ||
Comment 8•11 years ago
|
||
Goes back to Gecko 32 when bug 994081 landed.
Flags: needinfo?(mwu)
Assignee | ||
Updated•11 years ago
|
Comment 9•11 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•11 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•11 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•11 years ago
|
||
Xrender needs to be enabled/supported to trigger this bug.
Assignee | ||
Comment 13•11 years ago
|
||
Verified as fixed by user on bug 1081926 comment 22 .
Status: RESOLVED → VERIFIED
Keywords: verifyme
Assignee | ||
Comment 14•11 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•11 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•11 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•