Closed Bug 1030608 Opened 10 years ago Closed 10 years ago

vertical homescreen uses 9MB of additional KGSL

Categories

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

x86
macOS
defect

Tracking

()

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: gal, Assigned: BenWa)

References

Details

(Keywords: memory-footprint, perf, Whiteboard: [MemShrink:P1][c=memory p= s= u=2.0])

Blocks: 1029856
blocking-b2g: --- → 2.0?
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.
Blocks: 1029902
blocking-b2g: 2.0? → 2.0+
Keywords: footprint, perf
Whiteboard: [MemShrink]
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)
Assignee: nobody → bgirard
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.
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.
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.
IIRC, codeaura's gralloc size(width/height) is aligned to 32.
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.
Whiteboard: [MemShrink] → [MemShrink][c=memory p= s= u=2.0]
Severity: normal → blocker
Priority: -- → P1
Most of the activity towards dealing with this will first be in bug 1033538.
Some of this appears to be related to multiple copies of screenshots of the apps in the background.
Depends on: 1034347
Depends on: 1035474
Whiteboard: [MemShrink][c=memory p= s= u=2.0] → [MemShrink:P1][c=memory p= s= u=2.0]
Depends on: 1036654
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.
Kevin are you able to restore the old homescreen for testing? The instructions I received left me with a blank homescreen?
Flags: needinfo?(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?
Flags: needinfo?(kgrandon)
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?
Flags: needinfo?(sushilchauhan)
Summary: vertical homescreen uses 9MB of additional texture memory → vertical homescreen uses 9MB of additional KGSL
(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.
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.
(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.
Flags: needinfo?(sushilchauhan) → needinfo?(bgirard)
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.
Flags: needinfo?(bgirard)
I noticed bug 1037154 while working on this. This isn't exactly related to this issue however.
(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.
"/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.
Now that we have the will-change and slow path problems addressed for 2.0, can we take another measurement here?
I already have, see Comment 17. The gap is now 2.5 MB.
(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?
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.
We'll make sure that any dependent bugs that should block 2.0 are tagged as such.
Depends on: 1038988
Milan Any update here?
Flags: needinfo?(milan)
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.
No longer depends on: 1033538
Flags: needinfo?(milan)
Sounds reasonable. Please make sure we have the bugs covered we discussed yesterday (using canvas, images in memory multiple times).
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.0 S6 (18july)
Unable to verify this issue as it seems to be a back end issue.
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [QAnalyst-Triage+][QAnalyst-verify-]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.