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

[Bug] Dispatchers.Main still has long cold start delay (Phase II) #4064

Closed
colintheshots opened this issue Jul 15, 2019 · 5 comments
Closed
Assignees
Labels
🐞 bug Crashes, Something isn't working, .. E2 Estimation Point: easy, half a day to 2 days P1 Current sprint performance Possible performance wins
Milestone

Comments

@colintheshots
Copy link
Contributor

colintheshots commented Jul 15, 2019

Steps to reproduce

Look at the latest Nimbledroid.com report OR run cold startup with Systrace running.

Expected behavior

We should not block the main thread for hundreds of milliseconds to checksum the jar using FastServiceLoader or ServiceLoader classes.

Actual behavior

Even with the latest fixes from Kotlin Coroutines, Dispatchers.Main still blocks the main thread for too long. It takes 200ms to run FastServiceLoader, only a ~60ms improvement.

This bug is initially marked "waiting" because we're waiting on the Android team to fix it using R8. If we can use upgrade to the future stable 2.5.0 Android Gradle Plugin and a new version of Kotlin Coroutines which no longer uses FastServiceLoader, we can perform cold startup in a substantially shorter time. This is because R8 will optimize out all ServiceLoader calls and Kotlin Coroutines will switch back to the built-in ServiceLoader calls so they get optimized out.

Device information

  • Android device: Pixel 3XL
  • Fenix version: 1.0.1

┆Issue is synchronized with this Jira Task

@colintheshots colintheshots added 🐞 bug Crashes, Something isn't working, .. 🙅 waiting Issues that are blocked or has dependencies that are not ready performance Possible performance wins labels Jul 15, 2019
@vesta0 vesta0 added this to Items needed for Fenix Q3 in Fenix: A-S Bugs Aug 8, 2019
@colintheshots colintheshots added E2 Estimation Point: easy, half a day to 2 days P3 Some future sprint P2 Upcoming release and removed P3 Some future sprint labels Aug 15, 2019
@colintheshots
Copy link
Contributor Author

I'm calling this P2. It has a major effect on performance, but we're waiting on Google to release Android Gradle Plugin and JetBrains to release a new Kotlin coroutines version.

@colintheshots colintheshots added P1 Current sprint and removed P2 Upcoming release 🙅 waiting Issues that are blocked or has dependencies that are not ready labels Aug 21, 2019
@colintheshots colintheshots self-assigned this Aug 21, 2019
@colintheshots colintheshots added this to To be Triaged in Fenix Sprint Kanban via automation Aug 21, 2019
@colintheshots colintheshots moved this from To be Triaged to In Progress in Fenix Sprint Kanban Aug 21, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Aug 21, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Aug 21, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Aug 26, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Aug 26, 2019
@colintheshots colintheshots moved this from In Progress to Done in Fenix Sprint Kanban Aug 29, 2019
@colintheshots
Copy link
Contributor Author

This should be fixed in the latest master using a Dev version of R8. The results should appear in Nimbledroid in the next day or so.

@colintheshots colintheshots added this to the v2.0 milestone Aug 30, 2019
@vesta0 vesta0 moved this from Done to In Progress in Fenix Sprint Kanban Sep 2, 2019
@colintheshots colintheshots moved this from In Progress to Done in Fenix Sprint Kanban Sep 4, 2019
@sblatz
Copy link
Contributor

sblatz commented Sep 6, 2019

@colintheshots can we close this or put a qa needed label on it? :)

@colintheshots
Copy link
Contributor Author

We have good news here that confirms the performance gains right after the commit landed. I think we can close this safely.

Screenshot from 2019-09-03 12-50-21

@colintheshots colintheshots removed this from the v2.0 milestone Sep 6, 2019
@colintheshots colintheshots modified the milestones: v1.4, v2.0 Sep 6, 2019
@vesta0 vesta0 removed this from Items needed for Fenix Q3 in Fenix: A-S Bugs Sep 10, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Dec 13, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Dec 13, 2019
colintheshots added a commit to colintheshots/fenix that referenced this issue Dec 13, 2019
@colintheshots
Copy link
Contributor Author

I removed the pre-release R8 version and replaced this with an improved Kotlin coroutines library instead.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. E2 Estimation Point: easy, half a day to 2 days P1 Current sprint performance Possible performance wins
Projects
None yet
Development

No branches or pull requests

2 participants