Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache repository refresh to reduce load when refreshing takes time #7601

Open
shiftkey opened this issue May 22, 2019 · 0 comments
Open

cache repository refresh to reduce load when refreshing takes time #7601

shiftkey opened this issue May 22, 2019 · 0 comments
Labels
performance Relating to things affecting performance tech-debt Issues and pull requests related to addressing technical debt or improving the codebase

Comments

@shiftkey
Copy link
Member

As part of investigating performance improvements, I noticed two cases where users may find the app running multiple refreshes. Here's a log to illustrate what this looks like with tracing enabled:

Two particular cases stood out around the user interacting with the app:

  • the initial launch of the app - 2 or 3 refreshes can be initiated, depending on the setup
  • switching back and forth between Desktop and another app - app focus will always trigger a refresh, irrespective of the current state of the repository (which might already be refreshing)

On systems with slow I/O or complex repositories (also Windows will see this more acutely), the overall refresh operation may take multiple seconds. Adding more in-flight refreshes will only slow down the first refresh, leading to a sort of "self denial of service" in the app if the user is able to trigger these continually by interacting with the app.

To reduce the impact of Desktop's refreshing work we could track the current refresh operation in a per-repository cache, and if a refresh is active then a new refresh should not be started.

For reference we did something similar in #7501 for the pull request refresh:

public refreshPullRequests(repo: GitHubRepository, account: Account) {
const dbId = forceUnwrap("Can't refresh PRs, no dbId", repo.dbID)
const currentOp = this.currentRefreshOperations.get(dbId)
if (currentOp !== undefined) {
return currentOp
}
this.lastRefreshForRepository.set(dbId, Date.now())
this.emitIsLoadingPullRequests(repo, true)
const promise = this.fetchAndStorePullRequests(repo, account)
.catch(err => {
log.error(`Error refreshing pull requests for '${repo.fullName}'`, err)
})
.then(() => {
this.currentRefreshOperations.delete(dbId)
this.emitIsLoadingPullRequests(repo, false)
})
this.currentRefreshOperations.set(dbId, promise)
return promise
}

@shiftkey shiftkey added tech-debt Issues and pull requests related to addressing technical debt or improving the codebase performance Relating to things affecting performance labels May 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Relating to things affecting performance tech-debt Issues and pull requests related to addressing technical debt or improving the codebase
Projects
None yet
Development

No branches or pull requests

2 participants
@shiftkey and others