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

When clicking URL bar, UI waits on long delay for on capturing thumbnail #11825

Closed
mcomella opened this issue Jun 22, 2020 · 4 comments
Closed
Assignees
Labels
needs:product needs:triage Issue needs triage performance Possible performance wins

Comments

@mcomella
Copy link
Contributor

mcomella commented Jun 22, 2020

This is a breakout to fix an issue found in the investigations in #8795

STR:

  • Open a website and wait for it to load
  • Make sure that the bottom bar is visible
  • Tap on the URL bar

Expected:
Search screen appears quickly

Actual:
Long pause before search screen appears. Partial cause, from my perf investigation #8795 (comment):

I took a profile on my GS5: https://share.firefox.dev/37SL2of

It appears we're launching a coroutine on a background thread to capture a thumbnail and don't continue on the main thread until it completes. This is backed up by the profile visual where we do onClick, the compositor executes for a while, and we begin main thread execution:

Here's the code where we capture a thumbnail:
https://searchfox.org/mozilla-mobile/rev/f1f9880935a87f0dbdc9d107a9067dbd73338b96/fenix/app/src/main/java/org/mozilla/fenix/browser/BrowserAnimator.kt#123-136

We shouldn't block the UI from appearing on a background operation, especially one that doesn't update information on the subsequent screen.

┆Issue is synchronized with this Jira Task

@mcomella mcomella added the performance Possible performance wins label Jun 22, 2020
@mcomella mcomella added this to Top 10 Inter-Team bugs in Performance, front-end roadmap Jun 22, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Jun 22, 2020
@mcomella
Copy link
Contributor Author

It appears this thumbnail is captured to be used for the animation, which prevents the screen from flashing when the animation occurs. In the worst case where we're unable to perform the animation because capturing the thumbnail is too slow, we should check with UX/product but I think a fast flicker is better than a slow, smoother transition.

For non-extreme solutions, maybe we can:

  • timeout on the thumbnail capture (not preferred as this still delays the interaction)
  • take the thumbnails at different times so they're non-blocking (not sure how to do this efficiently)
  • use a different animation that doesn't require a thumbnail (e.g. we seem to be able to do alpha overlays over GV so we could just fade into the new fragment) - I guess this is the ideal one so far

@boek boek added this to Sprint in Fenix Sprint Kanban Jun 23, 2020
@mcomella
Copy link
Contributor Author

I ran an experiment where I commented out the thumbnail code and added onComplete where it should be: #8795 (comment) I only measured anecdotally before so I wanted to re-run to ensure this is actually a performance problem and not an inaccuracy with the profiler.

In the original code, on the GS5 over 3 runs, to show the first frame of the SearchFragment, it takes:

  • 760ms in the original code
  • 261.67ms in the patched version

This confirms fixing this issue will be a big improvement. (Note: this timing does not represent the total duration to show the search screen, just the first frame, but it still good for comparison)

@sblatz sblatz self-assigned this Jun 24, 2020
@sblatz
Copy link
Contributor

sblatz commented Jun 24, 2020

By changing this as suggested we will lose the cross fade animation we have. This is not possible to mimic without capturing the engine view as we cannot animate the GV layer.

@vesta0 what are your thoughts on losing this animation in favor of a performance win?

Current Cross Fade

cross fade currently

Proposed No Cross Fade

proposed no cross fade

@vesta0
Copy link
Collaborator

vesta0 commented Jun 24, 2020

I like the animation but performance takes priority. Thanks @sblatz

sblatz added a commit to sblatz/fenix that referenced this issue Jun 24, 2020
sblatz added a commit to sblatz/fenix that referenced this issue Jun 24, 2020
ekager pushed a commit to sblatz/fenix that referenced this issue Jun 25, 2020
ekager pushed a commit that referenced this issue Jun 25, 2020
@ekager ekager closed this as completed Jun 25, 2020
Fenix Sprint Kanban automation moved this from Sprint to Sprint 20.11 Done Jun 25, 2020
Performance, front-end roadmap automation moved this from Top 10 Inter-Team bugs to Done Jun 25, 2020
@liuche liuche mentioned this issue Jun 27, 2020
12 tasks
@data-sync-user data-sync-user changed the title When clicking URL bar, UI waits on long delay for on capturing thumbnail FNX2-16678 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail Aug 1, 2020
@data-sync-user data-sync-user changed the title FNX2-16678 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail FNX3-14937 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail Aug 11, 2020
Fenix Sprint Kanban automation moved this from Sprint 20.11 Done to In Progress Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX3-14937 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail FNX-12810 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail Aug 11, 2020
Fenix Sprint Kanban automation moved this from In Progress to Sprint 20.11 Done Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX-12810 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail FNX2-16678 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail Aug 11, 2020
@data-sync-user data-sync-user changed the title FNX2-16678 ⁃ When clicking URL bar, UI waits on long delay for on capturing thumbnail When clicking URL bar, UI waits on long delay for on capturing thumbnail May 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs:product needs:triage Issue needs triage performance Possible performance wins
Projects
Fenix Sprint Kanban
  
Sprint 20.11 Done
Development

No branches or pull requests

5 participants