1030608 - vertical homescreen uses 9MB of additional KGSL

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect, P1)

Description

[Andreas Gal :gal]

https://bugzilla.mozilla.org/show_bug.cgi?id=1029856#c10

Comment 1

[Benoit Girard (:BenWa)]

Quickly off the top of my head:
We have a display port with Vertical so we have 2 extra screen size layers for the top and bottom display port. If they have both a front and a back buffer then we get:
1000*800*2(of them)*2(front/back)*2(bytes per pixel if we're in 565) = 6 MB

The old homescreen would have similar peak memory but it was only during active panning.

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

Benoit can take a quick look when he's back, but this just may be something that's result of all the extra stuff we have.  I'm assuming this is on a recent build (the bug is recent, but it's unclear if the build was from that day), so we don't have layers for all the icons even without editing.  I also assume this wouldn't include the decoded images, but we probably have more of those (e.g., https://bugzilla.mozilla.org/show_bug.cgi?id=1008942#c13)

Comment 3

[Andreas Gal :gal]

In stead state we should render a couple invisible tiles but that shouldn't add up to 9MB. Thats insane. Please investigate. Something is wrong here.

Comment 4

[Andreas Gal :gal]

Is this an issue with tile size? The screen is 320x480. If our tile size is 512 we over-allocate and never use almost 50% of that texture memory if we scroll vertical only.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

Our tile size is a preference, defaulting to 256x256, but it's the same thing - we end up allocating 512 for the 320 we need. We could measure things with 160x160, but I don't know if the driver would give us the next power of two anyway.

Comment 6

[Sotaro Ikeda [:sotaro]]

IIRC, codeaura's gralloc size(width/height) is aligned to 32.

Comment 7

[Andreas Gal :gal]

GPUs usually want textures that are lane width aligned (32 bytes or so), but a modern GPU shouldn't mind NPOT textures. Too small textures are a problem because we end up switching textures too much (too many draw commands) so maybe we can do min(width, height)^2 as the texture size? Or half or quarter of it if thats too large.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Most of the activity towards dealing with this will first be in bug 1033538.

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Some of this appears to be related to multiple copies of screenshots of the apps in the background.

Comment 10

[Benoit Girard (:BenWa)]

If the patch from bug 1036654 is correct:
We have 10MB of gralloc:
- 7.3 MB of that is tiles
-- 1.1 MB is due to fragmentation
- 3 MB are from two fullscreen layers.

I don't have data on the old homescreen yet. So I can't yet say how much we've regressed gralloc by.

Comment 11

[Benoit Girard (:BenWa)]

Kevin are you able to restore the old homescreen for testing? The instructions I received left me with a blank homescreen?

Comment 12

[Kevin Grandon :kgrandon]

(In reply to Benoit Girard (:BenWa) from comment #11)
> Kevin are you able to restore the old homescreen for testing? The
> instructions I received left me with a blank homescreen?

Yes, by changing `appName` from `verticalhome` to `homescreen` in build/settings.js, I'm able to revert to the old homescreen after a make reset-gaia. Do you have a screenshot/logcat of what you're running into?

Comment 13

[Benoit Girard (:BenWa)]

We seem to barely use 10MB of texture. Note that 10MB includes the stride of the gralloc buffers but it doesn't include external fragmentation, if any. Changing the description to reflect that the increase of memory is reported by KGSL_alloc.

From http://lwn.net/Articles/394665/, I see that KGSL stands for Kernel Graphics Support Layer.

On my flame I see gralloc at 10MB and KGSL at 18MB. I'd like to understand what that other 8MB could be.

Sushil what are other big consumers of kgsl page alloc other than gralloc?

Comment 14

[Benoit Girard (:BenWa)]

(In reply to Kevin Grandon :kgrandon from comment #12)
> (In reply to Benoit Girard (:BenWa) from comment #11)
> > Kevin are you able to restore the old homescreen for testing? The
> > instructions I received left me with a blank homescreen?
> 
> Yes, by changing `appName` from `verticalhome` to `homescreen` in
> build/settings.js, I'm able to revert to the old homescreen after a make
> reset-gaia. Do you have a screenshot/logcat of what you're running into?

I just did a full b2g build + rebuild + make gaia-reset and I'm still seeing an empty homescreen...

and I just did a adb shell reboot and now it worked.

Comment 15

[Benoit Girard (:BenWa)]

Alright here are my results:
7,778,304 -> 10,244,096

I previously saw 18 MB but I updated and picked up bug 1034347, bug 1027231 and now I'm done to 10 MB. The regression now looks to be 2.5 MB.

Comment 16

[Sushil]

(In reply to Benoit Girard (:BenWa) from comment #15)
> Alright here are my results:
> 7,778,304 -> 10,244,096
> 
> I previously saw 18 MB but I updated and picked up bug 1034347, bug 1027231
> and now I'm done to 10 MB. The regression now looks to be 2.5 MB.

Benwa, let me know, if you still need the info? In this case, is gralloc using 7.77 MB and is KGSL page_alloc showing 10.25 MB ? While testing, also make sure to pick the fix from Bug 1029856.

All these numbers are in GPU Composition, I am not much familiar with KGSL so I need to check with Graphics about this query.

Comment 17

[Benoit Girard (:BenWa)]

Let me clarify:
/sys/class/kgsl/kgsl/page_alloc reports 7.77 MB for the old homescreen.
/sys/class/kgsl/kgsl/page_alloc reports 10.25 MB for the new homescreen.

Comment 18

[Benoit Girard (:BenWa)]

I noticed bug 1037154 while working on this. This isn't exactly related to this issue however.

Comment 19

[Sushil]

(In reply to Benoit Girard (:BenWa) from comment #17)
> Let me clarify:
> /sys/class/kgsl/kgsl/page_alloc reports 7.77 MB for the old homescreen.
> /sys/class/kgsl/kgsl/page_alloc reports 10.25 MB for the new homescreen.

Compare the number of rendered layers and size of each rendered layer on old home screen with the new home screen. The difference of 2.5 MB in KGSL page_alloc should be due to this.

Comment 20

[Sushil]

"/sys/class/kgsl/kgsl/page_alloc" indicates amount of physical memory allocated by kgsl driver for Gfx use across all applications using kgsl. Its the total memory allocated.

Comment 21

[Kevin Grandon :kgrandon]

Now that we have the will-change and slow path problems addressed for 2.0, can we take another measurement here?

Comment 22

[Benoit Girard (:BenWa)]

I already have, see Comment 17. The gap is now 2.5 MB.

Comment 23

[bhavana bajaj [:bajaj]]

(In reply to Benoit Girard (:BenWa) from comment #22)
> I already have, see Comment 17. The gap is now 2.5 MB.

So are the dependencies (1036654) going to help with the gap here ? Just trying to see what the next steps here are to resolve this on 2.0?

Comment 24

[Benoit Girard (:BenWa)]

Fixing bug 1033538 wont help since we fixed bug 1027231 which is a lower risk correction.

I'm using this opportunity to put better hooks to measure and audit how our GFX memory is being used. I have some ideas that I'm investigating at the moment. I'll post at least one dependent bug, hopefully tomorrow. I expect to get at least another 1 MB back.

Comment 25

[Milan Sreckovic [:milan] (needinfo for best results)]

We'll make sure that any dependent bugs that should block 2.0 are tagged as such.

Comment 26

[Preeti Raghunath(:Preeti)]

Milan

Any update here?

Comment 27

[Benoit Girard (:BenWa)]

Removing bug 1033538 since we have a work around in place.

We're down to 2.5 MB and once bug 1038988 lands we should at at about 1 MB regression which should be within the acceptable range considering we now pre-render more content.

Comment 28

[Andreas Gal :gal]

Sounds reasonable. Please make sure we have the bugs covered we discussed yesterday (using canvas, images in memory multiple times).

Comment 29

[Yeojin Chung [:YeojinC]]

Unable to verify this issue as it seems to be a back end issue.