Skip to content

Commit

Permalink
[Resolver] Selector performance (elastic#72380)
Browse files Browse the repository at this point in the history
* Memoize various selectors
* Improve performance of the selectors that calculate the `aria-flowto` attribute.
* more tests.
  • Loading branch information
Robert Austin committed Jul 20, 2020
1 parent 69cd45c commit 93c833c
Show file tree
Hide file tree
Showing 11 changed files with 427 additions and 280 deletions.
1 change: 0 additions & 1 deletion x-pack/plugins/security_solution/common/endpoint/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,7 +482,6 @@ export interface LegacyEndpointEvent {
type: string;
version: string;
};
process?: object;
rule?: object;
user?: object;
event?: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
* you may not use this file except in compliance with the Elastic License.
*/

/* eslint-disable no-shadow */

import { uniquePidForProcess, uniqueParentPidForProcess, orderByTime } from '../process_event';
import { IndexedProcessTree } from '../../types';
import { ResolverEvent } from '../../../../common/endpoint/types';
Expand All @@ -24,16 +26,15 @@ export function factory(
const uniqueProcessPid = uniquePidForProcess(process);
idToValue.set(uniqueProcessPid, process);

const uniqueParentPid = uniqueParentPidForProcess(process);
// if its defined and not ''
if (uniqueParentPid) {
let siblings = idToChildren.get(uniqueParentPid);
if (!siblings) {
siblings = [];
idToChildren.set(uniqueParentPid, siblings);
}
siblings.push(process);
// NB: If the value was null or undefined, use `undefined`
const uniqueParentPid: string | undefined = uniqueParentPidForProcess(process) ?? undefined;

let childrenWithTheSameParent = idToChildren.get(uniqueParentPid);
if (!childrenWithTheSameParent) {
childrenWithTheSameParent = [];
idToChildren.set(uniqueParentPid, childrenWithTheSameParent);
}
childrenWithTheSameParent.push(process);
}

// sort the children of each node
Expand All @@ -50,9 +51,8 @@ export function factory(
/**
* Returns an array with any children `ProcessEvent`s of the passed in `process`
*/
export function children(tree: IndexedProcessTree, process: ResolverEvent): ResolverEvent[] {
const id = uniquePidForProcess(process);
const currentProcessSiblings = tree.idToChildren.get(id);
export function children(tree: IndexedProcessTree, parentID: string | undefined): ResolverEvent[] {
const currentProcessSiblings = tree.idToChildren.get(parentID);
return currentProcessSiblings === undefined ? [] : currentProcessSiblings;
}

Expand All @@ -78,31 +78,6 @@ export function parent(
}
}

/**
* Returns the following sibling
*/
export function nextSibling(
tree: IndexedProcessTree,
sibling: ResolverEvent
): ResolverEvent | undefined {
const parentNode = parent(tree, sibling);
if (parentNode) {
// The siblings of `sibling` are the children of its parent.
const siblings = children(tree, parentNode);

// Find the sibling
const index = siblings.indexOf(sibling);

// if the sibling wasn't found, or if it was the last element in the array, return undefined
if (index === -1 || index === siblings.length - 1) {
return undefined;
}

// return the next sibling
return siblings[index + 1];
}
}

/**
* Number of processes in the tree
*/
Expand Down Expand Up @@ -133,6 +108,8 @@ export function root(tree: IndexedProcessTree) {
export function* levelOrder(tree: IndexedProcessTree) {
const rootNode = root(tree);
if (rootNode !== null) {
yield* baseLevelOrder(rootNode, children.bind(null, tree));
yield* baseLevelOrder(rootNode, (parentNode: ResolverEvent): ResolverEvent[] =>
children(tree, uniquePidForProcess(parentNode))
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import * as event from '../../../../common/endpoint/models/event';
import { ResolverEvent } from '../../../../common/endpoint/types';
import * as model from './index';
import { getFriendlyElapsedTime as elapsedTime } from '../../lib/date';
import { uniquePidForProcess } from '../process_event';

/**
* Graph the process tree
Expand Down Expand Up @@ -146,10 +147,12 @@ function widthsOfProcessSubtrees(indexedProcessTree: IndexedProcessTree): Proces
return widths;
}

const processesInReverseLevelOrder = [...model.levelOrder(indexedProcessTree)].reverse();
const processesInReverseLevelOrder: ResolverEvent[] = [
...model.levelOrder(indexedProcessTree),
].reverse();

for (const process of processesInReverseLevelOrder) {
const children = model.children(indexedProcessTree, process);
const children = model.children(indexedProcessTree, uniquePidForProcess(process));

const sumOfWidthOfChildren = function sumOfWidthOfChildren() {
return children.reduce(function sum(currentValue, child) {
Expand Down Expand Up @@ -226,7 +229,7 @@ function processEdgeLineSegments(
metadata: edgeLineMetadata,
};

const siblings = model.children(indexedProcessTree, parent);
const siblings = model.children(indexedProcessTree, uniquePidForProcess(parent));
const isFirstChild = process === siblings[0];

if (metadata.isOnlyChild) {
Expand Down Expand Up @@ -420,7 +423,7 @@ function* levelOrderWithWidths(
parentWidth,
};

const siblings = model.children(tree, parent);
const siblings = model.children(tree, uniquePidForProcess(parent));
if (siblings.length === 1) {
metadata.isOnlyChild = true;
metadata.lastChildWidth = width;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,8 @@ export const scale: (state: CameraState) => (time: number) => Vector2 = createSe
scalingConstants.maximum
);

return (time) => {
// memoizing this so the vector returned will be the same object reference if called with the same `time`.
return defaultMemoize((time) => {
/**
* If the animation has completed, return the `scaleNotCountingAnimation`, as
* the animation always completes with the scale set back at starting value.
Expand Down Expand Up @@ -247,12 +248,13 @@ export const scale: (state: CameraState) => (time: number) => Vector2 = createSe
*/
return [lerpedScale, lerpedScale];
}
};
});
} else {
/**
* The scale should be the same in both axes.
* Memoizing this so the vector returned will be the same object reference every time.
*/
return () => [scaleNotCountingAnimation, scaleNotCountingAnimation];
return defaultMemoize(() => [scaleNotCountingAnimation, scaleNotCountingAnimation]);
}

/**
Expand All @@ -277,22 +279,26 @@ export const clippingPlanes: (
) => (time: number) => ClippingPlanes = createSelector(
(state) => state.rasterSize,
scale,
(rasterSize, scaleAtTime) => (time: number) => {
const [scaleX, scaleY] = scaleAtTime(time);
const renderWidth = rasterSize[0];
const renderHeight = rasterSize[1];
const clippingPlaneRight = renderWidth / 2 / scaleX;
const clippingPlaneTop = renderHeight / 2 / scaleY;

return {
renderWidth,
renderHeight,
clippingPlaneRight,
clippingPlaneTop,
clippingPlaneLeft: -clippingPlaneRight,
clippingPlaneBottom: -clippingPlaneTop,
};
}
(rasterSize, scaleAtTime) =>
/**
* memoizing this for object reference equality.
*/
defaultMemoize((time: number) => {
const [scaleX, scaleY] = scaleAtTime(time);
const renderWidth = rasterSize[0];
const renderHeight = rasterSize[1];
const clippingPlaneRight = renderWidth / 2 / scaleX;
const clippingPlaneTop = renderHeight / 2 / scaleY;

return {
renderWidth,
renderHeight,
clippingPlaneRight,
clippingPlaneTop,
clippingPlaneLeft: -clippingPlaneRight,
clippingPlaneBottom: -clippingPlaneTop,
};
})
);

/**
Expand Down Expand Up @@ -323,7 +329,10 @@ export const translation: (state: CameraState) => (time: number) => Vector2 = cr
scale,
(state) => state.animation,
(panning, translationNotCountingCurrentPanning, scaleAtTime, animation) => {
return (time: number) => {
/**
* Memoizing this for object reference equality.
*/
return defaultMemoize((time: number) => {
const [scaleX, scaleY] = scaleAtTime(time);
if (animation !== undefined && animationIsActive(animation, time)) {
return vector2.lerp(
Expand All @@ -343,7 +352,7 @@ export const translation: (state: CameraState) => (time: number) => Vector2 = cr
} else {
return translationNotCountingCurrentPanning;
}
};
});
}
);

Expand All @@ -357,7 +366,10 @@ export const inverseProjectionMatrix: (
clippingPlanes,
translation,
(clippingPlanesAtTime, translationAtTime) => {
return (time: number) => {
/**
* Memoizing this for object reference equality (and reduced memory churn.)
*/
return defaultMemoize((time: number) => {
const {
renderWidth,
renderHeight,
Expand Down Expand Up @@ -404,7 +416,7 @@ export const inverseProjectionMatrix: (
translateForCamera,
multiply(scaleToClippingPlaneDimensions, multiply(invertY, screenToNDC))
);
};
});
}
);

Expand All @@ -415,7 +427,8 @@ export const viewableBoundingBox: (state: CameraState) => (time: number) => AABB
clippingPlanes,
inverseProjectionMatrix,
(clippingPlanesAtTime, matrixAtTime) => {
return (time: number) => {
// memoizing this so the AABB returned will be the same object reference if called with the same `time`.
return defaultMemoize((time: number) => {
const { renderWidth, renderHeight } = clippingPlanesAtTime(time);
const matrix = matrixAtTime(time);
const bottomLeftCorner: Vector2 = [0, renderHeight];
Expand All @@ -424,7 +437,7 @@ export const viewableBoundingBox: (state: CameraState) => (time: number) => AABB
minimum: vector2.applyMatrix3(bottomLeftCorner, matrix),
maximum: vector2.applyMatrix3(topRightCorner, matrix),
};
};
});
}
);

Expand All @@ -436,6 +449,8 @@ export const projectionMatrix: (state: CameraState) => (time: number) => Matrix3
clippingPlanes,
translation,
(clippingPlanesAtTime, translationAtTime) => {
// memoizing this so the matrix returned will be the same object reference if called with the same `time`.
// this should also save on some memory allocation
return defaultMemoize((time: number) => {
const {
renderWidth,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ import { DataState } from '../../types';
import { dataReducer } from './reducer';
import { DataAction } from './action';
import { createStore } from 'redux';
import {
mockTreeWithNoAncestorsAnd2Children,
mockTreeWith2AncestorsAndNoChildren,
} from '../mocks/resolver_tree';
import { uniquePidForProcess } from '../../models/process_event';
import { EndpointEvent } from '../../../../common/endpoint/types';

describe('data state', () => {
let actions: DataAction[] = [];

Expand Down Expand Up @@ -263,4 +270,87 @@ describe('data state', () => {
});
});
});
describe('with a tree with no descendants and 2 ancestors', () => {
const originID = 'c';
const firstAncestorID = 'b';
const secondAncestorID = 'a';
beforeEach(() => {
actions.push({
type: 'serverReturnedResolverData',
payload: {
result: mockTreeWith2AncestorsAndNoChildren({
originID,
firstAncestorID,
secondAncestorID,
}),
// this value doesn't matter
databaseDocumentID: '',
},
});
});
it('should have no flowto candidate for the origin', () => {
expect(selectors.ariaFlowtoCandidate(state())(originID)).toBe(null);
});
it('should have no flowto candidate for the first ancestor', () => {
expect(selectors.ariaFlowtoCandidate(state())(firstAncestorID)).toBe(null);
});
it('should have no flowto candidate for the second ancestor ancestor', () => {
expect(selectors.ariaFlowtoCandidate(state())(secondAncestorID)).toBe(null);
});
});
describe('with a tree with 2 children and no ancestors', () => {
const originID = 'c';
const firstChildID = 'd';
const secondChildID = 'e';
beforeEach(() => {
actions.push({
type: 'serverReturnedResolverData',
payload: {
result: mockTreeWithNoAncestorsAnd2Children({ originID, firstChildID, secondChildID }),
// this value doesn't matter
databaseDocumentID: '',
},
});
});
it('should have no flowto candidate for the origin', () => {
expect(selectors.ariaFlowtoCandidate(state())(originID)).toBe(null);
});
it('should use the second child as the flowto candidate for the first child', () => {
expect(selectors.ariaFlowtoCandidate(state())(firstChildID)).toBe(secondChildID);
});
it('should have no flowto candidate for the second child', () => {
expect(selectors.ariaFlowtoCandidate(state())(secondChildID)).toBe(null);
});
});
describe('with a tree where the root process has no parent info at all', () => {
const originID = 'c';
const firstChildID = 'd';
const secondChildID = 'e';
beforeEach(() => {
const tree = mockTreeWithNoAncestorsAnd2Children({ originID, firstChildID, secondChildID });
for (const event of tree.lifecycle) {
// delete the process.parent key, if present
// cast as `EndpointEvent` because `ResolverEvent` can also be `LegacyEndpointEvent` which has no `process` field
delete (event as EndpointEvent).process?.parent;
}

actions.push({
type: 'serverReturnedResolverData',
payload: {
result: tree,
// this value doesn't matter
databaseDocumentID: '',
},
});
});
it('should be able to calculate the aria flowto candidates for all processes nodes', () => {
const graphables = selectors.graphableProcesses(state());
expect(graphables.length).toBe(3);
for (const event of graphables) {
expect(() => {
selectors.ariaFlowtoCandidate(state())(uniquePidForProcess(event));
}).not.toThrow();
}
});
});
});
Loading

0 comments on commit 93c833c

Please sign in to comment.