From 17b91763bb0d06d30fc0920714975825572e084f Mon Sep 17 00:00:00 2001 From: Juntao Wang Date: Fri, 21 Jun 2024 10:58:56 -0400 Subject: [PATCH] Fix markdown tab render delay when toggling selected compared runs --- .../markdown/MarkdownCompare.tsx | 84 +++-------------- .../markdown/useFetchMarkdownMaps.ts | 90 +++++++++++++++++++ .../compareRuns/CompareRunsMetricsSection.tsx | 50 +++++++---- 3 files changed, 135 insertions(+), 89 deletions(-) create mode 100644 frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/useFetchMarkdownMaps.ts diff --git a/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare.tsx b/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare.tsx index 3d8d222d72..9c3160d241 100644 --- a/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare.tsx +++ b/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare.tsx @@ -13,29 +13,17 @@ import { Stack, StackItem, } from '@patternfly/react-core'; - -import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; - -import { RunArtifact } from '~/concepts/pipelines/apiHooks/mlmd/types'; -import { FullArtifactPath } from '~/concepts/pipelines/content/compareRuns/metricsSection/types'; -import { - getFullArtifactPaths, - getFullArtifactPathLabel, -} from '~/concepts/pipelines/content/compareRuns/metricsSection/utils'; import { CompareRunsEmptyState } from '~/concepts/pipelines/content/compareRuns/CompareRunsEmptyState'; import { PipelineRunArtifactSelect } from '~/concepts/pipelines/content/compareRuns/metricsSection/PipelineRunArtifactSelect'; import MarkdownView from '~/components/MarkdownView'; -import { - MAX_STORAGE_OBJECT_SIZE, - fetchStorageObject, - fetchStorageObjectSize, -} from '~/services/storageService'; -import { usePipelinesAPI } from '~/concepts/pipelines/context'; -import { extractS3UriComponents } from '~/concepts/pipelines/content/artifacts/utils'; +import { MAX_STORAGE_OBJECT_SIZE } from '~/services/storageService'; import { bytesAsRoundedGiB } from '~/utilities/number'; +import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; type MarkdownCompareProps = { - runArtifacts?: RunArtifact[]; + configMap: Record; + runMap: Record; + isEmpty: boolean; isLoaded: boolean; }; @@ -45,60 +33,13 @@ export type MarkdownAndTitle = { fileSize?: number; }; -const MarkdownCompare: React.FC = ({ runArtifacts, isLoaded }) => { +const MarkdownCompare: React.FC = ({ + configMap, + runMap, + isLoaded, + isEmpty, +}) => { const [expandedGraph, setExpandedGraph] = React.useState(undefined); - const { namespace } = usePipelinesAPI(); - - const fullArtifactPaths: FullArtifactPath[] = React.useMemo(() => { - if (!runArtifacts) { - return []; - } - - return getFullArtifactPaths(runArtifacts); - }, [runArtifacts]); - - const { configMap, runMap } = React.useMemo(() => { - const configMapBuilder: Record = {}; - const runMapBuilder: Record = {}; - - fullArtifactPaths - .map((fullPath) => ({ - run: fullPath.run, - title: getFullArtifactPathLabel(fullPath), - uri: fullPath.linkedArtifact.artifact.getUri(), - })) - .filter((markdown) => !!markdown.uri) - .forEach(async ({ uri, title, run }) => { - const uriComponents = extractS3UriComponents(uri); - if (!uriComponents) { - return; - } - const sizeBytes = await fetchStorageObjectSize(namespace, uriComponents.path).catch( - () => undefined, - ); - const text = await fetchStorageObject(namespace, uriComponents.path).catch(() => null); - - if (text === null) { - return; - } - - runMapBuilder[run.run_id] = run; - - const config = { - title, - config: text, - fileSize: sizeBytes, - }; - - if (run.run_id in configMapBuilder) { - configMapBuilder[run.run_id].push(config); - } else { - configMapBuilder[run.run_id] = [config]; - } - }); - - return { configMap: configMapBuilder, runMap: runMapBuilder }; - }, [fullArtifactPaths, namespace]); if (!isLoaded) { return ( @@ -108,9 +49,10 @@ const MarkdownCompare: React.FC = ({ runArtifacts, isLoade ); } - if (!runArtifacts || runArtifacts.length === 0) { + if (isEmpty) { return ; } + if (Object.keys(configMap).length === 0) { return ( diff --git a/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/useFetchMarkdownMaps.ts b/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/useFetchMarkdownMaps.ts new file mode 100644 index 0000000000..efd17e8b31 --- /dev/null +++ b/frontend/src/concepts/pipelines/content/compareRuns/metricsSection/markdown/useFetchMarkdownMaps.ts @@ -0,0 +1,90 @@ +import React from 'react'; +import { RunArtifact } from '~/concepts/pipelines/apiHooks/mlmd/types'; +import { extractS3UriComponents } from '~/concepts/pipelines/content/artifacts/utils'; +import { MarkdownAndTitle } from '~/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare'; +import { + getFullArtifactPathLabel, + getFullArtifactPaths, +} from '~/concepts/pipelines/content/compareRuns/metricsSection/utils'; +import { usePipelinesAPI } from '~/concepts/pipelines/context'; +import { PipelineRunKFv2 } from '~/concepts/pipelines/kfTypes'; +import { fetchStorageObject, fetchStorageObjectSize } from '~/services/storageService'; +import { allSettledPromises } from '~/utilities/allSettledPromises'; + +const useFetchMarkdownMaps = ( + markdownArtifacts?: RunArtifact[], +): { + configMap: Record; + runMap: Record; + configsLoaded: boolean; +} => { + const { namespace } = usePipelinesAPI(); + const [configsLoaded, setConfigsLoaded] = React.useState(false); + const [configMapBuilder, setConfigMapBuilder] = React.useState< + Record + >({}); + const [runMapBuilder, setRunMapBuilder] = React.useState>({}); + + const fullArtifactPaths = React.useMemo(() => { + if (!markdownArtifacts) { + return []; + } + + return getFullArtifactPaths(markdownArtifacts); + }, [markdownArtifacts]); + + const fetchStorageObjectPromises = React.useMemo( + () => + fullArtifactPaths + .filter((path) => !!path.linkedArtifact.artifact.getUri()) + .map(async (path) => { + const { run } = path; + const uriComponents = extractS3UriComponents(path.linkedArtifact.artifact.getUri()); + if (!uriComponents) { + return null; + } + const sizeBytes = await fetchStorageObjectSize(namespace, uriComponents.path).catch( + () => undefined, + ); + const text = await fetchStorageObject(namespace, uriComponents.path).catch(() => null); + + if (text === null) { + return null; + } + + return { run, sizeBytes, text, path }; + }), + [fullArtifactPaths, namespace], + ); + + React.useEffect(() => { + setConfigsLoaded(false); + setConfigMapBuilder({}); + setRunMapBuilder({}); + + allSettledPromises(fetchStorageObjectPromises).then(([successes]) => { + successes.forEach((result) => { + if (result.value) { + const { text, sizeBytes, run, path } = result.value; + setRunMapBuilder((runMap) => ({ ...runMap, [run.run_id]: run })); + + const config = { + title: getFullArtifactPathLabel(path), + config: text, + fileSize: sizeBytes, + }; + + setConfigMapBuilder((configMap) => ({ + ...configMap, + [run.run_id]: run.run_id in configMap ? [...configMap[run.run_id], config] : [config], + })); + } + }); + setConfigsLoaded(true); + }); + }, [fetchStorageObjectPromises]); + + return { configMap: configMapBuilder, runMap: runMapBuilder, configsLoaded }; +}; + +export default useFetchMarkdownMaps; diff --git a/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsMetricsSection.tsx b/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsMetricsSection.tsx index 6685d3e399..14792d8bb1 100644 --- a/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsMetricsSection.tsx +++ b/frontend/src/pages/pipelines/global/experiments/compareRuns/CompareRunsMetricsSection.tsx @@ -1,5 +1,5 @@ import React from 'react'; - +import _ from 'lodash-es'; import { ExpandableSection, Tab, TabContentBody, TabTitleText, Tabs } from '@patternfly/react-core'; import { useCompareRuns } from '~/concepts/pipelines/content/compareRuns/CompareRunsContext'; import { useGetArtifactTypes } from '~/concepts/pipelines/apiHooks/mlmd/useGetArtifactTypes'; @@ -18,6 +18,7 @@ import RocCurveCompare from '~/concepts/pipelines/content/compareRuns/metricsSec import ConfusionMatrixCompare from '~/concepts/pipelines/content/compareRuns/metricsSection/confusionMatrix/ConfusionMatrixCompare'; import MarkdownCompare from '~/concepts/pipelines/content/compareRuns/metricsSection/markdown/MarkdownCompare'; import { SupportedArea, useIsAreaAvailable } from '~/concepts/areas'; +import useFetchMarkdownMaps from '~/concepts/pipelines/content/compareRuns/metricsSection/markdown/useFetchMarkdownMaps'; export const CompareRunMetricsSection: React.FunctionComponent = () => { const { runs, selectedRuns } = useCompareRuns(); @@ -29,39 +30,47 @@ export const CompareRunMetricsSection: React.FunctionComponent = () => { ); const isS3EndpointAvailable = useIsAreaAvailable(SupportedArea.S3_ENDPOINT).status; - const selectedMlmdPackages = React.useMemo( - () => - mlmdPackages.filter((mlmdPackage) => - selectedRuns.some((run) => run.run_id === mlmdPackage.run.run_id), - ), - [mlmdPackages, selectedRuns], + const runArtifacts: RunArtifact[] = React.useMemo( + () => getRunArtifacts(mlmdPackages), + [mlmdPackages], ); const isLoaded = mlmdPackagesLoaded && artifactTypesLoaded; - const [ - scalarMetricsArtifactData, - confusionMatrixArtifactData, - rocCurveArtifactData, - markdownArtifactData, - ] = React.useMemo(() => { + const [markdownArtifacts, ...allArtifacts] = React.useMemo(() => { if (!isLoaded) { return [[], [], [], []]; } - const runArtifacts: RunArtifact[] = getRunArtifacts(selectedMlmdPackages); return [ + filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.MARKDOWN), filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.SCALAR_METRICS), filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.CONFUSION_MATRIX), filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.ROC_CURVE), - filterRunArtifactsByType(runArtifacts, artifactTypes, MetricsType.MARKDOWN), ]; - }, [artifactTypes, isLoaded, selectedMlmdPackages]); + }, [artifactTypes, isLoaded, runArtifacts]); + + const { configMap, configsLoaded, runMap } = useFetchMarkdownMaps(markdownArtifacts); + const [selectedConfigMap, selectedRunMap] = React.useMemo(() => { + const selectedIds = selectedRuns.map((run) => run.run_id); + return [_.pick(configMap, selectedIds), _.pick(runMap, selectedIds)]; + }, [configMap, runMap, selectedRuns]); + + const filterSelected = React.useCallback( + (runArtifact: RunArtifact) => selectedRuns.some((run) => run.run_id === runArtifact.run.run_id), + [selectedRuns], + ); + + const [scalarMetricsArtifactData, confusionMatrixArtifactData, rocCurveArtifactData] = + React.useMemo( + () => allArtifacts.map((artifacts) => artifacts.filter(filterSelected)), + [allArtifacts, filterSelected], + ); return ( setIsSectionOpen(isOpen)} + onToggle={(_event, isOpen) => setIsSectionOpen(isOpen)} isExpanded={isSectionOpen} isIndented data-testid="compare-runs-metrics-content" @@ -104,7 +113,12 @@ export const CompareRunMetricsSection: React.FunctionComponent = () => { data-testid="compare-runs-markdown-tab" > - + )}