From 59b5961a6c9727341afe54526b44d1365b94f20f Mon Sep 17 00:00:00 2001 From: Mark Hahnenberg Date: Wed, 11 Jan 2017 16:53:01 -0800 Subject: [PATCH] [syncback] Reuse currently open box if possible Summary: Previously we would unconditionally issue a SELECT when openBox was called. Now we check if the currently open box is the one we want first and return immediately if it is, avoiding the unnecessary SELECT (which can be quite expensive on large folders like INBOX). We were also calling closeBox after iterating all the messages in a thread to mark them as read/unread. This was unnecessary and was causing extra SELECTs to be issued. Now we don't! This diff is a 5x speedup over the old behavior when marking lots of threads in the same folder as read all at once. Test Plan: Run locally, measure perf with log statements Reviewers: evan, juan Reviewed By: evan, juan Differential Revision: https://phab.nylas.com/D3654 --- packages/isomorphic-core/src/imap-connection.js | 3 +++ packages/local-sync/src/local-sync-worker/sync-worker.es6 | 7 +++++-- .../src/local-sync-worker/syncback_tasks/task-helpers.js | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/isomorphic-core/src/imap-connection.js b/packages/isomorphic-core/src/imap-connection.js index 178039ede..2beb849df 100644 --- a/packages/isomorphic-core/src/imap-connection.js +++ b/packages/isomorphic-core/src/imap-connection.js @@ -196,6 +196,9 @@ class IMAPConnection extends EventEmitter { if (!this._imap) { throw new IMAPConnectionNotReadyError(`IMAPConnection::openBox`) } + if (folderName === this.getOpenBoxName()) { + return Promise.resolve(new IMAPBox(this, this._imap._box)); + } this._isOpeningBox = true return this.createConnectionPromise((resolve, reject) => { return this._imap.openBoxAsync(folderName, readOnly) diff --git a/packages/local-sync/src/local-sync-worker/sync-worker.es6 b/packages/local-sync/src/local-sync-worker/sync-worker.es6 index a8de1d6cc..d50efa45f 100644 --- a/packages/local-sync/src/local-sync-worker/sync-worker.es6 +++ b/packages/local-sync/src/local-sync-worker/sync-worker.es6 @@ -319,6 +319,7 @@ class SyncWorker { } async _runSyncbackTask(task) { + const before = new Date(); const syncbackRequest = task.syncbackRequestObject(); console.log(`🔃 📤 ${task.description()}`, syncbackRequest.props) try { @@ -330,11 +331,13 @@ class SyncWorker { const responseJSON = await this._conn.runOperation(task); syncbackRequest.status = "SUCCEEDED"; syncbackRequest.responseJSON = responseJSON || {}; - console.log(`🔃 📤 ${task.description()} Succeeded`) + const after = new Date(); + console.log(`🔃 📤 ${task.description()} Succeeded (${after.getTime() - before.getTime()}ms)`) } catch (error) { syncbackRequest.error = error; syncbackRequest.status = "FAILED"; - console.error(`🔃 📤 ${task.description()} Failed`, {syncbackRequest: syncbackRequest.toJSON()}) + const after = new Date(); + console.error(`🔃 📤 ${task.description()} Failed (${after.getTime() - before.getTime()}ms)`, {syncbackRequest: syncbackRequest.toJSON()}) } finally { await syncbackRequest.save(); } diff --git a/packages/local-sync/src/local-sync-worker/syncback_tasks/task-helpers.js b/packages/local-sync/src/local-sync-worker/syncback_tasks/task-helpers.js index 7436989af..d3435e4b6 100644 --- a/packages/local-sync/src/local-sync-worker/syncback_tasks/task-helpers.js +++ b/packages/local-sync/src/local-sync-worker/syncback_tasks/task-helpers.js @@ -18,7 +18,7 @@ const TaskHelpers = { imap.openBox(category.name, {readOnly: false}).then((box) => Promise.all(msgsInCategories[category.id].map((message) => callback({message, category, box}) - )).then(() => box.closeBox()) + )) ) ) })