From a06b6ace52988bff45012d78a29c9adbbeb32249 Mon Sep 17 00:00:00 2001 From: Tim Sweeney Date: Fri, 6 Dec 2024 08:57:21 -0800 Subject: [PATCH 1/4] chore(weave): Improve large object viewer renders (#3102) * init * init * Larger refactor * Larger refactor * Larger refactor * Larger refactor * Larger refactor --- .../Browse3/pages/CallPage/CallDetails.tsx | 41 ++----- .../Browse3/pages/CallPage/ObjectViewer.tsx | 81 ++++++++----- .../pages/CallPage/ObjectViewerSection.tsx | 4 +- .../traceServerCachingClient.ts | 106 ++++++++++++++++++ .../wfReactInterface/traceServerClient.ts | 4 +- .../typeViews/CustomWeaveTypeDispatcher.tsx | 8 ++ .../PIL.Image.Image/PILImageImage.tsx | 5 +- 7 files changed, 183 insertions(+), 66 deletions(-) create mode 100644 weave-js/src/components/PagePanelComponents/Home/Browse3/pages/wfReactInterface/traceServerCachingClient.ts diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/CallDetails.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/CallDetails.tsx index 8a9c011b5de..26be9f0c5d0 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/CallDetails.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/CallDetails.tsx @@ -1,4 +1,3 @@ -import {Typography} from '@mui/material'; import Box from '@mui/material/Box'; import _ from 'lodash'; import React, {FC, useContext, useMemo} from 'react'; @@ -10,9 +9,7 @@ import {Button} from '../../../../../Button'; import {useWeaveflowRouteContext, WeaveflowPeekContext} from '../../context'; import {CustomWeaveTypeProjectContext} from '../../typeViews/CustomWeaveTypeDispatcher'; import {CallsTable} from '../CallsPage/CallsTable'; -import {KeyValueTable} from '../common/KeyValueTable'; -import {CallLink, opNiceName} from '../common/Links'; -import {CenteredAnimatedLoader} from '../common/Loader'; +import {CallLink} from '../common/Links'; import {useWFHooks} from '../wfReactInterface/context'; import {CallSchema} from '../wfReactInterface/wfDataModelHooksInterface'; import {ButtonOverlay} from './ButtonOverlay'; @@ -20,6 +17,8 @@ import {ExceptionDetails, getExceptionInfo} from './Exceptions'; import {ObjectViewerSection} from './ObjectViewerSection'; import {OpVersionText} from './OpVersionText'; +const HEADER_HEIGHT_BUFFER = 60; + const Heading = styled.div` color: ${MOON_800}; font-weight: 600; @@ -104,7 +103,7 @@ export const CallDetails: FC<{ columns ); - const {singularChildCalls, multipleChildCallOpRefs} = useMemo( + const {multipleChildCallOpRefs} = useMemo( () => callGrouping(!childCalls.loading ? childCalls.result ?? [] : []), [childCalls.loading, childCalls.result] ); @@ -133,6 +132,7 @@ export const CallDetails: FC<{ 0 ? HEADER_HEIGHT_BUFFER : 0 + }px)`, p: 2, }}> {'traceback' in excInfo ? ( @@ -201,7 +204,6 @@ export const CallDetails: FC<{ sx={{ flex: '0 0 auto', height: '500px', - maxHeight: '95%', p: 2, display: 'flex', flexDirection: 'column', @@ -240,33 +242,6 @@ export const CallDetails: FC<{ ); })} - {childCalls.loading && } - {/* Disabling display of singular children while we decide if we want them here. */} - {false && singularChildCalls.length > 0 && ( - - {multipleChildCallOpRefs.length === 0 ? ( - Child calls - ) : ( - Singular child calls - )} - {singularChildCalls.map(c => ( - - - - ))} - - )} ); diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx index d741db64771..e8f39ad2880 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewer.tsx @@ -23,7 +23,11 @@ import {LoadingDots} from '../../../../../LoadingDots'; import {Browse2OpDefCode} from '../../../Browse2/Browse2OpDefCode'; import {isWeaveRef} from '../../filters/common'; import {StyledDataGrid} from '../../StyledDataGrid'; -import {isCustomWeaveTypePayload} from '../../typeViews/customWeaveType.types'; +import { + CustomWeaveTypePayload, + isCustomWeaveTypePayload, +} from '../../typeViews/customWeaveType.types'; +import {getCustomWeaveTypePreferredRowHeight} from '../../typeViews/CustomWeaveTypeDispatcher'; import { LIST_INDEX_EDGE_NAME, OBJECT_ATTR_EDGE_NAME, @@ -48,6 +52,10 @@ import { } from './traverse'; import {ValueView} from './ValueView'; +const DEFAULT_ROW_HEIGHT = 38; +const CODE_ROW_HEIGHT = 350; +const TABLE_ROW_HEIGHT = 450; + type Data = Record | any[]; type ObjectViewerProps = { @@ -440,6 +448,47 @@ export const ObjectViewer = ({ }); }, [apiRef, expandedIds, updateRowExpand]); + // Per https://mui.com/x/react-data-grid/row-height/#dynamic-row-height, always + // memoize the getRowHeight function. + const getRowHeight = useCallback((params: GridRowHeightParams) => { + const isNonRefString = + params.model.valueType === 'string' && !isWeaveRef(params.model.value); + const isArray = params.model.valueType === 'array'; + const isTableRef = + isWeaveRef(params.model.value) && + (parseRefMaybe(params.model.value) as any).weaveKind === 'table'; + const {isCode} = params.model; + const isCustomWeaveType = isCustomWeaveTypePayload(params.model.value); + if (!params.model.isLeaf) { + // This is a group header, so we want to use the default height + return DEFAULT_ROW_HEIGHT; + } else if (isNonRefString) { + // This is the only special case where we will allow for dynamic height. + // Since strings have special renders that take up different amounts of + // space, we will allow for dynamic height. + return 'auto'; + } else if (isCustomWeaveType) { + const type = (params.model.value as CustomWeaveTypePayload).weave_type + .type; + const preferredRowHeight = getCustomWeaveTypePreferredRowHeight(type); + if (preferredRowHeight) { + return preferredRowHeight; + } + return DEFAULT_ROW_HEIGHT; + } else if ((isArray && USE_TABLE_FOR_ARRAYS) || isTableRef) { + // Perfectly enough space for 1 page of data rows + return TABLE_ROW_HEIGHT; + } else if (isCode) { + // Probably will get negative feedback here since code that is < 20 lines + // will have some whitespace below the code. However, we absolutely need + // to have static height for all cells else the MUI data grid will jump around + // when cleaning up virtual rows. + return CODE_ROW_HEIGHT; + } else { + return DEFAULT_ROW_HEIGHT; + } + }, []); + // Finally, we memoize the inner data grid component. This is important to // reduce the number of re-renders when the data changes. const inner = useMemo(() => { @@ -473,31 +522,9 @@ export const ObjectViewer = ({ isGroupExpandedByDefault={node => { return expandedIds.includes(node.id); }} - autoHeight columnHeaderHeight={38} - getRowHeight={(params: GridRowHeightParams) => { - const isNonRefString = - params.model.valueType === 'string' && - !isWeaveRef(params.model.value); - const isArray = params.model.valueType === 'array'; - const isTableRef = - isWeaveRef(params.model.value) && - (parseRefMaybe(params.model.value) as any).weaveKind === 'table'; - const {isCode} = params.model; - const isCustomWeaveType = isCustomWeaveTypePayload( - params.model.value - ); - if ( - isNonRefString || - (isArray && USE_TABLE_FOR_ARRAYS) || - isTableRef || - isCode || - isCustomWeaveType - ) { - return 'auto'; - } - return 38; - }} + rowHeight={DEFAULT_ROW_HEIGHT} + getRowHeight={getRowHeight} hideFooter rowSelection={false} groupingColDef={groupingColDef} @@ -517,10 +544,10 @@ export const ObjectViewer = ({ }} /> ); - }, [apiRef, rows, columns, groupingColDef, expandedIds]); + }, [apiRef, rows, columns, getRowHeight, groupingColDef, expandedIds]); // Return the inner data grid wrapped in a div with overflow hidden. - return
{inner}
; + return
{inner}
; }; // Helper function to build the base ref for a given path. This function is used diff --git a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerSection.tsx b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerSection.tsx index d95d9a841de..d02a2e881ca 100644 --- a/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerSection.tsx +++ b/weave-js/src/components/PagePanelComponents/Home/Browse3/pages/CallPage/ObjectViewerSection.tsx @@ -185,7 +185,7 @@ const ObjectViewerSectionNonEmpty = ({ }, [data, isExpanded]); return ( - <> + {title}