Call glFlush() to SkiaGL's GLContext on memory-pressure

RESOLVED FIXED in Firefox 34

Status

()

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: sotaro, Assigned: sotaro)

Tracking

unspecified
mozilla35
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v2.0 fixed, b2g-v2.0M fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

(Whiteboard: [MemShrink:P2])

Attachments

(1 attachment, 1 obsolete attachment)

Assignee

Description

5 years ago
+++ This bug was initially created as a clone of Bug #1049195 +++

During investigating Bug 1049195, I recognized that glFush()/glFinish() works to reduce kgls memory usage. But on memory-pressure, glFlush() is not called to SkiaGL's GLContext.
Assignee

Updated

5 years ago
Blocks: 1049251
Assignee

Updated

5 years ago
Attachment #8481780 - Flags: review?(gwright)
Assignee

Comment 2

5 years ago
[Blocking Requested - why for this release]: Blocking 2.0+ bugs.
blocking-b2g: --- → 2.0?
Comment on attachment 8481780 [details] [diff] [review]
patch - Call glFlush() to SkiaGL's GLContext on memory-pressure

Review of attachment 8481780 [details] [diff] [review]:
-----------------------------------------------------------------

Why not call GrContext::flush()?

Updated

5 years ago
Whiteboard: [MemShrink]
blocking-b2g: 2.0? → 2.0+
Assignee

Comment 4

5 years ago
(In reply to George Wright (:gw280 PTO Aug 15th - 24th) from comment #3)
> Comment on attachment 8481780 [details] [diff] [review]
> patch - Call glFlush() to SkiaGL's GLContext on memory-pressure
> 
> Review of attachment 8481780 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why not call GrContext::flush()?

GrContext::flush() is called in GrContext::freeGpuResources(). But GrContext::flush() does not call glFlush().
http://mxr.mozilla.org/mozilla-central/source/gfx/skia/trunk/src/gpu/GrContext.cpp#216
Assignee

Comment 6

5 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> GrContext::flush() calls GrInOrderDrawBuffer::flush().
> http://mxr.mozilla.org/mozilla-central/source/gfx/skia/trunk/src/gpu/
> GrInOrderDrawBuffer.cpp#556

GrInOrderDrawBuffer::flush() does not call glFlush().
Comment on attachment 8481780 [details] [diff] [review]
patch - Call glFlush() to SkiaGL's GLContext on memory-pressure

Review of attachment 8481780 [details] [diff] [review]:
-----------------------------------------------------------------

Looks fine then, but please add a comment about the flush call to say that GrContext::flush() doesn't call glFlush.
Attachment #8481780 - Flags: review?(gwright) → review+
Assignee

Updated

5 years ago
Assignee: nobody → sotaro.ikeda.g
Assignee

Comment 8

5 years ago
Apply the comment.
Attachment #8481780 - Attachment is obsolete: true
Attachment #8482944 - Flags: review+
Whiteboard: [MemShrink] → [MemShrink:P2]
https://hg.mozilla.org/mozilla-central/rev/29b4ff415e17
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Note that due to recent policy changes, all B2G uplifts needs approval now regardless of blocking status. Please request aurora and b2g32 approval on this patch when you get a chance. Sorry for the inconvenience.
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 13

5 years ago
Comment on attachment 8482944 [details] [diff] [review]
patch - Call glFlush() to SkiaGL's GLContext on memory-pressure r=gw280

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): None.
User impact if declined: An application might be killed more than the fix is applied.
Testing completed: Tested locally on flame device.
Risk to taking this patch (and alternatives if risky): Low.
String or UUID changes made by this patch:
Attachment #8482944 - Flags: approval-mozilla-b2g32?
Attachment #8482944 - Flags: approval-mozilla-aurora?
Flags: needinfo?(sotaro.ikeda.g)
Assignee

Comment 14

5 years ago
(In reply to Sotaro Ikeda [:sotaro] from comment #13)
> 
> [Approval Request Comment]
> Bug caused by (feature/regressing bug #): None.
> User impact if declined: An application might be killed more than the fix is
> applied.

An application might be killed more than the fix is applied because of low memory.
Attachment #8482944 - Flags: approval-mozilla-b2g32?
Attachment #8482944 - Flags: approval-mozilla-b2g32+
Attachment #8482944 - Flags: approval-mozilla-aurora?
Attachment #8482944 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.