Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Optimize check for updates #9996

Closed
bbondy opened this issue Jul 14, 2017 · 2 comments
Closed

Optimize check for updates #9996

bbondy opened this issue Jul 14, 2017 · 2 comments
Assignees
Labels
needs-info Another team member needs information from the PR/issue opener. perf release-notes/include

Comments

@bbondy
Copy link
Member

bbondy commented Jul 14, 2017

Checking for updates converts Immutable app state to JS several times! That is a slow operation.

This should just do a lookup in immutableJS at least as a quick win. This would explain some beach-balling we get periodically.

  // Get the daily, week of year and month update checks
  var lastCheckYMD = AppStore.getState().toJS().updates['lastCheckYMD'] || null
  debug(`lastCheckYMD = ${lastCheckYMD}`)

  var lastCheckWOY = AppStore.getState().toJS().updates['lastCheckWOY'] || null
  debug(`lastCheckWOY = ${lastCheckWOY}`)

  var lastCheckMonth = AppStore.getState().toJS().updates['lastCheckMonth'] || null
  debug(`lastCheckMonth = ${lastCheckMonth}`)

  // Has the browser ever asked for an update
  var firstCheckMade = AppStore.getState().toJS().updates['firstCheckMade'] || false
  debug(`firstCheckMade = ${firstCheckMade}`)

screen shot 2017-07-14 at 5 45 15 pm

Test plan

Very important to test the update test from the manual test plan to set your version back and make sure it updates.

Also check the update log and make sure it has valid values for things like the last check time.

@bbondy bbondy added this to the 0.18.x (Release Channel) milestone Jul 14, 2017
@bsclifton bsclifton added the perf label Jul 15, 2017
bbondy added a commit that referenced this issue Jul 15, 2017
It could cause periodic jank after the first 2 minutes, and then again every hour after that

Fix #9996
@bbondy bbondy mentioned this issue Jul 15, 2017
8 tasks
@bbondy bbondy modified the milestones: 0.18.x (Release Channel), 0.17.17 (Release Channel) Jul 17, 2017
@bbondy bbondy self-assigned this Jul 17, 2017
bbondy added a commit that referenced this issue Jul 17, 2017
It could cause periodic jank after the first 2 minutes, and then again every hour after that

Fix #9996
@bsclifton
Copy link
Member

@bbondy any manual QA needed? If so please add steps; if not, please add QA/no-qa-required tag. Thanks 😄

@srirambv
Copy link
Collaborator

Verified manually on a clean profile on 0.17.16 with preview check on and off.

Log for the first check (with preview off)

2017-07-18T08:04:26.558Z - platformBaseUrl = https://brave-laptop-updates.global.ssl.fastly.net/1/releases/dev/0.8.3/winx64
2017-07-18T08:04:26.558Z - updateUrl = https://brave-download.global.ssl.fastly.net/multi-channel/releases/dev/winx64
2017-07-18T08:04:48.348Z - checkForUpdates
2017-07-18T08:04:48.348Z - lastCheckYMD = null
2017-07-18T08:04:48.348Z - lastCheckWOY = null
2017-07-18T08:04:48.384Z - lastCheckMonth = null
2017-07-18T08:04:48.384Z - firstCheckMade = false
2017-07-18T08:04:48.384Z - https://brave-laptop-updates.global.ssl.fastly.net/1/releases/dev/0.8.3/winx64?
2017-07-18T08:04:48.654Z - Metadata: {"version":"0.17.16","name":"Brave 0.17.16","pub_date":"2017-07-12T07:13:05.994Z","notes":"Fixed sync

Log after preview check on

2017-07-18T08:05:52.941Z - platformBaseUrl = https://brave-laptop-updates.global.ssl.fastly.net/1/releases/dev/0.8.3/winx64
2017-07-18T08:05:52.941Z - updateUrl = https://brave-download.global.ssl.fastly.net/multi-channel/releases/dev/winx64
2017-07-18T08:06:04.191Z - checkForUpdates
2017-07-18T08:06:04.191Z - lastCheckYMD = 2017-07-18
2017-07-18T08:06:04.207Z - lastCheckWOY = 195
2017-07-18T08:06:04.207Z - lastCheckMonth = 7
2017-07-18T08:06:04.207Z - firstCheckMade = true
2017-07-18T08:06:04.207Z - https://brave-laptop-updates.global.ssl.fastly.net/1/releases/dev/0.8.3/winx64?daily=false&weekly=false&monthly=false&first=false&accept_preview=true
2017-07-18T08:06:04.521Z - Metadata: {"version":"0.17.17","name":"Brave 0.17.17","pub_date":"2017-07-17T21:31:17.961Z","notes":"More details:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-info Another team member needs information from the PR/issue opener. perf release-notes/include
Projects
None yet
Development

No branches or pull requests

5 participants