From 46e138a3767765d6dc98966c55519b095a93be7d Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 21 Sep 2016 11:46:00 -0700 Subject: [PATCH 1/4] state: Use debounce to throttle saves --- package.json | 1 + src/renderer/lib/state.js | 15 ++++----------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 20efee83..785ada4d 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "capture-frame": "^1.0.0", "chromecasts": "^1.8.0", "create-torrent": "^3.24.5", + "debounce": "^1.0.0", "deep-equal": "^1.0.1", "dlnacasts": "^0.1.0", "drag-drop": "^2.12.1", diff --git a/src/renderer/lib/state.js b/src/renderer/lib/state.js index 2ea67df1..6b20da97 100644 --- a/src/renderer/lib/state.js +++ b/src/renderer/lib/state.js @@ -1,15 +1,18 @@ const appConfig = require('application-config')('WebTorrent') +const debounce = require('debounce') const path = require('path') const {EventEmitter} = require('events') const config = require('../../config') const migrations = require('./migrations') +const SAVE_THROTTLED_INTERVAL = 1000 + const State = module.exports = Object.assign(new EventEmitter(), { getDefaultPlayState, load, save, - saveThrottled + saveThrottled: debounce(save, SAVE_THROTTLED_INTERVAL) }) appConfig.filePath = path.join(config.CONFIG_PATH, 'config.json') @@ -199,7 +202,6 @@ function load (cb) { // Write state.saved to the JSON state file function save (state, cb) { console.log('Saving state to ' + appConfig.filePath) - delete state.saveStateTimeout // Clean up, so that we're not saving any pending state const copy = Object.assign({}, state.saved) @@ -229,12 +231,3 @@ function save (state, cb) { else State.emit('savedState') }) } - -// Write, but no more than once a second -function saveThrottled (state) { - if (state.saveStateTimeout) return - state.saveStateTimeout = setTimeout(function () { - if (!state.saveStateTimeout) return - save(state) - }, 1000) -} From 167da9dfd51a2acc0b6ae3c89bd564bc0fe74a22 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 21 Sep 2016 11:46:41 -0700 Subject: [PATCH 2/4] Double wait time until quit On my modern Macbook 12" I've run into "Saving state took too long. Quitting.". We have users with spinning disk drives, so let's be a bit more generous. --- src/main/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/index.js b/src/main/index.js index c82d8846..45b2c015 100644 --- a/src/main/index.js +++ b/src/main/index.js @@ -107,7 +107,7 @@ function init () { setTimeout(() => { console.error('Saving state took too long. Quitting.') app.quit() - }, 2000) // quit after 2 secs, at most + }, 4000) // quit after 4 secs, at most }) app.on('activate', function () { From 1e05487acd05b914ef24a530e21a8aea7661ef2c Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 21 Sep 2016 11:48:23 -0700 Subject: [PATCH 3/4] state: Use dispatch instead of direct call --- src/renderer/controllers/prefs-controller.js | 3 +-- src/renderer/controllers/torrent-list-controller.js | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/renderer/controllers/prefs-controller.js b/src/renderer/controllers/prefs-controller.js index b85578f4..7681cb8d 100644 --- a/src/renderer/controllers/prefs-controller.js +++ b/src/renderer/controllers/prefs-controller.js @@ -1,4 +1,3 @@ -const State = require('../lib/state') const {dispatch} = require('../lib/dispatcher') const ipcRenderer = require('electron').ipcRenderer @@ -56,7 +55,7 @@ module.exports = class PrefsController { ipcRenderer.send('setStartup', state.unsaved.prefs.startup) } state.saved.prefs = Object.assign(state.saved.prefs || {}, state.unsaved.prefs) - State.save(state) + dispatch('saveState') dispatch('checkDownloadPath') } } diff --git a/src/renderer/controllers/torrent-list-controller.js b/src/renderer/controllers/torrent-list-controller.js index 8bfc2b48..5e4668a2 100644 --- a/src/renderer/controllers/torrent-list-controller.js +++ b/src/renderer/controllers/torrent-list-controller.js @@ -4,7 +4,6 @@ const electron = require('electron') const {dispatch} = require('../lib/dispatcher') const {TorrentKeyNotFoundError} = require('../lib/errors') -const State = require('../lib/state') const sound = require('../lib/sound') const TorrentSummary = require('../lib/torrent-summary') @@ -159,7 +158,7 @@ module.exports = class TorrentListController { // remove torrent from saved list this.state.saved.torrents.splice(index, 1) - State.saveThrottled(this.state) + dispatch('saveStateThrottled') } // prevent user from going forward to a deleted torrent From 7c158e9f2c573c62ddf4b83966e07b9be629ec67 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 21 Sep 2016 11:59:23 -0700 Subject: [PATCH 4/4] Rename events to be consistent - Make State.save() always throttle calls -- since that's the common case. - Immediate saves are now the exception, with State.saveImmediate(). - The function is called State.save(), so the dispatch event should be 'stateSave'. --- src/main/index.js | 4 ++-- src/renderer/controllers/prefs-controller.js | 2 +- src/renderer/controllers/torrent-controller.js | 6 +++--- .../controllers/torrent-list-controller.js | 2 +- src/renderer/controllers/update-controller.js | 4 ++-- src/renderer/lib/state.js | 15 ++++++++------- src/renderer/main.js | 8 ++++---- 7 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/main/index.js b/src/main/index.js index 45b2c015..5d459ad4 100644 --- a/src/main/index.js +++ b/src/main/index.js @@ -102,8 +102,8 @@ function init () { app.isQuitting = true e.preventDefault() - windows.main.dispatch('saveState') // try to save state on exit - ipcMain.once('savedState', () => app.quit()) + windows.main.dispatch('stateSaveImmediate') // try to save state on exit + ipcMain.once('stateSaved', () => app.quit()) setTimeout(() => { console.error('Saving state took too long. Quitting.') app.quit() diff --git a/src/renderer/controllers/prefs-controller.js b/src/renderer/controllers/prefs-controller.js index 7681cb8d..d2773f1a 100644 --- a/src/renderer/controllers/prefs-controller.js +++ b/src/renderer/controllers/prefs-controller.js @@ -55,7 +55,7 @@ module.exports = class PrefsController { ipcRenderer.send('setStartup', state.unsaved.prefs.startup) } state.saved.prefs = Object.assign(state.saved.prefs || {}, state.unsaved.prefs) - dispatch('saveState') + dispatch('stateSaveImmediate') dispatch('checkDownloadPath') } } diff --git a/src/renderer/controllers/torrent-controller.js b/src/renderer/controllers/torrent-controller.js index 7e4ee824..73b6757d 100644 --- a/src/renderer/controllers/torrent-controller.js +++ b/src/renderer/controllers/torrent-controller.js @@ -129,20 +129,20 @@ module.exports = class TorrentController { const torrentSummary = this.getTorrentSummary(torrentKey) if (!torrentSummary) throw new Error('Not saving modtimes for deleted torrent ' + torrentKey) torrentSummary.fileModtimes = fileModtimes - dispatch('saveStateThrottled') + dispatch('stateSave') } torrentFileSaved (torrentKey, torrentFileName) { console.log('torrent file saved %s: %s', torrentKey, torrentFileName) const torrentSummary = this.getTorrentSummary(torrentKey) torrentSummary.torrentFileName = torrentFileName - dispatch('saveStateThrottled') + dispatch('stateSave') } torrentPosterSaved (torrentKey, posterFileName) { const torrentSummary = this.getTorrentSummary(torrentKey) torrentSummary.posterFileName = posterFileName - dispatch('saveStateThrottled') + dispatch('stateSave') } torrentAudioMetadata (infoHash, index, info) { diff --git a/src/renderer/controllers/torrent-list-controller.js b/src/renderer/controllers/torrent-list-controller.js index 5e4668a2..1bb8ac71 100644 --- a/src/renderer/controllers/torrent-list-controller.js +++ b/src/renderer/controllers/torrent-list-controller.js @@ -158,7 +158,7 @@ module.exports = class TorrentListController { // remove torrent from saved list this.state.saved.torrents.splice(index, 1) - dispatch('saveStateThrottled') + dispatch('stateSave') } // prevent user from going forward to a deleted torrent diff --git a/src/renderer/controllers/update-controller.js b/src/renderer/controllers/update-controller.js index a5fd3d74..f7773edc 100644 --- a/src/renderer/controllers/update-controller.js +++ b/src/renderer/controllers/update-controller.js @@ -1,4 +1,4 @@ -const State = require('../lib/state') +const {dispatch} = require('../lib/dispatcher') // Controls the UI checking for new versions of the app, prompting install module.exports = class UpdateController { @@ -21,6 +21,6 @@ module.exports = class UpdateController { let skipped = this.state.saved.skippedVersions if (!skipped) skipped = this.state.saved.skippedVersions = [] skipped.push(version) - State.saveThrottled(this.state) + dispatch('stateSave') } } diff --git a/src/renderer/lib/state.js b/src/renderer/lib/state.js index 6b20da97..6e37ff81 100644 --- a/src/renderer/lib/state.js +++ b/src/renderer/lib/state.js @@ -6,13 +6,14 @@ const {EventEmitter} = require('events') const config = require('../../config') const migrations = require('./migrations') -const SAVE_THROTTLED_INTERVAL = 1000 +const SAVE_DEBOUNCE_INTERVAL = 1000 const State = module.exports = Object.assign(new EventEmitter(), { getDefaultPlayState, load, - save, - saveThrottled: debounce(save, SAVE_THROTTLED_INTERVAL) + // state.save() calls are rate-limited. Use state.saveImmediate() to skip limit. + save: debounce(saveImmediate, SAVE_DEBOUNCE_INTERVAL), + saveImmediate }) appConfig.filePath = path.join(config.CONFIG_PATH, 'config.json') @@ -101,7 +102,7 @@ function getDefaultPlayState () { } /* If the saved state file doesn't exist yet, here's what we use instead */ -function setupSavedState (cb) { +function setupStateSaved (cb) { const fs = require('fs-extra') const parseTorrent = require('parse-torrent') const parallel = require('run-parallel') @@ -185,7 +186,7 @@ function load (cb) { appConfig.read(function (err, saved) { if (err || !saved.version) { console.log('Missing config file: Creating new one') - setupSavedState(onSaved) + setupStateSaved(onSaved) } else { onSaved(null, saved) } @@ -200,7 +201,7 @@ function load (cb) { } // Write state.saved to the JSON state file -function save (state, cb) { +function saveImmediate (state, cb) { console.log('Saving state to ' + appConfig.filePath) // Clean up, so that we're not saving any pending state @@ -228,6 +229,6 @@ function save (state, cb) { appConfig.write(copy, (err) => { if (err) console.error(err) - else State.emit('savedState') + else State.emit('stateSaved') }) } diff --git a/src/renderer/main.js b/src/renderer/main.js index d0104e31..4407abb9 100644 --- a/src/renderer/main.js +++ b/src/renderer/main.js @@ -257,8 +257,8 @@ const dispatchHandlers = { 'onOpen': onOpen, 'error': onError, 'uncaughtError': (proc, err) => telemetry.logUncaughtError(proc, err), - 'saveState': () => State.save(state), - 'saveStateThrottled': () => State.saveThrottled(state), + 'stateSave': () => State.save(state), + 'stateSaveImmediate': () => State.saveImmediate(state), 'update': () => {} // No-op, just trigger an update } @@ -308,7 +308,7 @@ function setupIpc () { ipcRenderer.send('ipcReady') - State.on('savedState', () => ipcRenderer.send('savedState')) + State.on('stateSaved', () => ipcRenderer.send('stateSaved')) } // Quits any modal popovers and returns to the torrent list screen @@ -465,7 +465,7 @@ function onFullscreenChanged (e, isFullScreen) { function onWindowBoundsChanged (e, newBounds) { state.saved.bounds = newBounds - dispatch('saveStateThrottled') + dispatch('stateSave') } function checkDownloadPath () {