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

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Sotaro Ikeda [:sotaro]]

+++ 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.

Comment 1

[Sotaro Ikeda [:sotaro]]

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

Comment 2

[Sotaro Ikeda [:sotaro]]

[Blocking Requested - why for this release]: Blocking 2.0+ bugs.

Comment 3

[George Wright (:gw280) (needinfo me!)]

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()?

Comment 4

[Sotaro Ikeda [:sotaro]]

(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

Comment 5

[Sotaro Ikeda [:sotaro]]

GrContext::flush() calls GrInOrderDrawBuffer::flush().
http://mxr.mozilla.org/mozilla-central/source/gfx/skia/trunk/src/gpu/GrInOrderDrawBuffer.cpp#556

Comment 6

[Sotaro Ikeda [:sotaro]]

(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 7

[George Wright (:gw280) (needinfo me!)]

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.

Comment 8

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Call glFlush() to SkiaGL's GLContext on memory-pressure r=gw280

Apply the comment.

Comment 9

[Sotaro Ikeda [:sotaro]]

https://tbpl.mozilla.org/?tree=Try&rev=8fb65a4fb30e

Comment 10

[Sotaro Ikeda [:sotaro]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/29b4ff415e17

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/29b4ff415e17

Comment 12

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 13

[Sotaro Ikeda [:sotaro]]

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:

Comment 14

[Sotaro Ikeda [:sotaro]]

(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.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a5d0c86dca93
https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/b4f4f0423815

Comment 16

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/b4f4f0423815