From 04995ffe8cf30a82a698b3a39db12b546d3ed43e Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Wed, 27 Nov 2019 11:28:09 -0500 Subject: [PATCH 1/3] add queuing for getResource calls --- src/util/actor.js | 54 ++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 26 deletions(-) diff --git a/src/util/actor.js b/src/util/actor.js index 940a0030f0b..659fb4e4c25 100644 --- a/src/util/actor.js +++ b/src/util/actor.js @@ -110,40 +110,42 @@ class Actor { cancel(); } } else { - // In workers, store the tasks that we need to process before actually processing them. This - // is necessary because we want to keep receiving messages, and in particular, - // messages. Some tasks may take a while in the worker thread, so before - // executing the next task in our queue, postMessage preempts this and - // messages can be processed. We're using a MessageChannel object to get throttle the - // process() flow to one at a time. - this.tasks[id] = data; - this.taskQueue.push(id); - if (isWorker()) { + if (isWorker() || data.type === 'getResource') { + // In workers, store the tasks that we need to process before actually processing them. This + // is necessary because we want to keep receiving messages, and in particular, + // messages. Some tasks may take a while in the worker thread, so before + // executing the next task in our queue, postMessage preempts this and + // messages can be processed. We're using a MessageChannel object to get throttle the + // process() flow to one at a time. + this.tasks[id] = data; + this.taskQueue.push(id); this.invoker.trigger(); } else { // In the main thread, process messages immediately so that other work does not slip in // between getting partial data back from workers. - this.process(); + this.process(id, data); } } } - process() { - if (!this.taskQueue.length) { - return; - } - const id = this.taskQueue.shift(); - const task = this.tasks[id]; - delete this.tasks[id]; - // Schedule another process call if we know there's more to process _before_ invoking the - // current task. This is necessary so that processing continues even if the current task - // doesn't execute successfully. - if (this.taskQueue.length) { - this.invoker.trigger(); - } - if (!task) { - // If the task ID doesn't have associated task data anymore, it was canceled. - return; + process(id: number, task: any) { + if (id === undefined && task === undefined) { + if (!this.taskQueue.length) { + return; + } + id = this.taskQueue.shift(); + task = this.tasks[id]; + delete this.tasks[id]; + // Schedule another process call if we know there's more to process _before_ invoking the + // current task. This is necessary so that processing continues even if the current task + // doesn't execute successfully. + if (this.taskQueue.length) { + this.invoker.trigger(); + } + if (!task) { + // If the task ID doesn't have associated task data anymore, it was canceled. + return; + } } if (task.type === '') { From d28ac5668f35d9354b9ae1fc7dfbb15b9296aec7 Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Wed, 27 Nov 2019 13:10:47 -0500 Subject: [PATCH 2/3] split methods, unharcode --- src/util/actor.js | 45 ++++++++++++++++++++++++--------------------- src/util/ajax.js | 3 ++- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/util/actor.js b/src/util/actor.js index 659fb4e4c25..df9ec3412cb 100644 --- a/src/util/actor.js +++ b/src/util/actor.js @@ -53,7 +53,7 @@ class Actor { * @param targetMapId A particular mapId to which to send this message. * @private */ - send(type: string, data: mixed, callback: ?Function, targetMapId: ?string): ?Cancelable { + send(type: string, data: mixed, callback: ?Function, targetMapId: ?string, mustQueue: ?string): ?Cancelable { // We're using a string ID instead of numbers because they are being used as object keys // anyway, and thus stringified implicitly. We use random IDs because an actor may receive // message from multiple other actors which could run in different execution context. A @@ -68,6 +68,7 @@ class Actor { type, hasCallback: !!callback, targetMapId, + mustQueue, sourceMapId: this.mapId, data: serialize(data, buffers) }, buffers); @@ -110,7 +111,7 @@ class Actor { cancel(); } } else { - if (isWorker() || data.type === 'getResource') { + if (isWorker() || data.mustQueue) { // In workers, store the tasks that we need to process before actually processing them. This // is necessary because we want to keep receiving messages, and in particular, // messages. Some tasks may take a while in the worker thread, so before @@ -123,31 +124,33 @@ class Actor { } else { // In the main thread, process messages immediately so that other work does not slip in // between getting partial data back from workers. - this.process(id, data); + this.processTask(id, data); } } } - process(id: number, task: any) { - if (id === undefined && task === undefined) { - if (!this.taskQueue.length) { - return; - } - id = this.taskQueue.shift(); - task = this.tasks[id]; - delete this.tasks[id]; - // Schedule another process call if we know there's more to process _before_ invoking the - // current task. This is necessary so that processing continues even if the current task - // doesn't execute successfully. - if (this.taskQueue.length) { - this.invoker.trigger(); - } - if (!task) { - // If the task ID doesn't have associated task data anymore, it was canceled. - return; - } + process() { + if (!this.taskQueue.length) { + return; } + const id = this.taskQueue.shift(); + const task = this.tasks[id]; + delete this.tasks[id]; + // Schedule another process call if we know there's more to process _before_ invoking the + // current task. This is necessary so that processing continues even if the current task + // doesn't execute successfully. + if (this.taskQueue.length) { + this.invoker.trigger(); + } + if (!task) { + // If the task ID doesn't have associated task data anymore, it was canceled. + return; + } + + this.processTask(id, task); + } + processTask(id: number, task: any) { if (task.type === '') { // The done() function in the counterpart has been called, and we are now // firing the callback in the originating actor, if there is one. diff --git a/src/util/ajax.js b/src/util/ajax.js index 3c3e29605aa..38189d4ef77 100644 --- a/src/util/ajax.js +++ b/src/util/ajax.js @@ -230,7 +230,8 @@ export const makeRequest = function(requestParameters: RequestParameters, callba return makeFetchRequest(requestParameters, callback); } if (isWorker() && self.worker && self.worker.actor) { - return self.worker.actor.send('getResource', requestParameters, callback); + const queueOnMainThread = true; + return self.worker.actor.send('getResource', requestParameters, callback, undefined, queueOnMainThread); } } return makeXMLHttpRequest(requestParameters, callback); From 1028521eae756ae638a02fa8ec7423e0d8cfc43d Mon Sep 17 00:00:00 2001 From: Ansis Brammanis Date: Wed, 27 Nov 2019 14:04:36 -0500 Subject: [PATCH 3/3] fix type --- src/util/actor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/actor.js b/src/util/actor.js index df9ec3412cb..ac441726c41 100644 --- a/src/util/actor.js +++ b/src/util/actor.js @@ -53,7 +53,7 @@ class Actor { * @param targetMapId A particular mapId to which to send this message. * @private */ - send(type: string, data: mixed, callback: ?Function, targetMapId: ?string, mustQueue: ?string): ?Cancelable { + send(type: string, data: mixed, callback: ?Function, targetMapId: ?string, mustQueue: boolean = false): ?Cancelable { // We're using a string ID instead of numbers because they are being used as object keys // anyway, and thus stringified implicitly. We use random IDs because an actor may receive // message from multiple other actors which could run in different execution context. A