From a94e7900213c9b6eca667d6a359e2334ab640d65 Mon Sep 17 00:00:00 2001 From: ppisljar Date: Wed, 20 Nov 2019 05:18:02 -0800 Subject: [PATCH] interpreter - caching --- .../data/public/search/expressions/esaggs.ts | 1 + .../common/execution/execution.test.ts | 6 ++ .../expressions/common/execution/execution.ts | 29 +++++++- .../execution/execution_contract.test.ts | 1 + .../expressions/common/execution/types.ts | 5 ++ .../common/executor/executor.test.ts | 67 +++++++++++++++++++ .../expressions/common/executor/executor.ts | 6 +- .../expression_function.ts | 18 ++++- .../common/expression_functions/specs/var.ts | 1 + .../expression_functions/specs/var_set.ts | 1 + .../common/expression_functions/types.ts | 5 ++ src/plugins/expressions/common/mocks.ts | 1 + src/plugins/expressions/public/loader.ts | 3 + src/plugins/expressions/public/types/index.ts | 1 + .../vis_type_timeseries/public/metrics_fn.ts | 1 + .../public/embeddable/visualize_embeddable.ts | 11 +-- 16 files changed, 148 insertions(+), 9 deletions(-) diff --git a/src/plugins/data/public/search/expressions/esaggs.ts b/src/plugins/data/public/search/expressions/esaggs.ts index 1021ef0f91d52..0ca5af10b4c2a 100644 --- a/src/plugins/data/public/search/expressions/esaggs.ts +++ b/src/plugins/data/public/search/expressions/esaggs.ts @@ -213,6 +213,7 @@ const handleCourierRequest = async ({ export const esaggs = (): EsaggsExpressionFunctionDefinition => ({ name, + noCache: true, type: 'kibana_datatable', inputTypes: ['kibana_context', 'null'], help: i18n.translate('data.functions.esaggs.help', { diff --git a/src/plugins/expressions/common/execution/execution.test.ts b/src/plugins/expressions/common/execution/execution.test.ts index ff331d7c5ddaf..e21f35360d865 100644 --- a/src/plugins/expressions/common/execution/execution.test.ts +++ b/src/plugins/expressions/common/execution/execution.test.ts @@ -40,6 +40,7 @@ const createExecution = ( ast: parseExpression(expression), context, debug, + functionCache: new Map(), }); return execution; }; @@ -143,6 +144,7 @@ describe('Execution', () => { const execution = new Execution({ executor, expression, + functionCache: new Map(), }); expect(execution.expression).toBe(expression); }); @@ -153,6 +155,7 @@ describe('Execution', () => { const execution = new Execution({ ast: parseExpression(expression), executor, + functionCache: new Map(), }); expect(execution.expression).toBe(expression); }); @@ -620,6 +623,7 @@ describe('Execution', () => { executor, ast: parseExpression('add val=1 | throws | add val=3'), debug: true, + functionCache: new Map(), }); execution.start(0); await execution.result; @@ -638,6 +642,7 @@ describe('Execution', () => { executor, ast: parseExpression('add val=1 | throws | add val=3'), debug: true, + functionCache: new Map(), }); execution.start(0); await execution.result; @@ -659,6 +664,7 @@ describe('Execution', () => { executor, ast: parseExpression('add val=1 | throws | add val=3'), debug: true, + functionCache: new Map(), }); execution.start(0); await execution.result; diff --git a/src/plugins/expressions/common/execution/execution.ts b/src/plugins/expressions/common/execution/execution.ts index d4c9b0a25d45b..e1a810e69c41b 100644 --- a/src/plugins/expressions/common/execution/execution.ts +++ b/src/plugins/expressions/common/execution/execution.ts @@ -39,6 +39,8 @@ import { ArgumentType, ExpressionFunction } from '../expression_functions'; import { getByAlias } from '../util/get_by_alias'; import { ExecutionContract } from './execution_contract'; +const maxCacheSize = 1000; + const createAbortErrorValue = () => createError({ message: 'The expression was aborted.', @@ -52,7 +54,7 @@ export interface ExecutionParams< ast?: ExpressionAstExpression; expression?: string; context?: ExtraContext; - + functionCache: Map; /** * Whether to execute expression in *debug mode*. In *debug mode* inputs and * outputs as well as all resolved arguments and time it took to execute each @@ -120,6 +122,8 @@ export class Execution< */ private readonly firstResultFuture = new Defer(); + private functionCache: Map = new Map(); + /** * Contract is a public representation of `Execution` instances. Contract we * can return to other plugins for their consumption. @@ -269,13 +273,34 @@ export class Execution< return input; } + async getCachedResults( + fn: ExpressionFunction, + normalizedInput: unknown, + args: Record + ) { + let fnOutput; + const hash = calculateObjectHash([fn.name, normalizedInput, args, this.context.search]); + if (!this.context.disableCache && !fn.disableCache && this.functionCache.has(hash)) { + fnOutput = this.functionCache.get(hash); + } else { + fnOutput = await this.race(fn.fn(normalizedInput, args, this.context)); + if (!fn.disableCache) { + while (this.functionCache.size >= maxCacheSize) { + this.functionCache.delete(this.functionCache.keys().next().value); + } + this.functionCache.set(hash, fnOutput); + } + } + return fnOutput; + } + async invokeFunction( fn: ExpressionFunction, input: unknown, args: Record ): Promise { const normalizedInput = this.cast(input, fn.inputTypes); - const output = await this.race(fn.fn(normalizedInput, args, this.context)); + const output = await this.getCachedResults(fn, normalizedInput, args); // Validate that the function returned the type it said it would. // This isn't required, but it keeps function developers honest. diff --git a/src/plugins/expressions/common/execution/execution_contract.test.ts b/src/plugins/expressions/common/execution/execution_contract.test.ts index c33f8a1a0f36e..50ef7f00e2f5d 100644 --- a/src/plugins/expressions/common/execution/execution_contract.test.ts +++ b/src/plugins/expressions/common/execution/execution_contract.test.ts @@ -31,6 +31,7 @@ const createExecution = ( executor, ast: parseExpression(expression), context, + functionCache: new Map(), }); return execution; }; diff --git a/src/plugins/expressions/common/execution/types.ts b/src/plugins/expressions/common/execution/types.ts index 7c26e586fb790..f3ab47fa9a508 100644 --- a/src/plugins/expressions/common/execution/types.ts +++ b/src/plugins/expressions/common/execution/types.ts @@ -42,6 +42,11 @@ export interface ExecutionContext; + /** + * Prevents caching in the current execution. + */ + disableCache?: boolean; + /** * Adds ability to abort current execution. */ diff --git a/src/plugins/expressions/common/executor/executor.test.ts b/src/plugins/expressions/common/executor/executor.test.ts index 81845401d32e4..9cfacac3f4e68 100644 --- a/src/plugins/expressions/common/executor/executor.test.ts +++ b/src/plugins/expressions/common/executor/executor.test.ts @@ -152,4 +152,71 @@ describe('Executor', () => { }); }); }); + + describe('caching', () => { + const functionCache: Map = new Map(); + const fakeCacheEntry = { type: 'kibana_context', value: 'test' }; + let executor: Executor; + + beforeAll(() => { + executor = new Executor(undefined, functionCache); + executor.registerFunction(expressionFunctions.variable); + executor.registerFunction(expressionFunctions.kibana); + }); + + afterEach(() => { + functionCache.clear(); + }); + + it('caches the result of function', async () => { + await executor.run('kibana', null); + expect(functionCache.size).toEqual(1); + const entry = functionCache.keys().next().value; + functionCache.set(entry, fakeCacheEntry); + const result = await executor.run('kibana', null); + expect(functionCache.size).toEqual(1); + expect(result).toEqual(fakeCacheEntry); + }); + + it('doesnt cache if disableCache flag is enabled', async () => { + await executor.run('kibana', null); + expect(functionCache.size).toEqual(1); + const entry = functionCache.keys().next().value; + functionCache.set(entry, fakeCacheEntry); + const result = await executor.run('kibana', null, { disableCache: true }); + expect(functionCache.size).toEqual(1); + expect(result).not.toEqual(fakeCacheEntry); + }); + + it('doesnt cache results of functions that have disableCache property set', async () => { + await executor.run('var name="test"', null); + expect(functionCache.size).toEqual(0); + }); + + describe('doesnt use cached version', () => { + const cachedVersion = { test: 'value' }; + + beforeAll(async () => { + await executor.run('kibana', null); + expect(functionCache.size).toEqual(1); + const entry: string = Object.keys(functionCache)[0]; + functionCache.set(entry, cachedVersion); + }); + + it('input changed', async () => { + const result = await executor.run('kibana', { type: 'kibana_context', value: 'test' }); + expect(result).not.toEqual(cachedVersion); + }); + + it('arguments changed', async () => { + const result = await executor.run('kibana', null); + expect(result).not.toEqual(cachedVersion); + }); + + it('search context changed', async () => { + const result = await executor.run('kibana', null, { search: { filters: [] } }); + expect(result).not.toEqual(cachedVersion); + }); + }); + }); }); diff --git a/src/plugins/expressions/common/executor/executor.ts b/src/plugins/expressions/common/executor/executor.ts index 2b5f9f2556d89..36944ac75d252 100644 --- a/src/plugins/expressions/common/executor/executor.ts +++ b/src/plugins/expressions/common/executor/executor.ts @@ -105,10 +105,13 @@ export class Executor = Record) { + private functionCache: Map; + + constructor(state?: ExecutorState, functionCache: Map = new Map()) { this.state = createExecutorContainer(state); this.functions = new FunctionsRegistry(this); this.types = new TypesRegistry(this); + this.functionCache = functionCache || new Map(); } public registerFunction( @@ -186,6 +189,7 @@ export class Executor = Record>; + /** + * Opt-out of caching this function. By default function outputs are cached and given the same inputs cached result is returned. + */ + disableCache?: boolean; + /** * List of allowed type names for input value of this function. If this * property is set the input of function will be cast to the first possible diff --git a/src/plugins/expressions/common/mocks.ts b/src/plugins/expressions/common/mocks.ts index 502d88ac955ae..a0d96db43defa 100644 --- a/src/plugins/expressions/common/mocks.ts +++ b/src/plugins/expressions/common/mocks.ts @@ -38,6 +38,7 @@ export const createMockExecutionContext = data: {} as any, }, search: {}, + disableCache: false, }; return { diff --git a/src/plugins/expressions/public/loader.ts b/src/plugins/expressions/public/loader.ts index c4c40e0812e48..28a012dd9eb4a 100644 --- a/src/plugins/expressions/public/loader.ts +++ b/src/plugins/expressions/public/loader.ts @@ -181,6 +181,9 @@ export class ExpressionLoader { if (params.variables && this.params) { this.params.variables = params.variables; } + if (params.disableCache !== undefined) { + this.params.disableCache = params.disableCache; + } this.params.inspectorAdapters = (params.inspectorAdapters || this.execution?.inspect()) as Adapters; diff --git a/src/plugins/expressions/public/types/index.ts b/src/plugins/expressions/public/types/index.ts index 5e349c95d2555..74f6083a4db59 100644 --- a/src/plugins/expressions/public/types/index.ts +++ b/src/plugins/expressions/public/types/index.ts @@ -51,6 +51,7 @@ export interface IExpressionLoaderParams { uiState?: unknown; inspectorAdapters?: Adapters; onRenderError?: RenderErrorHandlerFnType; + disableCache?: boolean; } export interface ExpressionRenderError extends Error { diff --git a/src/plugins/vis_type_timeseries/public/metrics_fn.ts b/src/plugins/vis_type_timeseries/public/metrics_fn.ts index b573225feaab1..f9fcd64d6681a 100644 --- a/src/plugins/vis_type_timeseries/public/metrics_fn.ts +++ b/src/plugins/vis_type_timeseries/public/metrics_fn.ts @@ -49,6 +49,7 @@ export const createMetricsFn = (): ExpressionFunctionDefinition< Output > => ({ name: 'tsvb', + noCache: true, type: 'render', inputTypes: ['kibana_context', 'null'], help: i18n.translate('visTypeTimeseries.function.help', { diff --git a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts index fe8a9adff4052..1ddc1d38213ab 100644 --- a/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts +++ b/src/plugins/visualizations/public/embeddable/visualize_embeddable.ts @@ -152,7 +152,7 @@ export class VisualizeEmbeddable this.autoRefreshFetchSubscription = timefilter .getAutoRefreshFetch$() - .subscribe(this.updateHandler.bind(this)); + .subscribe(this.updateHandler.bind(this, true)); this.subscriptions.push( Rx.merge(this.getOutput$(), this.getInput$()).subscribe(() => { @@ -364,10 +364,10 @@ export class VisualizeEmbeddable } public reload = () => { - this.handleVisUpdate(); + this.handleVisUpdate(true); }; - private async updateHandler() { + private async updateHandler(disableCache: boolean = false) { const expressionParams: IExpressionLoaderParams = { searchContext: { timeRange: this.timeRange, @@ -376,6 +376,7 @@ export class VisualizeEmbeddable }, uiState: this.vis.uiState, inspectorAdapters: this.inspectorAdapters, + disableCache, }; if (this.abortController) { this.abortController.abort(); @@ -393,8 +394,8 @@ export class VisualizeEmbeddable } } - private handleVisUpdate = async () => { - this.updateHandler(); + private handleVisUpdate = async (disableCache: boolean = false) => { + this.updateHandler(disableCache); }; private uiStateChangeHandler = () => {