-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Move asset selection filtering to webworker (#26315)
## Summary & Motivation Move our heavy asset selection filtering to a webworker. Added a feature flag as an escape hatch. ## How I Tested These Changes Tested primarily with cloud-proxy with customers that have large graphs.
- Loading branch information
Showing
30 changed files
with
681 additions
and
150 deletions.
There are no files selected for viewing
36 changes: 36 additions & 0 deletions
36
js_modules/dagster-ui/packages/ui-core/jest/mocks/ComputeGraphData.worker.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import {setFeatureFlagsInternal} from '../../src/app/Flags'; | ||
| import {computeGraphData} from '../../src/asset-graph/ComputeGraphData'; | ||
| import {ComputeGraphDataMessageType} from '../../src/asset-graph/ComputeGraphData.types'; | ||
|
|
||
| // eslint-disable-next-line import/no-default-export | ||
| export default class MockWorker { | ||
| onmessage = (_: any) => {}; | ||
|
|
||
| addEventListener(_type: string, handler: any) { | ||
| this.onmessage = handler; | ||
| } | ||
|
|
||
| // mock expects data: { } instead of e: { data: { } } | ||
| async postMessage(data: ComputeGraphDataMessageType) { | ||
| if (data.type === 'computeGraphData') { | ||
| if (data.flagAssetSelectionSyntax) { | ||
| setFeatureFlagsInternal({flagAssetSelectionSyntax: true}); | ||
| } | ||
| const state = await computeGraphData(data); | ||
| this.onmessage({data: state}); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const originalWorker = global.Worker; | ||
| // @ts-expect-error - test shenanigans | ||
| global.Worker = function ComputeGraphDataMockWorkerWrapper( | ||
| url: string | URL, | ||
| opts?: WorkerOptions, | ||
| ) { | ||
| if (url.toString().endsWith('ComputeGraphData.worker')) { | ||
| return new MockWorker(); | ||
| } else { | ||
| return new originalWorker(url, opts); | ||
| } | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
79 changes: 79 additions & 0 deletions
79
js_modules/dagster-ui/packages/ui-core/src/asset-graph/ComputeGraphData.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| import groupBy from 'lodash/groupBy'; | ||
|
|
||
| import {ComputeGraphDataMessageType} from './ComputeGraphData.types'; | ||
| import {GraphData, buildGraphData, toGraphId} from './Utils'; | ||
| import {AssetNodeForGraphQueryFragment} from './types/useAssetGraphData.types'; | ||
| import {GraphDataState} from './useAssetGraphData'; | ||
| import {filterAssetSelectionByQuery} from '../asset-selection/AntlrAssetSelection'; | ||
| import {doesFilterArrayMatchValueArray} from '../ui/Filters/doesFilterArrayMatchValueArray'; | ||
|
|
||
| export function computeGraphData({ | ||
| repoFilteredNodes, | ||
| graphQueryItems, | ||
| opsQuery, | ||
| kinds: _kinds, | ||
| hideEdgesToNodesOutsideQuery, | ||
| }: Omit<ComputeGraphDataMessageType, 'type'>): GraphDataState { | ||
| if (repoFilteredNodes === undefined || graphQueryItems === undefined) { | ||
| return { | ||
| allAssetKeys: [], | ||
| graphAssetKeys: [], | ||
| assetGraphData: null, | ||
| }; | ||
| } | ||
|
|
||
| // Filter the set of all AssetNodes down to those matching the `opsQuery`. | ||
| // In the future it might be ideal to move this server-side, but we currently | ||
| // get to leverage the useQuery cache almost 100% of the time above, making this | ||
| // super fast after the first load vs a network fetch on every page view. | ||
| const {all: allFilteredByOpQuery} = filterAssetSelectionByQuery(graphQueryItems, opsQuery); | ||
| const kinds = _kinds?.map((c) => c.toLowerCase()); | ||
| const all = kinds?.length | ||
| ? allFilteredByOpQuery.filter( | ||
| ({node}) => | ||
| node.kinds && | ||
| doesFilterArrayMatchValueArray( | ||
| kinds, | ||
| node.kinds.map((k) => k.toLowerCase()), | ||
| ), | ||
| ) | ||
| : allFilteredByOpQuery; | ||
|
|
||
| // Assemble the response into the data structure used for layout, traversal, etc. | ||
| const assetGraphData = buildGraphData(all.map((n) => n.node)); | ||
| if (hideEdgesToNodesOutsideQuery) { | ||
| removeEdgesToHiddenAssets(assetGraphData, repoFilteredNodes); | ||
| } | ||
|
|
||
| return { | ||
| allAssetKeys: repoFilteredNodes.map((n) => n.assetKey), | ||
| graphAssetKeys: all.map((n) => ({path: n.node.assetKey.path})), | ||
| assetGraphData, | ||
| }; | ||
| } | ||
|
|
||
| const removeEdgesToHiddenAssets = ( | ||
| graphData: GraphData, | ||
| allNodes: AssetNodeForGraphQueryFragment[], | ||
| ) => { | ||
| const allNodesById = groupBy(allNodes, (n) => toGraphId(n.assetKey)); | ||
| const notSourceAsset = (id: string) => !!allNodesById[id]; | ||
|
|
||
| for (const node of Object.keys(graphData.upstream)) { | ||
| for (const edge of Object.keys(graphData.upstream[node]!)) { | ||
| if (!graphData.nodes[edge] && notSourceAsset(node)) { | ||
| delete graphData.upstream[node]![edge]; | ||
| delete graphData.downstream[edge]![node]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for (const node of Object.keys(graphData.downstream)) { | ||
| for (const edge of Object.keys(graphData.downstream[node]!)) { | ||
| if (!graphData.nodes[edge] && notSourceAsset(node)) { | ||
| delete graphData.upstream[edge]![node]; | ||
| delete graphData.downstream[node]![edge]; | ||
| } | ||
| } | ||
| } | ||
| }; |
12 changes: 12 additions & 0 deletions
12
js_modules/dagster-ui/packages/ui-core/src/asset-graph/ComputeGraphData.types.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import {AssetNodeForGraphQueryFragment} from './types/useAssetGraphData.types'; | ||
| import {AssetGraphFetchScope, AssetGraphQueryItem} from './useAssetGraphData'; | ||
|
|
||
| export type ComputeGraphDataMessageType = { | ||
| type: 'computeGraphData'; | ||
| repoFilteredNodes?: AssetNodeForGraphQueryFragment[]; | ||
| graphQueryItems?: AssetGraphQueryItem[]; | ||
| opsQuery: string; | ||
| kinds: AssetGraphFetchScope['kinds']; | ||
| hideEdgesToNodesOutsideQuery?: boolean; | ||
| flagAssetSelectionSyntax?: boolean; | ||
| }; |
Oops, something went wrong.
b0bcb20There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deploy preview for dagit-core-storybook ready!
✅ Preview
https://dagit-core-storybook-chouxcc75-elementl.vercel.app
Built with commit b0bcb20.
This pull request is being automatically deployed with vercel-action