From 320ac0901d200ee618c20dcc08f3dd2c8bddee30 Mon Sep 17 00:00:00 2001 From: "Joshua A. Horton" Date: Mon, 4 Mar 2019 09:01:34 +0700 Subject: [PATCH] Implements a shutdown() function on LMLayer, killing the worker. --- common/predictive-text/index.ts | 8 ++++++++ .../unit_tests/in_browser/cases/top-level-lmlayer.js | 2 ++ .../in_browser/cases/worker-dummy-integration.js | 1 + .../in_browser/cases/worker-wordlist-integration.js | 1 + .../unit_tests/in_browser/cases/worker.js | 1 + common/predictive-text/worker/index.ts | 11 +++++++++++ 6 files changed, 24 insertions(+) diff --git a/common/predictive-text/index.ts b/common/predictive-text/index.ts index 0c43d6bd4b..88ea5c1de1 100644 --- a/common/predictive-text/index.ts +++ b/common/predictive-text/index.ts @@ -113,6 +113,14 @@ namespace com.keyman.text.prediction { } } + /** + * Clears out any computational resources in use by the LMLayer, including shutting + * down any internal WebWorkers. + */ + public shutdown() { + this._worker.terminate(); + } + /** * Given a function, this utility returns the source code within it, as a string. * This is intended to unwrap the "wrapped" source code created in the LMLayerWorker diff --git a/common/predictive-text/unit_tests/in_browser/cases/top-level-lmlayer.js b/common/predictive-text/unit_tests/in_browser/cases/top-level-lmlayer.js index 2083e2e29c..a97e84b20d 100644 --- a/common/predictive-text/unit_tests/in_browser/cases/top-level-lmlayer.js +++ b/common/predictive-text/unit_tests/in_browser/cases/top-level-lmlayer.js @@ -6,6 +6,7 @@ describe('LMLayer', function () { it('should construct with zero arguments', function () { let lmLayer = new LMLayer(); assert.instanceOf(lmLayer, LMLayer); + lmLayer.shutdown(); }); }); @@ -25,6 +26,7 @@ describe('LMLayer', function () { let worker = new Worker(uri); worker.onmessage = function thisShouldBeCalled(event) { assert.propertyVal(event, 'data', 'fhqwhgads'); + worker.terminate(); done(); }; }) diff --git a/common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js b/common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js index 5137e042ad..360699bf84 100644 --- a/common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js +++ b/common/predictive-text/unit_tests/in_browser/cases/worker-dummy-integration.js @@ -35,6 +35,7 @@ describe('LMLayer using dummy model', function () { return lmLayer.predict(zeroTransform(), emptyContext()); }).then(function (suggestions) { assert.deepEqual(suggestions, iGotDistractedByHazel()[3]); + lmLayer.shutdown(); return Promise.resolve(); }); }); diff --git a/common/predictive-text/unit_tests/in_browser/cases/worker-wordlist-integration.js b/common/predictive-text/unit_tests/in_browser/cases/worker-wordlist-integration.js index a5e14af879..281c652e9f 100644 --- a/common/predictive-text/unit_tests/in_browser/cases/worker-wordlist-integration.js +++ b/common/predictive-text/unit_tests/in_browser/cases/worker-wordlist-integration.js @@ -35,6 +35,7 @@ describe('LMLayer using the word list model', function () { return lmLayer.predict(type('q'), atEndOfBuffer('the ')); }).then(function (suggestions) { assert.isAtLeast(suggestions.length, EXPECTED_SUGGESTIONS); + lmLayer.shutdown(); return Promise.resolve(); }); }); diff --git a/common/predictive-text/unit_tests/in_browser/cases/worker.js b/common/predictive-text/unit_tests/in_browser/cases/worker.js index be81f4c5ef..693967323b 100644 --- a/common/predictive-text/unit_tests/in_browser/cases/worker.js +++ b/common/predictive-text/unit_tests/in_browser/cases/worker.js @@ -17,6 +17,7 @@ describe('LMLayerWorker', function () { let worker = new Worker(uri); worker.onmessage = function thisShouldBeCalled(message) { done(); + worker.terminate(); }; worker.postMessage({ message: 'initialize', diff --git a/common/predictive-text/worker/index.ts b/common/predictive-text/worker/index.ts index 7aa85f1a60..6621cc818c 100644 --- a/common/predictive-text/worker/index.ts +++ b/common/predictive-text/worker/index.ts @@ -72,6 +72,9 @@ class LMLayerWorker { /** * By default, it's self.importScripts(), but can be overridden * so that this can be tested **outside of a Worker**. + * + * To function properly, self.importScripts() must be bound to self + * before being stored here, else it will fail. */ private _importScripts: ImportScripts; @@ -161,6 +164,13 @@ class LMLayerWorker { this._importScripts(url); } + public unloadModel() { + // Right now, this seems sufficient to clear out the old model. + // The only existing reference to a loaded model is held by + // transitionToReadyState's `handleMessage` closure. (The `model` var) + this.setupInitialState(); + } + /** * Sets the initial state, i.e., `uninitialized`. * This state only handles `initialized` messages, and will @@ -229,6 +239,7 @@ class LMLayerWorker { scope.onmessage = worker.onMessage.bind(worker); // Ensures that the worker instance is accessible for loaded model scripts. + // Assists unit-testing. scope['LMLayerWorker'] = worker; scope['models'] = models;