commit 09c3a95a7e17f88972f1e616f6a90e4b8d2a2531
parent 4ac2da42d1c1d5eba26323dbe1d2ced0e7036e0d
Author: Dan Stillman <dstillman@zotero.org>
Date: Thu, 5 May 2016 04:34:19 -0400
Improve downloaded object processing
- Use an increasing notifier batch size, so objects initially appear one by one
but then start showing up in batches, up to 50. (UI updates are expensive, so
for larger syncs we don't want to update after each object.)
- Avoid separate save to update attachment file sync state, which was also
happening outside of notifier batches (causing individual updates regardless
of the batch size)
- Add a 10ms delay after processing each object, which keeps the UI responsive
during downloads. #989 could reduce this to 1 during idle, to save a few
minutes when downloading very large libraries.
Diffstat:
2 files changed, 69 insertions(+), 38 deletions(-)
diff --git a/chrome/content/zotero/xpcom/sync/syncEngine.js b/chrome/content/zotero/xpcom/sync/syncEngine.js
@@ -591,8 +591,9 @@ Zotero.Sync.Data.Engine.prototype._downloadObjects = Zotero.Promise.coroutine(fu
);
var conflicts = [];
+ var num = 0;
- // Process batches as soon as they're available
+ // Process batches when they're available, one at a time
yield Zotero.Promise.map(
json,
function (batch) {
@@ -618,9 +619,31 @@ Zotero.Sync.Data.Engine.prototype._downloadObjects = Zotero.Promise.coroutine(fu
objectType,
this.libraryID,
batch,
- this._getOptions()
+ this._getOptions({
+ onObjectProcessed: () => {
+ num++;
+ },
+ // Increase the notifier batch size as we go
+ getNotifierBatchSize: () => {
+ var size;
+ if (num < 10) {
+ size = 1;
+ }
+ else if (num < 50) {
+ size = 5;
+ }
+ else if (num < 150) {
+ size = 25;
+ }
+ else {
+ size = 50;
+ }
+ return Math.min(size, batch.length);
+ }
+ })
)
.then(function (results) {
+ num += results.length;
let processedKeys = [];
let conflictResults = [];
results.forEach(x => {
@@ -639,7 +662,10 @@ Zotero.Sync.Data.Engine.prototype._downloadObjects = Zotero.Promise.coroutine(fu
keys = Zotero.Utilities.arrayDiff(keys, processedKeys);
conflicts.push(...conflictResults);
}.bind(this));
- }.bind(this)
+ }.bind(this),
+ {
+ concurrency: 1
+ }
);
if (!keys.length || keys.length == lastLength) {
@@ -1433,9 +1459,12 @@ Zotero.Sync.Data.Engine.prototype._fullSync = Zotero.Promise.coroutine(function*
});
-Zotero.Sync.Data.Engine.prototype._getOptions = function () {
+Zotero.Sync.Data.Engine.prototype._getOptions = function (additionalOpts = {}) {
var options = {};
this.optionNames.forEach(x => options[x] = this[x]);
+ for (let opt in additionalOpts) {
+ options[opt] = additionalOpts[opt];
+ }
return options;
}
diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js
@@ -535,14 +535,19 @@ Zotero.Sync.Data.Local = {
});
}
- var batchSize = 10;
+ var batchSize = options.getNotifierBatchSize ? options.getNotifierBatchSize() : json.length;
var notifierQueues = [];
+
try {
for (let i = 0; i < json.length; i++) {
// Batch notifier updates
if (notifierQueues.length == batchSize) {
yield Zotero.Notifier.commit(notifierQueues);
notifierQueues = [];
+ // Get the current batch size, which might have increased
+ if (options.getNotifierBatchSize) {
+ batchSize = options.getNotifierBatchSize()
+ }
}
let notifierQueue = new Zotero.Notifier.Queue;
@@ -786,6 +791,13 @@ Zotero.Sync.Data.Local = {
throw e;
}
}
+ finally {
+ if (options.onObjectProcessed) {
+ options.onObjectProcessed();
+ }
+ }
+
+ yield Zotero.Promise.delay(10);
}
}
finally {
@@ -825,32 +837,10 @@ Zotero.Sync.Data.Local = {
},
- _onObjectProcessed: Zotero.Promise.coroutine(function* (obj, jsonObject, isNewObject, storageDetailsChanged) {
- var jsonData = jsonObject.data;
-
- // Delete older versions of the object in the cache
- yield this.deleteCacheObjectVersions(
- obj.objectType, obj.libraryID, jsonData.key, null, jsonData.version - 1
- );
-
- // Delete from sync queue
- yield this._removeObjectFromSyncQueue(obj.objectType, obj.libraryID, jsonData.key);
-
- // Mark updated attachments for download
- if (obj.objectType == 'item' && obj.isImportedAttachment()) {
- // If storage changes were made (attachment mtime or hash), mark
- // library as requiring download
- if (isNewObject || storageDetailsChanged) {
- Zotero.Libraries.get(obj.libraryID).storageDownloadNeeded = true;
- }
-
- yield this._checkAttachmentForDownload(
- obj, jsonData.mtime, isNewObject
- );
- }
- }),
-
-
+ /**
+ * Check whether an attachment's file mod time matches the given mod time, and mark the file
+ * for download if not (or if this is a new attachment)
+ */
_checkAttachmentForDownload: Zotero.Promise.coroutine(function* (item, mtime, isNewObject) {
var markToDownload = false;
if (!isNewObject) {
@@ -882,7 +872,6 @@ Zotero.Sync.Data.Local = {
}
if (markToDownload) {
item.attachmentSyncState = "to_download";
- yield item.save({ skipAll: true });
}
}),
@@ -956,7 +945,7 @@ Zotero.Sync.Data.Local = {
Zotero.debug("Processing resolved conflicts");
- let batchSize = 50;
+ let batchSize = mergeData.length;
let notifierQueues = [];
try {
for (let i = 0; i < mergeData.length; i++) {
@@ -1125,6 +1114,9 @@ Zotero.Sync.Data.Local = {
if (!options.skipData) {
obj.fromJSON(json.data);
}
+ if (obj.objectType == 'item' && obj.isImportedAttachment()) {
+ this._checkAttachmentForDownload(obj, json.data.mtime, options.isNewObject);
+ }
obj.version = json.data.version;
if (!options.saveAsChanged) {
obj.synced = true;
@@ -1143,12 +1135,22 @@ Zotero.Sync.Data.Local = {
yield this.saveCacheObject(obj.objectType, obj.libraryID, json.data);
results.processed = true;
- yield this._onObjectProcessed(
- obj,
- json,
- options.isNewObject,
- options.storageDetailsChanged
+ // Delete older versions of the object in the cache
+ yield this.deleteCacheObjectVersions(
+ obj.objectType, obj.libraryID, json.key, null, json.version - 1
);
+
+ // Delete from sync queue
+ yield this._removeObjectFromSyncQueue(obj.objectType, obj.libraryID, json.key);
+
+ // Mark updated attachments for download
+ if (obj.objectType == 'item' && obj.isImportedAttachment()) {
+ // If storage changes were made (attachment mtime or hash), mark
+ // library as requiring download
+ if (options.isNewObject || options.storageDetailsChanged) {
+ Zotero.Libraries.get(obj.libraryID).storageDownloadNeeded = true;
+ }
+ }
}
catch (e) {
// For now, allow sync to proceed after all errors
