Skip to content

Commit ffc28c7

Browse files
author
Robert Austin
authored
[Resolver] Selector performance (#72380) (#72433)
* Memoize various selectors * Improve performance of the selectors that calculate the `aria-flowto` attribute. * more tests.
1 parent 9ac0081 commit ffc28c7

File tree

11 files changed

+427
-280
lines changed

11 files changed

+427
-280
lines changed

x-pack/plugins/security_solution/common/endpoint/types.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,6 @@ export interface LegacyEndpointEvent {
482482
type: string;
483483
version: string;
484484
};
485-
process?: object;
486485
rule?: object;
487486
user?: object;
488487
event?: {

x-pack/plugins/security_solution/public/resolver/models/indexed_process_tree/index.ts

Lines changed: 15 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7+
/* eslint-disable no-shadow */
8+
79
import { uniquePidForProcess, uniqueParentPidForProcess, orderByTime } from '../process_event';
810
import { IndexedProcessTree } from '../../types';
911
import { ResolverEvent } from '../../../../common/endpoint/types';
@@ -24,16 +26,15 @@ export function factory(
2426
const uniqueProcessPid = uniquePidForProcess(process);
2527
idToValue.set(uniqueProcessPid, process);
2628

27-
const uniqueParentPid = uniqueParentPidForProcess(process);
28-
// if its defined and not ''
29-
if (uniqueParentPid) {
30-
let siblings = idToChildren.get(uniqueParentPid);
31-
if (!siblings) {
32-
siblings = [];
33-
idToChildren.set(uniqueParentPid, siblings);
34-
}
35-
siblings.push(process);
29+
// NB: If the value was null or undefined, use `undefined`
30+
const uniqueParentPid: string | undefined = uniqueParentPidForProcess(process) ?? undefined;
31+
32+
let childrenWithTheSameParent = idToChildren.get(uniqueParentPid);
33+
if (!childrenWithTheSameParent) {
34+
childrenWithTheSameParent = [];
35+
idToChildren.set(uniqueParentPid, childrenWithTheSameParent);
3636
}
37+
childrenWithTheSameParent.push(process);
3738
}
3839

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

@@ -78,31 +78,6 @@ export function parent(
7878
}
7979
}
8080

81-
/**
82-
* Returns the following sibling
83-
*/
84-
export function nextSibling(
85-
tree: IndexedProcessTree,
86-
sibling: ResolverEvent
87-
): ResolverEvent | undefined {
88-
const parentNode = parent(tree, sibling);
89-
if (parentNode) {
90-
// The siblings of `sibling` are the children of its parent.
91-
const siblings = children(tree, parentNode);
92-
93-
// Find the sibling
94-
const index = siblings.indexOf(sibling);
95-
96-
// if the sibling wasn't found, or if it was the last element in the array, return undefined
97-
if (index === -1 || index === siblings.length - 1) {
98-
return undefined;
99-
}
100-
101-
// return the next sibling
102-
return siblings[index + 1];
103-
}
104-
}
105-
10681
/**
10782
* Number of processes in the tree
10883
*/
@@ -133,6 +108,8 @@ export function root(tree: IndexedProcessTree) {
133108
export function* levelOrder(tree: IndexedProcessTree) {
134109
const rootNode = root(tree);
135110
if (rootNode !== null) {
136-
yield* baseLevelOrder(rootNode, children.bind(null, tree));
111+
yield* baseLevelOrder(rootNode, (parentNode: ResolverEvent): ResolverEvent[] =>
112+
children(tree, uniquePidForProcess(parentNode))
113+
);
137114
}
138115
}

x-pack/plugins/security_solution/public/resolver/models/indexed_process_tree/isometric_taxi_layout.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import * as event from '../../../../common/endpoint/models/event';
1919
import { ResolverEvent } from '../../../../common/endpoint/types';
2020
import * as model from './index';
2121
import { getFriendlyElapsedTime as elapsedTime } from '../../lib/date';
22+
import { uniquePidForProcess } from '../process_event';
2223

2324
/**
2425
* Graph the process tree
@@ -146,10 +147,12 @@ function widthsOfProcessSubtrees(indexedProcessTree: IndexedProcessTree): Proces
146147
return widths;
147148
}
148149

149-
const processesInReverseLevelOrder = [...model.levelOrder(indexedProcessTree)].reverse();
150+
const processesInReverseLevelOrder: ResolverEvent[] = [
151+
...model.levelOrder(indexedProcessTree),
152+
].reverse();
150153

151154
for (const process of processesInReverseLevelOrder) {
152-
const children = model.children(indexedProcessTree, process);
155+
const children = model.children(indexedProcessTree, uniquePidForProcess(process));
153156

154157
const sumOfWidthOfChildren = function sumOfWidthOfChildren() {
155158
return children.reduce(function sum(currentValue, child) {
@@ -226,7 +229,7 @@ function processEdgeLineSegments(
226229
metadata: edgeLineMetadata,
227230
};
228231

229-
const siblings = model.children(indexedProcessTree, parent);
232+
const siblings = model.children(indexedProcessTree, uniquePidForProcess(parent));
230233
const isFirstChild = process === siblings[0];
231234

232235
if (metadata.isOnlyChild) {
@@ -420,7 +423,7 @@ function* levelOrderWithWidths(
420423
parentWidth,
421424
};
422425

423-
const siblings = model.children(tree, parent);
426+
const siblings = model.children(tree, uniquePidForProcess(parent));
424427
if (siblings.length === 1) {
425428
metadata.isOnlyChild = true;
426429
metadata.lastChildWidth = width;

x-pack/plugins/security_solution/public/resolver/store/camera/selectors.ts

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,8 @@ export const scale: (state: CameraState) => (time: number) => Vector2 = createSe
164164
scalingConstants.maximum
165165
);
166166

167-
return (time) => {
167+
// memoizing this so the vector returned will be the same object reference if called with the same `time`.
168+
return defaultMemoize((time) => {
168169
/**
169170
* If the animation has completed, return the `scaleNotCountingAnimation`, as
170171
* the animation always completes with the scale set back at starting value.
@@ -247,12 +248,13 @@ export const scale: (state: CameraState) => (time: number) => Vector2 = createSe
247248
*/
248249
return [lerpedScale, lerpedScale];
249250
}
250-
};
251+
});
251252
} else {
252253
/**
253254
* The scale should be the same in both axes.
255+
* Memoizing this so the vector returned will be the same object reference every time.
254256
*/
255-
return () => [scaleNotCountingAnimation, scaleNotCountingAnimation];
257+
return defaultMemoize(() => [scaleNotCountingAnimation, scaleNotCountingAnimation]);
256258
}
257259

258260
/**
@@ -277,22 +279,26 @@ export const clippingPlanes: (
277279
) => (time: number) => ClippingPlanes = createSelector(
278280
(state) => state.rasterSize,
279281
scale,
280-
(rasterSize, scaleAtTime) => (time: number) => {
281-
const [scaleX, scaleY] = scaleAtTime(time);
282-
const renderWidth = rasterSize[0];
283-
const renderHeight = rasterSize[1];
284-
const clippingPlaneRight = renderWidth / 2 / scaleX;
285-
const clippingPlaneTop = renderHeight / 2 / scaleY;
286-
287-
return {
288-
renderWidth,
289-
renderHeight,
290-
clippingPlaneRight,
291-
clippingPlaneTop,
292-
clippingPlaneLeft: -clippingPlaneRight,
293-
clippingPlaneBottom: -clippingPlaneTop,
294-
};
295-
}
282+
(rasterSize, scaleAtTime) =>
283+
/**
284+
* memoizing this for object reference equality.
285+
*/
286+
defaultMemoize((time: number) => {
287+
const [scaleX, scaleY] = scaleAtTime(time);
288+
const renderWidth = rasterSize[0];
289+
const renderHeight = rasterSize[1];
290+
const clippingPlaneRight = renderWidth / 2 / scaleX;
291+
const clippingPlaneTop = renderHeight / 2 / scaleY;
292+
293+
return {
294+
renderWidth,
295+
renderHeight,
296+
clippingPlaneRight,
297+
clippingPlaneTop,
298+
clippingPlaneLeft: -clippingPlaneRight,
299+
clippingPlaneBottom: -clippingPlaneTop,
300+
};
301+
})
296302
);
297303

298304
/**
@@ -323,7 +329,10 @@ export const translation: (state: CameraState) => (time: number) => Vector2 = cr
323329
scale,
324330
(state) => state.animation,
325331
(panning, translationNotCountingCurrentPanning, scaleAtTime, animation) => {
326-
return (time: number) => {
332+
/**
333+
* Memoizing this for object reference equality.
334+
*/
335+
return defaultMemoize((time: number) => {
327336
const [scaleX, scaleY] = scaleAtTime(time);
328337
if (animation !== undefined && animationIsActive(animation, time)) {
329338
return vector2.lerp(
@@ -343,7 +352,7 @@ export const translation: (state: CameraState) => (time: number) => Vector2 = cr
343352
} else {
344353
return translationNotCountingCurrentPanning;
345354
}
346-
};
355+
});
347356
}
348357
);
349358

@@ -357,7 +366,10 @@ export const inverseProjectionMatrix: (
357366
clippingPlanes,
358367
translation,
359368
(clippingPlanesAtTime, translationAtTime) => {
360-
return (time: number) => {
369+
/**
370+
* Memoizing this for object reference equality (and reduced memory churn.)
371+
*/
372+
return defaultMemoize((time: number) => {
361373
const {
362374
renderWidth,
363375
renderHeight,
@@ -404,7 +416,7 @@ export const inverseProjectionMatrix: (
404416
translateForCamera,
405417
multiply(scaleToClippingPlaneDimensions, multiply(invertY, screenToNDC))
406418
);
407-
};
419+
});
408420
}
409421
);
410422

@@ -415,7 +427,8 @@ export const viewableBoundingBox: (state: CameraState) => (time: number) => AABB
415427
clippingPlanes,
416428
inverseProjectionMatrix,
417429
(clippingPlanesAtTime, matrixAtTime) => {
418-
return (time: number) => {
430+
// memoizing this so the AABB returned will be the same object reference if called with the same `time`.
431+
return defaultMemoize((time: number) => {
419432
const { renderWidth, renderHeight } = clippingPlanesAtTime(time);
420433
const matrix = matrixAtTime(time);
421434
const bottomLeftCorner: Vector2 = [0, renderHeight];
@@ -424,7 +437,7 @@ export const viewableBoundingBox: (state: CameraState) => (time: number) => AABB
424437
minimum: vector2.applyMatrix3(bottomLeftCorner, matrix),
425438
maximum: vector2.applyMatrix3(topRightCorner, matrix),
426439
};
427-
};
440+
});
428441
}
429442
);
430443

@@ -436,6 +449,8 @@ export const projectionMatrix: (state: CameraState) => (time: number) => Matrix3
436449
clippingPlanes,
437450
translation,
438451
(clippingPlanesAtTime, translationAtTime) => {
452+
// memoizing this so the matrix returned will be the same object reference if called with the same `time`.
453+
// this should also save on some memory allocation
439454
return defaultMemoize((time: number) => {
440455
const {
441456
renderWidth,

x-pack/plugins/security_solution/public/resolver/store/data/selectors.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ import { DataState } from '../../types';
99
import { dataReducer } from './reducer';
1010
import { DataAction } from './action';
1111
import { createStore } from 'redux';
12+
import {
13+
mockTreeWithNoAncestorsAnd2Children,
14+
mockTreeWith2AncestorsAndNoChildren,
15+
} from '../mocks/resolver_tree';
16+
import { uniquePidForProcess } from '../../models/process_event';
17+
import { EndpointEvent } from '../../../../common/endpoint/types';
18+
1219
describe('data state', () => {
1320
let actions: DataAction[] = [];
1421

@@ -263,4 +270,87 @@ describe('data state', () => {
263270
});
264271
});
265272
});
273+
describe('with a tree with no descendants and 2 ancestors', () => {
274+
const originID = 'c';
275+
const firstAncestorID = 'b';
276+
const secondAncestorID = 'a';
277+
beforeEach(() => {
278+
actions.push({
279+
type: 'serverReturnedResolverData',
280+
payload: {
281+
result: mockTreeWith2AncestorsAndNoChildren({
282+
originID,
283+
firstAncestorID,
284+
secondAncestorID,
285+
}),
286+
// this value doesn't matter
287+
databaseDocumentID: '',
288+
},
289+
});
290+
});
291+
it('should have no flowto candidate for the origin', () => {
292+
expect(selectors.ariaFlowtoCandidate(state())(originID)).toBe(null);
293+
});
294+
it('should have no flowto candidate for the first ancestor', () => {
295+
expect(selectors.ariaFlowtoCandidate(state())(firstAncestorID)).toBe(null);
296+
});
297+
it('should have no flowto candidate for the second ancestor ancestor', () => {
298+
expect(selectors.ariaFlowtoCandidate(state())(secondAncestorID)).toBe(null);
299+
});
300+
});
301+
describe('with a tree with 2 children and no ancestors', () => {
302+
const originID = 'c';
303+
const firstChildID = 'd';
304+
const secondChildID = 'e';
305+
beforeEach(() => {
306+
actions.push({
307+
type: 'serverReturnedResolverData',
308+
payload: {
309+
result: mockTreeWithNoAncestorsAnd2Children({ originID, firstChildID, secondChildID }),
310+
// this value doesn't matter
311+
databaseDocumentID: '',
312+
},
313+
});
314+
});
315+
it('should have no flowto candidate for the origin', () => {
316+
expect(selectors.ariaFlowtoCandidate(state())(originID)).toBe(null);
317+
});
318+
it('should use the second child as the flowto candidate for the first child', () => {
319+
expect(selectors.ariaFlowtoCandidate(state())(firstChildID)).toBe(secondChildID);
320+
});
321+
it('should have no flowto candidate for the second child', () => {
322+
expect(selectors.ariaFlowtoCandidate(state())(secondChildID)).toBe(null);
323+
});
324+
});
325+
describe('with a tree where the root process has no parent info at all', () => {
326+
const originID = 'c';
327+
const firstChildID = 'd';
328+
const secondChildID = 'e';
329+
beforeEach(() => {
330+
const tree = mockTreeWithNoAncestorsAnd2Children({ originID, firstChildID, secondChildID });
331+
for (const event of tree.lifecycle) {
332+
// delete the process.parent key, if present
333+
// cast as `EndpointEvent` because `ResolverEvent` can also be `LegacyEndpointEvent` which has no `process` field
334+
delete (event as EndpointEvent).process?.parent;
335+
}
336+
337+
actions.push({
338+
type: 'serverReturnedResolverData',
339+
payload: {
340+
result: tree,
341+
// this value doesn't matter
342+
databaseDocumentID: '',
343+
},
344+
});
345+
});
346+
it('should be able to calculate the aria flowto candidates for all processes nodes', () => {
347+
const graphables = selectors.graphableProcesses(state());
348+
expect(graphables.length).toBe(3);
349+
for (const event of graphables) {
350+
expect(() => {
351+
selectors.ariaFlowtoCandidate(state())(uniquePidForProcess(event));
352+
}).not.toThrow();
353+
}
354+
});
355+
});
266356
});

0 commit comments

Comments
 (0)