Skip to content

Commit

Permalink
move asset selection graph building to webworker
Browse files Browse the repository at this point in the history
  • Loading branch information
salazarm committed Dec 6, 2024
1 parent f6686c6 commit 6fb58ba
Show file tree
Hide file tree
Showing 17 changed files with 272 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import {FeatureFlag} from 'shared/app/FeatureFlags.oss';
* Default values for feature flags when they are unset.
*/
export const DEFAULT_FEATURE_FLAG_VALUES: Partial<Record<FeatureFlag, boolean>> = {
[FeatureFlag.flagAssetSelectionWorker]: true,

// Flags for tests
[FeatureFlag.__TestFlagDefaultTrue]: true,
[FeatureFlag.__TestFlagDefaultFalse]: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export enum FeatureFlag {
flagLegacyRunsPage = 'flagLegacyRunsPage',
flagAssetSelectionSyntax = 'flagAssetSelectionSyntax',
flagRunSelectionSyntax = 'flagRunSelectionSyntax',
flagAssetSelectionWorker = 'flagAssetSelectionWorker',

// Flags for tests
__TestFlagDefaultNone = '__TestFlagDefaultNone',
Expand Down
12 changes: 6 additions & 6 deletions js_modules/dagster-ui/packages/ui-core/src/app/Flags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const initializeFeatureFlags = () => {
flags.forEach((flag: FeatureFlag) => {
migratedFlags[flag] = true;
});
setFeatureFlagsInternal(migratedFlags, false); // Prevent broadcasting during migration
setFeatureFlagsInternal(migratedFlags);
flags = migratedFlags;
}

Expand All @@ -37,16 +37,15 @@ const initializeFeatureFlags = () => {

/**
* Internal function to set feature flags without broadcasting.
* Used during initialization and migration.
* Used during initialization and migration and by web-workers.
*/
const setFeatureFlagsInternal = (flags: FeatureFlagMap, broadcast: boolean = true) => {
export const setFeatureFlagsInternal = (flags: FeatureFlagMap) => {
if (typeof flags !== 'object' || Array.isArray(flags)) {
throw new Error('flags must be an object mapping FeatureFlag to boolean values');
}
currentFeatureFlags = flags;
localStorage.setItem(DAGSTER_FLAGS_KEY, JSON.stringify(flags));
if (broadcast) {
featureFlagsChannel.postMessage('updated');
if (typeof localStorage !== 'undefined') {
localStorage.setItem(DAGSTER_FLAGS_KEY, JSON.stringify(flags));
}
};

Expand Down Expand Up @@ -128,4 +127,5 @@ export const useFeatureFlags = (): Readonly<Record<FeatureFlag, boolean>> => {
*/
export const setFeatureFlags = (flags: FeatureFlagMap) => {
setFeatureFlagsInternal(flags);
featureFlagsChannel.postMessage('updated');
};
2 changes: 1 addition & 1 deletion js_modules/dagster-ui/packages/ui-core/src/app/Util.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function asyncMemoize<T, R, U extends (arg: T, ...rest: any[]) => Promise
hashFn?: (arg: T, ...rest: any[]) => any,
hashSize?: number,
): U {
const cache = new LRU(hashSize || 50);
const cache = new LRU<any, R>(hashSize || 50);
return (async (arg: T, ...rest: any[]) => {
const key = hashFn ? hashFn(arg, ...rest) : arg;
if (cache.has(key)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,13 @@ export const AssetGraphExplorer = (props: Props) => {
() => (_node: AssetNodeForGraphQueryFragment) => true,
);

const {fetchResult, assetGraphData, graphQueryItems, allAssetKeys} = useAssetGraphData(
props.explorerPath.opsQuery,
{...props.fetchOptions, hideNodesMatching},
);
const {
loading: graphDataLoading,
fetchResult,
assetGraphData,
graphQueryItems,
allAssetKeys,
} = useAssetGraphData(props.explorerPath.opsQuery, {...props.fetchOptions, hideNodesMatching});

const {explorerPath, onChangeExplorerPath} = props;

Expand All @@ -119,7 +122,7 @@ export const AssetGraphExplorer = (props: Props) => {
() => (fullAssetGraphData ? Object.values(fullAssetGraphData.nodes) : []),
[fullAssetGraphData],
),
loading: fetchResult.loading,
loading: graphDataLoading,
viewType: props.viewType,
assetSelection: explorerPath.opsQuery,
setAssetSelection: React.useCallback(
Expand Down Expand Up @@ -169,7 +172,7 @@ export const AssetGraphExplorer = (props: Props) => {
filterButton={button}
kindFilter={kindFilter}
groupsFilter={groupsFilter}
filteredAssetsLoading={filteredAssetsLoading}
loading={filteredAssetsLoading || graphDataLoading}
{...props}
/>
);
Expand All @@ -183,7 +186,7 @@ type WithDataProps = Props & {
assetGraphData: GraphData;
fullAssetGraphData: GraphData;
graphQueryItems: AssetGraphQueryItem[];
filteredAssetsLoading: boolean;
loading: boolean;

filterButton: React.ReactNode;
filterBar: React.ReactNode;
Expand All @@ -209,7 +212,7 @@ const AssetGraphExplorerWithData = ({
viewType,
kindFilter,
groupsFilter,
filteredAssetsLoading,
loading: dataLoading,
}: WithDataProps) => {
const findAssetLocation = useFindAssetLocation();
const [highlighted, setHighlighted] = React.useState<string[] | null>(null);
Expand All @@ -235,7 +238,11 @@ const AssetGraphExplorerWithData = ({
});
const focusGroupIdAfterLayoutRef = React.useRef('');

const {layout, loading, async} = useAssetLayout(
const {
layout,
loading: layoutLoading,
async,
} = useAssetLayout(
assetGraphData,
expandedGroups,
useMemo(() => ({direction}), [direction]),
Expand Down Expand Up @@ -665,6 +672,8 @@ const AssetGraphExplorerWithData = ({
</SVGViewport>
) : null;

const loading = layoutLoading || dataLoading;

const explorer = (
<SplitPanelContainer
key="explorer"
Expand All @@ -673,7 +682,7 @@ const AssetGraphExplorerWithData = ({
firstMinSize={400}
secondMinSize={400}
first={
filteredAssetsLoading ? (
loading ? (
<LoadingContainer>
<Box margin={{bottom: 24}}>Loading assets…</Box>
<Spinner purpose="page" />
Expand Down Expand Up @@ -769,7 +778,7 @@ const AssetGraphExplorerWithData = ({
)
}
second={
filteredAssetsLoading ? null : selectedGraphNodes.length === 1 && selectedGraphNodes[0] ? (
loading ? null : selectedGraphNodes.length === 1 && selectedGraphNodes[0] ? (
<RightInfoPanel>
<RightInfoPanelContent>
<ErrorBoundary region="asset sidebar" resetErrorOnChange={[selectedGraphNodes[0].id]}>
Expand Down Expand Up @@ -814,7 +823,7 @@ const AssetGraphExplorerWithData = ({
setShowSidebar(false);
}}
onFilterToGroup={onFilterToGroup}
loading={filteredAssetsLoading}
loading={loading}
/>
}
second={explorer}
Expand Down
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];
}
}
}
};
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;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {FeatureFlag} from 'shared/app/FeatureFlags.oss';

import {computeGraphData} from './ComputeGraphData';
import {ComputeGraphDataMessageType} from './ComputeGraphData.types';
import {setFeatureFlags} from '../app/Flags';

type WorkerMessageData = ComputeGraphDataMessageType;

self.addEventListener('message', async (event: MessageEvent & {data: WorkerMessageData}) => {
const {data} = event;

if (data.type === 'computeGraphData') {
if (data.flagAssetSelectionSyntax) {
setFeatureFlags({[FeatureFlag.flagAssetSelectionSyntax]: true});
}
const state = await computeGraphData(data);
self.postMessage(state);
}
});

self.onmessage = function (event) {
if (event.data === 'close') {
self.close(); // Terminates the worker
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export const graphHasCycles = (graphData: GraphData) => {
};
let hasCycles = false;
while (nodes.size !== 0 && !hasCycles) {
hasCycles = search([], nodes.values().next().value);
hasCycles = search([], nodes.values().next().value!);
}
return hasCycles;
};
Expand Down
Loading

0 comments on commit 6fb58ba

Please sign in to comment.