Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Issue #8535, #7304: Prevent dispatching individual actions to restore tabs #8589

Merged
merged 2 commits into from
Oct 2, 2020

Conversation

csadilek
Copy link
Contributor

@csadilek csadilek commented Oct 1, 2020

First draft to validate performance impact. Instead of dispatching individual actions to update the state of the restored tab we make sure that the initial state is correct, based on the Snapshot.

RestoreAction is already a "bulk action", but tabs didn't include the full initial state and subsequent actions were required, which is fixed here.

Copy link
Contributor

@jonalmeida jonalmeida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a few samples with this patch on Fenix having 150 open tabs. Originally, SessionManager.restore took ~300ms.

We're now seeing much lower numbers (in milliseconds): 37.9, 30.4, 45.7, 32.8

This averages to 36.7ms.

@csadilek csadilek marked this pull request as ready for review October 2, 2020 14:32
@csadilek
Copy link
Contributor Author

csadilek commented Oct 2, 2020

@pocmo @jonalmeida I added a commit to here to introduce a state flag to indicate that restore is complete. Not sure if the name is ideal? We only need to set this to true once and for now applications should dispatch the action. Later, we can introduce a RestoreUseCase.

Copy link
Contributor

@pocmo pocmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That'll do. 👍

@csadilek csadilek added the 🛬 needs landing PRs that are ready to land label Oct 2, 2020
@mergify mergify bot merged commit 277e5ae into mozilla-mobile:master Oct 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🛬 needs landing PRs that are ready to land
Projects
None yet
3 participants