Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Expressions caching #51160

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/plugins/data/public/search/expressions/esaggs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
6 changes: 6 additions & 0 deletions src/plugins/expressions/common/execution/execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const createExecution = (
ast: parseExpression(expression),
context,
debug,
functionCache: new Map(),
});
return execution;
};
Expand Down Expand Up @@ -143,6 +144,7 @@ describe('Execution', () => {
const execution = new Execution({
executor,
expression,
functionCache: new Map(),
});
expect(execution.expression).toBe(expression);
});
Expand All @@ -153,6 +155,7 @@ describe('Execution', () => {
const execution = new Execution({
ast: parseExpression(expression),
executor,
functionCache: new Map(),
});
expect(execution.expression).toBe(expression);
});
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down
29 changes: 27 additions & 2 deletions src/plugins/expressions/common/execution/execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.',
Expand All @@ -52,7 +54,7 @@ export interface ExecutionParams<
ast?: ExpressionAstExpression;
expression?: string;
context?: ExtraContext;

functionCache: Map<string, any>;
/**
* 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
Expand Down Expand Up @@ -120,6 +122,8 @@ export class Execution<
*/
private readonly firstResultFuture = new Defer<Output | ExpressionValueError>();

private functionCache: Map<string, any> = new Map();

/**
* Contract is a public representation of `Execution` instances. Contract we
* can return to other plugins for their consumption.
Expand Down Expand Up @@ -269,13 +273,34 @@ export class Execution<
return input;
}

async getCachedResults(
fn: ExpressionFunction,
normalizedInput: unknown,
args: Record<string, unknown>
) {
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<string, unknown>
): Promise<any> {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const createExecution = (
executor,
ast: parseExpression(expression),
context,
functionCache: new Map(),
});
return execution;
};
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/expressions/common/execution/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ export interface ExecutionContext<Input = unknown, InspectorAdapters extends Ada
*/
types: Record<string, ExpressionType>;

/**
* Prevents caching in the current execution.
*/
disableCache?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something we should expose through execution context to all functions? How would functions use this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should already be provided to all functions on the executionContext.

if some function would to do some internal caching (esaggs with make it slow) this would indicate that it should do it.


/**
* Adds ability to abort current execution.
*/
Expand Down
67 changes: 67 additions & 0 deletions src/plugins/expressions/common/executor/executor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,71 @@ describe('Executor', () => {
});
});
});

describe('caching', () => {
const functionCache: Map<string, any> = 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);
});
});
});
});
6 changes: 5 additions & 1 deletion src/plugins/expressions/common/executor/executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,13 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
*/
public readonly types: TypesRegistry;

constructor(state?: ExecutorState<Context>) {
private functionCache: Map<string, any>;

constructor(state?: ExecutorState<Context>, functionCache: Map<string, any> = new Map()) {
this.state = createExecutorContainer<Context>(state);
this.functions = new FunctionsRegistry(this);
this.types = new TypesRegistry(this);
this.functionCache = functionCache || new Map();
}

public registerFunction(
Expand Down Expand Up @@ -186,6 +189,7 @@ export class Executor<Context extends Record<string, unknown> = Record<string, u
...this.context,
...context,
} as Context & ExtraContext,
functionCache: this.functionCache,
debug,
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ export class ExpressionFunction {
*/
type: string;

/**
* Opt-out of caching this function. By default function outputs are cached and given the same inputs cached result is returned.
*/
disableCache: boolean;

/**
* Function to run function (context, args)
*/
Expand All @@ -61,7 +66,17 @@ export class ExpressionFunction {
inputTypes: string[] | undefined;

constructor(functionDefinition: AnyExpressionFunctionDefinition) {
const { name, type, aliases, fn, help, args, inputTypes, context } = functionDefinition;
const {
name,
type,
aliases,
fn,
help,
args,
inputTypes,
context,
disableCache,
} = functionDefinition;

this.name = name;
this.type = type;
Expand All @@ -70,6 +85,7 @@ export class ExpressionFunction {
Promise.resolve(fn(input, params, handlers as ExecutionContext));
this.help = help || '';
this.inputTypes = inputTypes || context?.types;
this.disableCache = !!disableCache;

for (const [key, arg] of Object.entries(args || {})) {
this.args[key] = new ExpressionFunctionParameter(key, arg);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const variable: ExpressionFunctionVar = {
help: i18n.translate('expressions.functions.var.help', {
defaultMessage: 'Updates the Kibana global context.',
}),
disableCache: true,
args: {
name: {
types: ['string'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const variableSet: ExpressionFunctionVarSet = {
help: i18n.translate('expressions.functions.varset.help', {
defaultMessage: 'Updates the Kibana global context.',
}),
disableCache: true,
args: {
name: {
types: ['string'],
Expand Down
5 changes: 5 additions & 0 deletions src/plugins/expressions/common/expression_functions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ export interface ExpressionFunctionDefinition<
*/
type?: TypeToString<UnwrapPromiseOrReturn<Output>>;

/**
* 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
Expand Down
1 change: 1 addition & 0 deletions src/plugins/expressions/common/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export const createMockExecutionContext = <ExtraContext extends object = object>
data: {} as any,
},
search: {},
disableCache: false,
};

return {
Expand Down
3 changes: 3 additions & 0 deletions src/plugins/expressions/public/loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/plugins/expressions/public/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export interface IExpressionLoaderParams {
uiState?: unknown;
inspectorAdapters?: Adapters;
onRenderError?: RenderErrorHandlerFnType;
disableCache?: boolean;
}

export interface ExpressionRenderError extends Error {
Expand Down
1 change: 1 addition & 0 deletions src/plugins/vis_type_timeseries/public/metrics_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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,
Expand All @@ -376,6 +376,7 @@ export class VisualizeEmbeddable
},
uiState: this.vis.uiState,
inspectorAdapters: this.inspectorAdapters,
disableCache,
};
if (this.abortController) {
this.abortController.abort();
Expand All @@ -393,8 +394,8 @@ export class VisualizeEmbeddable
}
}

private handleVisUpdate = async () => {
this.updateHandler();
private handleVisUpdate = async (disableCache: boolean = false) => {
this.updateHandler(disableCache);
};

private uiStateChangeHandler = () => {
Expand Down