[Vertical Homescreen] Use three columns by default everywhere

RESOLVED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::Homescreen
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

unspecified
2.1 S2 (15aug)
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [systemsfe])

Attachments

(5 attachments)

(Assignee)

Description

4 years ago
To solve bug 1029902 we changed the column layout for 256mb devices. Now that we have more information we would like to try to ship three columns everywhere.

We should measure to make sure that this won't impact memory usage.
(Assignee)

Comment 1

4 years ago
Created attachment 8464733 [details] [review]
Pull request - Update homescreen columns
(Assignee)

Comment 2

4 years ago
Created attachment 8464742 [details] [review]
Pull request - Revert column settings memory changes
(Assignee)

Updated

4 years ago
Summary: [Vertical Homescreens] Use three columns by default everywhere → [Vertical Homescreen] Use three columns by default everywhere
(Assignee)

Comment 3

4 years ago
Hey Benwa, Kyle - before we try to go to three columns by default everywhere I just wanted to verify that there are no memory problems. I measured using get_about_memory and saw the following results with around 200 icons:

When passing --minimize: both 3 column and 4 column use the same memory.
When not passing --minimize: 3 column seems to use ~20mb of more memory than 4 column.

Does --minimize discard image data, and if so - are we ok landing with the difference in these numbers?
Flags: needinfo?(khuey)
Flags: needinfo?(bgirard)
I'm going to assume --minimize does mean discarding image data. ~20mb more is big difference. Are you sure you're not seeing a fluctuation? If you don't use minimize try leaving the system idle for 30+ seconds.
Flags: needinfo?(bgirard)
--minimize does discard image data.

20 MB = 200 * 4 bytes / pixel * (3_column_size^2 - 4_column_size^2)

That number is consistent with moving from 64px icons in the 4 column layout to ~175px icons in the 3 column layout, if we're decoding every icon.  But if the 2.7x number you gave me on Wednesday about the overpainting is accurate we would expect to only have to paint 16 * 2.7 = 43.2 icons at any given time.  So something is going wrong here.
Flags: needinfo?(khuey)
(Assignee)

Comment 6

4 years ago
Ok, thanks guys. After re-testing without --minimuze and letting both stabilize for ~30s the numbers are much more inline with each other. I think we should be good to get this landed then.
I really don't think we can hand wave away the transient spike.  If we need 20 MB more to paint the homescreen every time we do that we're OOM killing 20 MB more of background apps than we were before.
(Assignee)

Comment 8

4 years ago
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #7)
> I really don't think we can hand wave away the transient spike.  If we need
> 20 MB more to paint the homescreen every time we do that we're OOM killing
> 20 MB more of background apps than we were before.

Everything is in 'heap-unclassified', I have a small screenshot of the spike, but not the memory report unfortunately. I will try to reproduce the spike on the 4-column to see if it's in the same ballpark.
That's interesting.  We should look into that next week.
Created attachment 8467091 [details]
ion_fromstart_master_20140804_layout_homescreen.png

Measuring ION memory on master with 3 and 4 icons layout, using a userdebug build.
Created attachment 8467092 [details]
ion_fromstart_master_20140804_apps_layout3_homescreen.png

Measuring ION memory on master with 3 icons layout, using a userdebug build, with 50 or 100 preloaded apps.
Created attachment 8467093 [details]
ion_fromstart_master_20140804_apps_layout4_homescreen.png

Measuring ION memory on master with 4 icons layout, using a userdebug build, with 50 or 100 preloaded apps.
(Assignee)

Comment 13

4 years ago
Comment on attachment 8464733 [details] [review]
Pull request - Update homescreen columns

Aus - would you mind throwing a review on this one? Thanks!
Attachment #8464733 - Flags: review?(aus)
(Assignee)

Comment 14

4 years ago
[Blocking Requested - why for this release]: Shipping three columns by default everywhere is a Product/UX blocker.
blocking-b2g: --- → 2.0?

Comment 15

4 years ago
Comment on attachment 8464733 [details] [review]
Pull request - Update homescreen columns

Looks good to me for back-out purposes. Looks like you got a b2g process crash in one of the test runs. It would be nice to re-run it to make sure it's OK (even though the debug b2g process passed the same test suite).
Attachment #8464733 - Flags: review?(aus) → review+
(Assignee)

Comment 16

4 years ago
Thanks for the review Aus!

Homescreen revert landed: https://github.com/mozilla-b2g/gaia/commit/8355af88970e875cb7e4e9d1852e108dab489829

Settings revert landed: https://github.com/mozilla-b2g/gaia/commit/8001b271c194cf929a42a18cfc061788cf555ed7
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Based on email-discussion and product needs.
blocking-b2g: 2.0? → 2.0+
v2.0: https://github.com/mozilla-b2g/gaia/commit/5ad694de462c772b63f2ddced283b2d1ccc3b47b
v2.0: https://github.com/mozilla-b2g/gaia/commit/92d2815f3ec3bd9b35c0e2de8dc34cdf5a3bda19
status-b2g-v2.0: --- → fixed
status-b2g-v2.1: --- → fixed
Target Milestone: 2.1 S1 (1aug) → 2.1 S2 (15aug)
You need to log in before you can comment on or make changes to this bug.