Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Offload toTabs() computation to a worker thread #7191

Closed
MarcLeclair opened this issue Dec 16, 2019 · 5 comments
Closed

Offload toTabs() computation to a worker thread #7191

MarcLeclair opened this issue Dec 16, 2019 · 5 comments
Labels
P2 Upcoming release performance Possible performance wins

Comments

@MarcLeclair
Copy link
Contributor

MarcLeclair commented Dec 16, 2019

Why/User Benefit/User Problem

The more tabs we have in our UI, the longer it takes to start our app ( as well as rendering our View Holders). For instance, running ~10 tabs and opening Fenix will take an extra 2% on a Moto G5 on a regular start up. So this means ~ 148 ms for HomeFragment.onStart instead of 40 in the current set up. There's also been random cases where I got really high number from toTab around 700 ms but I can't reproduce it, so I'm still trying to figure out what the culprit was there.

*Keep in mind, my evaluation of off loading the toTabs computation on a separate thread was a quick implementation, there might be better ways to do it.

** This also seems to help performance throughout the app on the HomeFragment, but I noticed that quickly as a by-product of my implementation, so more research should go into that

Impact

For a user that keeps a lot of tabs open (i.e, opening links from external apps will pile on your tabs -- I'm guilty of that), the load time can get considerably longer.

Acceptance Criteria (how do I know when I’m done?)

Start up time shouldn't be affected by the number of tabs open, especially the string computation that comes along with toTabs. Honnestly, the thing that takes the most time in here is the toShortURL

*** I will add my traces later, I just need to order them so they are clearly labeled

┆Issue is synchronized with this Jira Task

@MarcLeclair MarcLeclair added the performance Possible performance wins label Dec 16, 2019
@mcomella
Copy link
Contributor

Triage: we're concerned whether the potential improvements are worth the risks.

@ecsmyth mentioned they'll look into the telemetry on average number of tabs open.

@NotWoods
Copy link
Contributor

We could also pass Sessions to our RecyclerView and compute toTab only when the tab is visible on the screen.

@mcomella
Copy link
Contributor

Moving to In Progress so devs don't accidentally take this (action is on Eric).

@ecsmyth
Copy link

ecsmyth commented Jan 8, 2020

Given the change to the design of the home screen (6190), I think this issue can return to the backlog for now.

@ecsmyth ecsmyth removed their assignment Jan 8, 2020
@mcomella
Copy link
Contributor

mcomella commented Aug 3, 2020

Closing: we're addressing startup holistically and I added this to our start up spreadsheet to track it.

@mcomella mcomella closed this as completed Aug 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 Upcoming release performance Possible performance wins
Projects
None yet
Development

No branches or pull requests

5 participants