From dce21dc9fae0437c4451dde1b4d7b2a3a43c86b2 Mon Sep 17 00:00:00 2001 From: Tai Chou Date: Mon, 22 Mar 2021 11:33:18 -0700 Subject: [PATCH 1/7] Update orch package --- Composer/packages/server/package.json | 2 +- Composer/yarn.lock | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Composer/packages/server/package.json b/Composer/packages/server/package.json index 85d2b63071..949627e6d0 100644 --- a/Composer/packages/server/package.json +++ b/Composer/packages/server/package.json @@ -82,7 +82,7 @@ "@microsoft/bf-dispatcher": "^4.11.0-beta.20201016.393c6b2", "@microsoft/bf-generate-library": "^4.10.0-daily.20210225.217555", "@microsoft/bf-lu": "4.12.0-rc0", - "@microsoft/bf-orchestrator": "4.12.0-beta.20210316.cdd0819", + "@microsoft/bf-orchestrator": "4.13.0-beta.20210316.e8ec340", "applicationinsights": "^1.8.7", "archiver": "^5.0.2", "axios": "^0.21.1", diff --git a/Composer/yarn.lock b/Composer/yarn.lock index bbe9f76e9c..f0a577f57d 100644 --- a/Composer/yarn.lock +++ b/Composer/yarn.lock @@ -3947,10 +3947,10 @@ tslib "^2.0.3" xml2js "^0.4.19" -"@microsoft/bf-dispatcher@4.12.0-beta.20210316.cdd0819": - version "4.12.0-beta.20210316.cdd0819" - resolved "https://registry.yarnpkg.com/@microsoft/bf-dispatcher/-/bf-dispatcher-4.12.0-beta.20210316.cdd0819.tgz#454a612d4e17edc964675259be84ccc1c51425b4" - integrity sha512-TzkkoUTIITiBjwB8+UIjle9PCRJH8jolczOTEQszCDB3t7twfs54tLXzTmNVBS/bP9EAgZ+TY4xzd8IhoK/XdQ== +"@microsoft/bf-dispatcher@4.13.0-beta.20210316.e8ec340": + version "4.13.0-beta.20210316.e8ec340" + resolved "https://registry.yarnpkg.com/@microsoft/bf-dispatcher/-/bf-dispatcher-4.13.0-beta.20210316.e8ec340.tgz#de2024f41b217c0aa937807855ff519ea1dc370f" + integrity sha512-meLDe5MKbXdnPYBy+pXHhH6k/cdGs7zW3Vw9Powf+OfX+oBEgxo8xlfa/J1rIBsleIiGWU9reU+D6gKK/16Uyg== dependencies: "@microsoft/bf-lu" next "@oclif/command" "~1.5.19" @@ -4057,12 +4057,12 @@ semver "^5.5.1" tslib "^2.0.3" -"@microsoft/bf-orchestrator@4.12.0-beta.20210316.cdd0819": - version "4.12.0-beta.20210316.cdd0819" - resolved "https://registry.yarnpkg.com/@microsoft/bf-orchestrator/-/bf-orchestrator-4.12.0-beta.20210316.cdd0819.tgz#1869942269909759eb81e8ce9a538019392c2def" - integrity sha512-1jIYXtw+x6mm7cM0iCunf/hM7d+xrI5D3UnVMUrILK8t2oByuLyj7NUyt7oSrSmlAwhEAtYGJ/5xpdqdIxKw0Q== +"@microsoft/bf-orchestrator@4.13.0-beta.20210316.e8ec340": + version "4.13.0-beta.20210316.e8ec340" + resolved "https://registry.yarnpkg.com/@microsoft/bf-orchestrator/-/bf-orchestrator-4.13.0-beta.20210316.e8ec340.tgz#74ae2b91cedc292e19ac33d3f974d1d4c70874ad" + integrity sha512-WdWKQkOev4mX1bdk002UsD3DU9jdQms0X4c+wyawgX8XEWFJSNT42EKfKea9jbP5DtiXhwRjzeaDIOpuTgHIFw== dependencies: - "@microsoft/bf-dispatcher" "4.12.0-beta.20210316.cdd0819" + "@microsoft/bf-dispatcher" "4.13.0-beta.20210316.e8ec340" "@microsoft/bf-lu" next "@types/fs-extra" "~8.1.0" "@types/node-fetch" "~2.5.5" From 56de6b976a249afb50b8c319f5036ed501ce7a4e Mon Sep 17 00:00:00 2001 From: Tai Chou Date: Mon, 22 Mar 2021 20:22:09 -0700 Subject: [PATCH 2/7] initial implementation --- .../packages/server/src/models/bot/builder.ts | 2 + .../models/bot/process/orchestratorBuilder.ts | 11 ++ .../models/bot/process/orchestratorWorker.ts | 111 +++++++++++++++++- .../server/src/models/bot/process/types.ts | 2 +- 4 files changed, 122 insertions(+), 4 deletions(-) diff --git a/Composer/packages/server/src/models/bot/builder.ts b/Composer/packages/server/src/models/bot/builder.ts index bda9eb69fe..c703d723a2 100644 --- a/Composer/packages/server/src/models/bot/builder.ts +++ b/Composer/packages/server/src/models/bot/builder.ts @@ -102,6 +102,8 @@ export class Builder { setEnvDefault('QNA_USER_AGENT', userAgent); try { + //warm up the orchestrator cache if we're using it, before deleting and recreating the generated folder + await orchestratorBuilder.warmupCache(this.botDir, this.generatedFolderPath); await this.createGeneratedDir(); //do cross train before publish await this.crossTrain(luFiles, qnaFiles, allFiles); diff --git a/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts b/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts index 1743c3dbfb..93484e7437 100644 --- a/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts +++ b/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts @@ -33,6 +33,17 @@ class OrchestratorBuilder { }); } + public async warmupCache(projectId: string, generatedFolderPath: string) { + const msgId = uniqueId(); + const msg = { id: msgId, payload: { type: 'warmup', projectId, generatedFolderPath } }; + + return new Promise((resolve, reject) => { + this.resolves[msgId] = resolve; + this.rejects[msgId] = reject; + OrchestratorBuilder.worker.send(msg); + }); + } + // Handle incoming calculation result public handleMsg(msg: ResponseMsg) { const { id, error, payload } = msg; diff --git a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts index 7760be7c39..1f3bc7a66f 100644 --- a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts +++ b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts @@ -3,10 +3,11 @@ import { FileInfo } from '@bfc/shared'; import { LabelResolver, Orchestrator } from '@microsoft/bf-orchestrator'; -import { writeFile } from 'fs-extra'; +import { writeFile, readdir, readFile, pathExists, readJson } from 'fs-extra'; +import { partition } from 'lodash'; import { Path } from '../../../utility/path'; -import { IOrchestratorBuildOutput } from '../interface'; +import { IOrchestratorBuildOutput, IOrchestratorSettings } from '../interface'; import { RequestMsg } from './types'; @@ -28,6 +29,105 @@ export class LabelResolversCache { } const cache = new LabelResolversCache(); + +/** + * Orchestrator: Warm up the LabelResolversCache if .blu files already exist. + * + * The Orchestrator build process is iterative - the results of every build are cached, and the cache + * is used in subsequent builds to reduce the number of utterance embeddings that have to be re-calculated. + * + * However, if a user starts a new session of Composer and reopens the same bot project, + * the caches will be empty and training will begin from scratch again. + * + * If a user has ever built a bot with Orchestrator, embeddings (in the form of .blu files) for each + * utterance will be stored in the /generated folder. + * + * We warm up the LabelResolversCache with these blu files and pass this cache to the normal build + * process. Re-hydrating the cache from files is still cheaper than recalculating the embeddings from scratch. + * + * @param projectId + * @param modelPath + * @param storage + * @param generatedFolderPath + */ +export async function warmUpCache(generatedFolderPath: string, projectId: string) { + //warm up the cache only if it's empty... + if (cache.get(projectId).size == 0) { + if (!(await pathExists(generatedFolderPath))) { + return false; + } + + const bluFiles = (await readdir(generatedFolderPath)).filter((fileName) => fileName.endsWith('.blu')); + + //if there are blu file in the generated folders to hydrate with... + if (bluFiles.length == 0) { + return false; + } + + const orchestratorSettingsPath = Path.resolve(generatedFolderPath, 'orchestrator.settings.json'); + if (!(await pathExists(orchestratorSettingsPath))) { + return false; + } + + //todo - typeguards and safety checks needed + const orchestratorSettings: IOrchestratorSettings = await readJson(orchestratorSettingsPath); + + let [en_files, multilang_files] = partition(bluFiles, (f) => f.split('.')?.[1].startsWith('en')); + + let enLabelResolvers: Map = new Map(); + + const enSnapShotData = await Promise.all( + en_files.map( + async (f) => + [f.replace('.blu', '.lu'), new Uint8Array(await readFile(Path.join(generatedFolderPath, f)))] as [ + string, + Uint8Array + ] + ) + ); + + if (orchestratorSettings.orchestrator?.models?.en) { + try { + enLabelResolvers = await Orchestrator.getLabelResolversAsync( + orchestratorSettings.orchestrator.models.en, + '', + new Map(enSnapShotData), + false + ); + } catch (err) {} + } + + //todo: reduce code duplication + const multilangSnapShotData = await Promise.all( + multilang_files.map( + async (f) => + [f.replace('.blu', '.lu'), new Uint8Array(await readFile(Path.join(generatedFolderPath, f)))] as [ + string, + Uint8Array + ] + ) + ); + + let multiLangLabelResolvers: Map = new Map(); + + if (orchestratorSettings.orchestrator?.models?.multilang) { + try { + multiLangLabelResolvers = await Orchestrator.getLabelResolversAsync( + orchestratorSettings.orchestrator.models.multilang, + '', + new Map(multilangSnapShotData), + false + ); + } catch (err) {} + } + + cache.set(projectId, new Map([...enLabelResolvers, ...multiLangLabelResolvers])); + + return true; + } + return false; +} + /** * Orchestrator: Build command to compile .lu files into Binary LU (.blu) snapshots. * @@ -39,7 +139,6 @@ const cache = new LabelResolversCache(); * @param fullEmbedding - Use larger embeddings and skip size optimization (default: false) * @returns An object containing snapshot bytes and recognizer dialogs for each .lu file */ - export async function orchestratorBuilder( projectId: string, files: FileInfo[], @@ -91,6 +190,12 @@ const handleMessage = async (msg: RequestMsg) => { process.send?.({ id: msg.id, payload: snapshots }); break; } + case 'warmup': { + const { generatedFolderPath, projectId } = payload; + const done = await warmUpCache(generatedFolderPath, projectId); + process.send?.({ id: msg.id, payload: done }); + break; + } } } catch (error) { return { id: msg.id, error }; diff --git a/Composer/packages/server/src/models/bot/process/types.ts b/Composer/packages/server/src/models/bot/process/types.ts index af9e9bbff8..2a5abf105c 100644 --- a/Composer/packages/server/src/models/bot/process/types.ts +++ b/Composer/packages/server/src/models/bot/process/types.ts @@ -3,7 +3,7 @@ import { FileInfo } from '@bfc/shared'; export type BuildPayload = { - type: 'build'; + type: 'build' | 'warmup'; projectId: string; files: FileInfo[]; modelPath: string; From 1eaa77439a5077d47f6057b834bbde697549d949 Mon Sep 17 00:00:00 2001 From: Tai Chou Date: Tue, 23 Mar 2021 22:57:23 -0700 Subject: [PATCH 3/7] Tighten up code and exception handling --- .../server/src/controllers/orchestrator.ts | 4 +- .../packages/server/src/models/bot/builder.ts | 5 + .../models/bot/process/orchestratorWorker.ts | 110 +++++++----------- 3 files changed, 48 insertions(+), 71 deletions(-) diff --git a/Composer/packages/server/src/controllers/orchestrator.ts b/Composer/packages/server/src/controllers/orchestrator.ts index 188434637f..c6451f301b 100644 --- a/Composer/packages/server/src/controllers/orchestrator.ts +++ b/Composer/packages/server/src/controllers/orchestrator.ts @@ -52,7 +52,7 @@ async function downloadDefaultModel(req: Request, res: Response) { const lang = req.body; if (!isDefaultModelRequest(lang)) { - res.send(400); + res.sendStatus(400); return; } @@ -62,7 +62,7 @@ async function downloadDefaultModel(req: Request, res: Response) { if (await pathExists(modelPath)) { state = DownloadState.ALREADYDOWNLOADED; - return res.send(201); + return res.sendStatus(201); } const onProgress = (msg: string) => { diff --git a/Composer/packages/server/src/models/bot/builder.ts b/Composer/packages/server/src/models/bot/builder.ts index c703d723a2..993de6a009 100644 --- a/Composer/packages/server/src/models/bot/builder.ts +++ b/Composer/packages/server/src/models/bot/builder.ts @@ -104,6 +104,11 @@ export class Builder { try { //warm up the orchestrator cache if we're using it, before deleting and recreating the generated folder await orchestratorBuilder.warmupCache(this.botDir, this.generatedFolderPath); + } catch (error) { + throw new Error(error.message ?? 'Orchestrator cache warmup hit unexpected error'); + } + + try { await this.createGeneratedDir(); //do cross train before publish await this.crossTrain(luFiles, qnaFiles, allFiles); diff --git a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts index 1f3bc7a66f..8540dfb679 100644 --- a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts +++ b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts @@ -51,81 +51,53 @@ const cache = new LabelResolversCache(); * @param generatedFolderPath */ export async function warmUpCache(generatedFolderPath: string, projectId: string) { - //warm up the cache only if it's empty... - if (cache.get(projectId).size == 0) { - if (!(await pathExists(generatedFolderPath))) { - return false; - } - - const bluFiles = (await readdir(generatedFolderPath)).filter((fileName) => fileName.endsWith('.blu')); + //warm up the cache only if it's empty + if (!((await pathExists(generatedFolderPath)) || cache.get(projectId).size > 0)) { + return false; + } - //if there are blu file in the generated folders to hydrate with... - if (bluFiles.length == 0) { - return false; - } + const bluFiles = (await readdir(generatedFolderPath)).filter((fileName) => fileName.endsWith('.blu')); - const orchestratorSettingsPath = Path.resolve(generatedFolderPath, 'orchestrator.settings.json'); - if (!(await pathExists(orchestratorSettingsPath))) { - return false; - } + if (!bluFiles.length) { + return false; + } - //todo - typeguards and safety checks needed - const orchestratorSettings: IOrchestratorSettings = await readJson(orchestratorSettingsPath); - - let [en_files, multilang_files] = partition(bluFiles, (f) => f.split('.')?.[1].startsWith('en')); - - let enLabelResolvers: Map = new Map(); - - const enSnapShotData = await Promise.all( - en_files.map( - async (f) => - [f.replace('.blu', '.lu'), new Uint8Array(await readFile(Path.join(generatedFolderPath, f)))] as [ - string, - Uint8Array - ] - ) - ); - - if (orchestratorSettings.orchestrator?.models?.en) { - try { - enLabelResolvers = await Orchestrator.getLabelResolversAsync( - orchestratorSettings.orchestrator.models.en, - '', - new Map(enSnapShotData), - false - ); - } catch (err) {} - } + const orchestratorSettingsPath = Path.resolve(generatedFolderPath, 'orchestrator.settings.json'); + if (!(await pathExists(orchestratorSettingsPath))) { + return false; + } - //todo: reduce code duplication - const multilangSnapShotData = await Promise.all( - multilang_files.map( - async (f) => - [f.replace('.blu', '.lu'), new Uint8Array(await readFile(Path.join(generatedFolderPath, f)))] as [ - string, - Uint8Array - ] - ) - ); - - let multiLangLabelResolvers: Map = new Map(); - - if (orchestratorSettings.orchestrator?.models?.multilang) { - try { - multiLangLabelResolvers = await Orchestrator.getLabelResolversAsync( - orchestratorSettings.orchestrator.models.multilang, - '', - new Map(multilangSnapShotData), - false - ); - } catch (err) {} - } + const orchestratorSettings: IOrchestratorSettings = await readJson(orchestratorSettingsPath); + + let [enLuFiles, multiLangLuFiles] = partition(bluFiles, (f) => f.split('.')?.[1].startsWith('en')); + + const modelDatas = [ + { model: orchestratorSettings.orchestrator?.models?.en, lang: 'en', luFiles: enLuFiles }, + { model: orchestratorSettings.orchestrator?.models?.multilang, lang: 'multilang', luFiles: multiLangLuFiles }, + ]; + + const [enMap, multilangMap] = await Promise.all( + modelDatas.map(async (modelData) => { + const snapshotData = await Promise.all( + modelData.luFiles.map( + async (f) => + [f.replace('.blu', '.lu'), new Uint8Array(await readFile(Path.join(generatedFolderPath, f)))] as [ + string, + Uint8Array + ] + ) + ); + + if (modelData.model) { + return await Orchestrator.getLabelResolversAsync(modelData.model, '', new Map(snapshotData), false); + } + return new Map(); + }) + ); - cache.set(projectId, new Map([...enLabelResolvers, ...multiLangLabelResolvers])); + cache.set(projectId, new Map([...enMap, ...multilangMap])); - return true; - } - return false; + return true; } /** From 58b96b38988dbb47e1c062fd6f80490c4214e39c Mon Sep 17 00:00:00 2001 From: Tai Chou Date: Wed, 24 Mar 2021 23:58:41 -0700 Subject: [PATCH 4/7] Unit tests --- .../bot/__tests__/orchestratorWorker.test.ts | 157 ++++++++++++++++++ .../models/bot/process/orchestratorWorker.ts | 27 ++- 2 files changed, 175 insertions(+), 9 deletions(-) create mode 100644 Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts diff --git a/Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts b/Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts new file mode 100644 index 0000000000..f2616b3cfd --- /dev/null +++ b/Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts @@ -0,0 +1,157 @@ +import { cache, warmUpCache } from '../process/orchestratorWorker'; +import { LabelResolver, Utility, Orchestrator } from '@microsoft/bf-orchestrator'; +import { pathExists, readdir, readJson } from 'fs-extra'; + +jest.mock('@microsoft/bf-orchestrator'); +jest.mock('fs-extra', () => ({ + pathExists: jest.fn(async (path) => path === './generatedFolder' || path.endsWith('orchestrator.settings.json')), + readdir: jest.fn(async (path) => { + if (path === './generatedFolder') { + return ['test.en.lu', 'test.en.blu', 'test.zh-cn.blu', 'settings.json', '/path']; + } + return []; + }), + readJson: jest.fn(async (file) => { + return { + orchestrator: { + models: { + en: './model/en.onnx', + multilang: './model/multilang.onnx', + }, + snapshots: { + test_zh_cn: './generated/test.zh-cn.blu', + }, + }, + }; + }), + readFile: jest.fn(async (file) => { + return Buffer.from('test blu file'); + }), +})); + +describe('Orchestrator Warmup Cache', () => { + beforeAll(async () => { + Utility.toPrintDebuggingLogToConsole = false; //disable Orchestrator logging + }); + + beforeEach(async () => { + (Orchestrator.getLabelResolversAsync as jest.Mock).mockImplementation( + async (intentModelPath: string, _: string, snapshots: Map) => { + return new Map(); + } + ); + + (readdir as jest.Mock).mockClear(); + (pathExists as jest.Mock).mockClear(); + (Orchestrator.getLabelResolversAsync as jest.Mock).mockClear(); + + cache.clear(); + }); + + it('exits on invalid generatedFolderPath', async () => { + expect(await warmUpCache('badpath', 'abc')).toBeFalsy(); + }); + + it('exits if cache for project has contents', async () => { + const data: [string, LabelResolver] = ['test.en.lu', {} as LabelResolver]; + cache.set('abc', new Map([data])); + expect(cache.get('abc').size).toBe(1); + + expect(await warmUpCache('./generatedFolder', 'abc')).toBeFalsy(); + }); + + it('exits if no blu files in generated folder', async () => { + expect(cache.get('abc').size).toBe(0); + + expect(await warmUpCache('./emptyGeneratedFolder', 'abc')).toBeFalsy(); + expect(Orchestrator.getLabelResolversAsync).toHaveBeenCalledTimes(0); + }); + + it('exits if Orchestrator settings is invalid', async () => { + (Orchestrator.getLabelResolversAsync as jest.Mock).mockImplementation( + async (intentModelPath: string, _: string, snapshots: Map) => { + return new Map(); + } + ); + (readJson as jest.Mock).mockImplementationOnce(async (file) => 'corrupted settings'); + + await warmUpCache('./generatedFolder', 'abc'); + expect(pathExists).toHaveBeenCalledTimes(2); + expect(readJson).toHaveBeenCalled(); + + expect(Orchestrator.getLabelResolversAsync).toHaveBeenCalledTimes(0); + }); + + it('exits if Orchestrator settings cannot be read', async () => { + (readJson as jest.Mock).mockImplementationOnce(async (file) => undefined); + + expect(await warmUpCache('./generatedFolder', 'abc')).toBeFalsy(); + expect(pathExists).toHaveBeenCalledTimes(2); + expect(readJson).toHaveBeenCalled(); + + expect(Orchestrator.getLabelResolversAsync).toHaveBeenCalledTimes(0); + }); + + it('sends correct data shape to Orchestrator library for en + multilang', async () => { + expect(cache.get('abc').size).toBe(0); + expect(await readdir('./generatedFolder')).toContain('test.en.blu'); + + await warmUpCache('./generatedFolder', 'abc'); + + expect(Orchestrator.getLabelResolversAsync).toHaveBeenCalledTimes(2); + expect(Orchestrator.getLabelResolversAsync).toHaveBeenNthCalledWith( + 1, + './model/en.onnx', + '', + new Map([['test.en.lu', new Uint8Array(Buffer.from('test blu file'))]]), + false + ); + expect(Orchestrator.getLabelResolversAsync).toHaveBeenNthCalledWith( + 2, + './model/multilang.onnx', + '', + new Map([['test.zh-cn.lu', new Uint8Array(Buffer.from('test blu file'))]]), + false + ); + }); + + it('sends correct data shape to Orchestrator library for en only', async () => { + expect(cache.get('abc').size).toBe(0); + + (readdir as jest.Mock).mockImplementationOnce(async (path: string) => ['test.en.blu', 'test.en-us.blu']); + + await warmUpCache('./generatedFolder', 'abc'); + + expect(Orchestrator.getLabelResolversAsync).toHaveBeenCalledTimes(1); + expect(Orchestrator.getLabelResolversAsync).toHaveBeenNthCalledWith( + 1, + './model/en.onnx', + '', + new Map([ + ['test.en-us.lu', new Uint8Array(Buffer.from('test blu file'))], + ['test.en.lu', new Uint8Array(Buffer.from('test blu file'))], + ]), + false + ); + }); + + it('sends correct data shape to Orchestrator library for multilang only', async () => { + expect(cache.get('abc').size).toBe(0); + + (readdir as jest.Mock).mockImplementationOnce(async (path: string) => ['test.zh-cn.blu', 'test.ja-jp.blu']); + + await warmUpCache('./generatedFolder', 'abc'); + + expect(Orchestrator.getLabelResolversAsync).toHaveBeenCalledTimes(1); + expect(Orchestrator.getLabelResolversAsync).toHaveBeenNthCalledWith( + 1, + './model/multilang.onnx', + '', + new Map([ + ['test.zh-cn.lu', new Uint8Array(Buffer.from('test blu file'))], + ['test.ja-jp.lu', new Uint8Array(Buffer.from('test blu file'))], + ]), + false + ); + }); +}); diff --git a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts index 8540dfb679..0d61d89160 100644 --- a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts +++ b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts @@ -26,9 +26,13 @@ export class LabelResolversCache { public removeProject(projectId: string) { this.projects.delete(projectId); } + + public clear() { + this.projects.clear(); + } } -const cache = new LabelResolversCache(); +export const cache = new LabelResolversCache(); /** * Orchestrator: Warm up the LabelResolversCache if .blu files already exist. @@ -51,8 +55,8 @@ const cache = new LabelResolversCache(); * @param generatedFolderPath */ export async function warmUpCache(generatedFolderPath: string, projectId: string) { - //warm up the cache only if it's empty - if (!((await pathExists(generatedFolderPath)) || cache.get(projectId).size > 0)) { + //warm up the cache only if it's empty and we've built this bot before + if (!(await pathExists(generatedFolderPath)) || cache.get(projectId).size > 0) { return false; } @@ -67,13 +71,19 @@ export async function warmUpCache(generatedFolderPath: string, projectId: string return false; } + // an implementation detail is that we need to use the right model to reproduce the right LabelResolvers + // so we get the model versions from a pre-existing settings file, and split the files based on + // language const orchestratorSettings: IOrchestratorSettings = await readJson(orchestratorSettingsPath); + if (!orchestratorSettings?.orchestrator?.models || !orchestratorSettings?.orchestrator?.models) { + return false; + } let [enLuFiles, multiLangLuFiles] = partition(bluFiles, (f) => f.split('.')?.[1].startsWith('en')); const modelDatas = [ - { model: orchestratorSettings.orchestrator?.models?.en, lang: 'en', luFiles: enLuFiles }, - { model: orchestratorSettings.orchestrator?.models?.multilang, lang: 'multilang', luFiles: multiLangLuFiles }, + { model: orchestratorSettings?.orchestrator?.models?.en, lang: 'en', luFiles: enLuFiles }, + { model: orchestratorSettings?.orchestrator?.models?.multilang, lang: 'multilang', luFiles: multiLangLuFiles }, ]; const [enMap, multilangMap] = await Promise.all( @@ -88,10 +98,9 @@ export async function warmUpCache(generatedFolderPath: string, projectId: string ) ); - if (modelData.model) { - return await Orchestrator.getLabelResolversAsync(modelData.model, '', new Map(snapshotData), false); - } - return new Map(); + return modelData.model && snapshotData.length + ? await Orchestrator.getLabelResolversAsync(modelData.model, '', new Map(snapshotData), false) + : new Map(); }) ); From bffbbc999864998e48a5c246a90a8dbd40f6de54 Mon Sep 17 00:00:00 2001 From: Tai Chou Date: Thu, 25 Mar 2021 00:11:20 -0700 Subject: [PATCH 5/7] Fix linter errors --- .../src/models/bot/__tests__/orchestratorWorker.test.ts | 8 ++++++-- .../server/src/models/bot/process/orchestratorWorker.ts | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts b/Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts index f2616b3cfd..de1680565d 100644 --- a/Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts +++ b/Composer/packages/server/src/models/bot/__tests__/orchestratorWorker.test.ts @@ -1,7 +1,11 @@ -import { cache, warmUpCache } from '../process/orchestratorWorker'; +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + import { LabelResolver, Utility, Orchestrator } from '@microsoft/bf-orchestrator'; import { pathExists, readdir, readJson } from 'fs-extra'; +import { cache, warmUpCache } from '../process/orchestratorWorker'; + jest.mock('@microsoft/bf-orchestrator'); jest.mock('fs-extra', () => ({ pathExists: jest.fn(async (path) => path === './generatedFolder' || path.endsWith('orchestrator.settings.json')), @@ -19,7 +23,7 @@ jest.mock('fs-extra', () => ({ multilang: './model/multilang.onnx', }, snapshots: { - test_zh_cn: './generated/test.zh-cn.blu', + testZhCn: './generated/test.zh-cn.blu', }, }, }; diff --git a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts index 0d61d89160..e667725b33 100644 --- a/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts +++ b/Composer/packages/server/src/models/bot/process/orchestratorWorker.ts @@ -4,7 +4,7 @@ import { FileInfo } from '@bfc/shared'; import { LabelResolver, Orchestrator } from '@microsoft/bf-orchestrator'; import { writeFile, readdir, readFile, pathExists, readJson } from 'fs-extra'; -import { partition } from 'lodash'; +import partition from 'lodash/partition'; import { Path } from '../../../utility/path'; import { IOrchestratorBuildOutput, IOrchestratorSettings } from '../interface'; @@ -79,7 +79,7 @@ export async function warmUpCache(generatedFolderPath: string, projectId: string return false; } - let [enLuFiles, multiLangLuFiles] = partition(bluFiles, (f) => f.split('.')?.[1].startsWith('en')); + const [enLuFiles, multiLangLuFiles] = partition(bluFiles, (f) => f.split('.')?.[1].startsWith('en')); const modelDatas = [ { model: orchestratorSettings?.orchestrator?.models?.en, lang: 'en', luFiles: enLuFiles }, From 0173319b59f9c6cd4dae8d11695db9739807785b Mon Sep 17 00:00:00 2001 From: Tai Chou Date: Thu, 25 Mar 2021 00:24:06 -0700 Subject: [PATCH 6/7] Don't rethrow error for cache - safe to continue on --- Composer/packages/server/src/models/bot/builder.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Composer/packages/server/src/models/bot/builder.ts b/Composer/packages/server/src/models/bot/builder.ts index 993de6a009..e606fbdbd0 100644 --- a/Composer/packages/server/src/models/bot/builder.ts +++ b/Composer/packages/server/src/models/bot/builder.ts @@ -102,10 +102,10 @@ export class Builder { setEnvDefault('QNA_USER_AGENT', userAgent); try { - //warm up the orchestrator cache if we're using it, before deleting and recreating the generated folder + //warm up the orchestrator build cache before deleting and recreating the generated folder await orchestratorBuilder.warmupCache(this.botDir, this.generatedFolderPath); - } catch (error) { - throw new Error(error.message ?? 'Orchestrator cache warmup hit unexpected error'); + } catch (err) { + log(err); } try { From a72f7522dfc41eb799737f0da2dbc2a2be4c87fd Mon Sep 17 00:00:00 2001 From: Tai Chou Date: Thu, 25 Mar 2021 16:17:06 -0700 Subject: [PATCH 7/7] Fix build error of worker script when testing --- .../server/src/models/bot/process/orchestratorBuilder.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts b/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts index 93484e7437..bf8dcfcfb6 100644 --- a/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts +++ b/Composer/packages/server/src/models/bot/process/orchestratorBuilder.ts @@ -68,7 +68,10 @@ class OrchestratorBuilder { const workerScriptPath = path.join(__dirname, 'orchestratorWorker.ts'); if (fs.existsSync(workerScriptPath)) { // set exec arguments to empty, avoid fork nodemon `--inspect` error - this._worker = fork(workerScriptPath, [], { execArgv: ['-r', 'ts-node/register'] }); + this._worker = fork(workerScriptPath, [], { + execArgv: ['-r', 'ts-node/register'], + env: { TS_NODE_PROJECT: path.resolve(__dirname, '..', '..', '..', '..', 'tsconfig.json') }, + }); } else { // set exec arguments to empty, avoid fork nodemon `--inspect` error this._worker = fork(path.join(__dirname, 'orchestratorWorker.js'), [], { execArgv: [] });