Skip to content

Commit 9d4d985

Browse files
author
oatkiller
committed
fix aria flowto performance
1 parent e309652 commit 9d4d985

File tree

8 files changed

+312
-198
lines changed

8 files changed

+312
-198
lines changed

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

Lines changed: 24 additions & 26 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';
@@ -27,12 +29,12 @@ export function factory(
2729
const uniqueParentPid = uniqueParentPidForProcess(process);
2830
// if its defined and not ''
2931
if (uniqueParentPid) {
30-
let siblings = idToChildren.get(uniqueParentPid);
31-
if (!siblings) {
32-
siblings = [];
33-
idToChildren.set(uniqueParentPid, siblings);
32+
let childrenWithTheSameParent = idToChildren.get(uniqueParentPid);
33+
if (!childrenWithTheSameParent) {
34+
childrenWithTheSameParent = [];
35+
idToChildren.set(uniqueParentPid, childrenWithTheSameParent);
3436
}
35-
siblings.push(process);
37+
childrenWithTheSameParent.push(process);
3638
}
3739
}
3840

@@ -50,9 +52,8 @@ export function factory(
5052
/**
5153
* Returns an array with any children `ProcessEvent`s of the passed in `process`
5254
*/
53-
export function children(tree: IndexedProcessTree, process: ResolverEvent): ResolverEvent[] {
54-
const id = uniquePidForProcess(process);
55-
const currentProcessSiblings = tree.idToChildren.get(id);
55+
export function children(tree: IndexedProcessTree, parentID: string | undefined): ResolverEvent[] {
56+
const currentProcessSiblings = tree.idToChildren.get(parentID);
5657
return currentProcessSiblings === undefined ? [] : currentProcessSiblings;
5758
}
5859

@@ -81,26 +82,21 @@ export function parent(
8182
/**
8283
* Returns the following sibling
8384
*/
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);
85+
export function siblings(tree: IndexedProcessTree, node: ResolverEvent): ResolverEvent[] {
86+
// this can be undefined, since a node may have no parent.
87+
const parentID: string | undefined = uniqueParentPidForProcess(node);
9288

93-
// Find the sibling
94-
const index = siblings.indexOf(sibling);
89+
// nodes with the same parent ID.
90+
// if `node` has no parent ID, this is nodes with no parent ID.
91+
const childrenWithTheSameParent: undefined | ResolverEvent[] = tree.idToChildren.get(parentID);
9592

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];
93+
// this shouldn't happen if the node was in `tree`.
94+
if (!childrenWithTheSameParent) {
95+
return [];
10396
}
97+
98+
// Return all children with the same parent as `node`, except `node` itself.
99+
return [...childrenWithTheSameParent.filter((child) => child !== node)];
104100
}
105101

106102
/**
@@ -133,6 +129,8 @@ export function root(tree: IndexedProcessTree) {
133129
export function* levelOrder(tree: IndexedProcessTree) {
134130
const rootNode = root(tree);
135131
if (rootNode !== null) {
136-
yield* baseLevelOrder(rootNode, children.bind(null, tree));
132+
yield* baseLevelOrder(rootNode, (parentNode: ResolverEvent): ResolverEvent[] =>
133+
children(tree, uniquePidForProcess(parentNode))
134+
);
137135
}
138136
}

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/data/selectors.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ 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';
1216
describe('data state', () => {
1317
let actions: DataAction[] = [];
1418

@@ -263,4 +267,56 @@ describe('data state', () => {
263267
});
264268
});
265269
});
270+
describe('with a tree with no descendants and 2 ancestors', () => {
271+
const originID = 'c';
272+
const firstAncestorID = 'b';
273+
const secondAncestorID = 'a';
274+
beforeEach(() => {
275+
actions.push({
276+
type: 'serverReturnedResolverData',
277+
payload: {
278+
result: mockTreeWith2AncestorsAndNoChildren({
279+
originID,
280+
firstAncestorID,
281+
secondAncestorID,
282+
}),
283+
// this value doesn't matter
284+
databaseDocumentID: '',
285+
},
286+
});
287+
});
288+
it('should have no flowto candidate for the origin', () => {
289+
expect(selectors.ariFlowtoCandidate(state())(originID)).toBe(null);
290+
});
291+
it('should have no flowto candidate for the first ancestor', () => {
292+
expect(selectors.ariFlowtoCandidate(state())(firstAncestorID)).toBe(null);
293+
});
294+
it('should have no flowto candidate for the second ancestor ancestor', () => {
295+
expect(selectors.ariFlowtoCandidate(state())(secondAncestorID)).toBe(null);
296+
});
297+
});
298+
describe('with a tree with 2 children and no ancestors', () => {
299+
const originID = 'c';
300+
const firstChildID = 'd';
301+
const secondChildID = 'e';
302+
beforeEach(() => {
303+
actions.push({
304+
type: 'serverReturnedResolverData',
305+
payload: {
306+
result: mockTreeWithNoAncestorsAnd2Children({ originID, firstChildID, secondChildID }),
307+
// this value doesn't matter
308+
databaseDocumentID: '',
309+
},
310+
});
311+
});
312+
it('should have no flowto candidate for the origin', () => {
313+
expect(selectors.ariFlowtoCandidate(state())(originID)).toBe(null);
314+
});
315+
it('should use the second child as the flowto candidate for the first child', () => {
316+
expect(selectors.ariFlowtoCandidate(state())(firstChildID)).toBe(secondChildID);
317+
});
318+
it('should have no flowto candidate for the second child', () => {
319+
expect(selectors.ariFlowtoCandidate(state())(secondChildID)).toBe(null);
320+
});
321+
});
266322
});

0 commit comments

Comments
 (0)