From 7487a3b4d2cca2013ba755ce2bdfa937a0341180 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Fri, 1 Nov 2024 16:03:21 +0300 Subject: [PATCH] Addtional tiny performance follow-up (#11460) This PR mostly improve performance of the assets table: 1. It removes calc of the `clipPath` on scroll which triggers position recalculation in `Navigator2D` 2. Adds caching for parsing a category (we do this a lot across components but we have only a few categories) and runtime validatation has relatively large perf penalty 3. Adds dom-based virtualization for rows (we still need to add proper react based virtualization though) --- .../components/dashboard/AssetRow.tsx | 2 +- .../dashboard/column/columnUtils.ts | 2 +- app/gui/src/dashboard/layouts/AssetsTable.tsx | 49 ++----------------- .../layouts/CategorySwitcher/Category.ts | 38 +++++++++++++- app/gui/src/dashboard/tailwind.css | 6 +++ app/gui/tailwind.config.js | 3 ++ 6 files changed, 51 insertions(+), 49 deletions(-) diff --git a/app/gui/src/dashboard/components/dashboard/AssetRow.tsx b/app/gui/src/dashboard/components/dashboard/AssetRow.tsx index 48edd9dbb33f..00551333a48c 100644 --- a/app/gui/src/dashboard/components/dashboard/AssetRow.tsx +++ b/app/gui/src/dashboard/components/dashboard/AssetRow.tsx @@ -539,7 +539,7 @@ export const AssetRow = React.memo(function AssetRow(props: AssetRowProps) { } }} className={tailwindMerge.twMerge( - 'h-table-row rounded-full transition-all ease-in-out rounded-rows-child', + 'h-table-row rounded-full transition-all ease-in-out rounded-rows-child [contain-intrinsic-size:40px] [content-visibility:auto]', visibility, (isDraggedOver || selected) && 'selected', )} diff --git a/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts b/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts index bf0c159839af..912b0678efa4 100644 --- a/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts +++ b/app/gui/src/dashboard/components/dashboard/column/columnUtils.ts @@ -64,7 +64,7 @@ export const COLUMN_SHOW_TEXT_ID: Readonly> = { } satisfies { [C in Column]: `${C}ColumnShow` } const COLUMN_CSS_CLASSES = - 'text-left bg-clip-padding border-transparent border-y border-2 last:border-r-0 last:rounded-r-full last:w-full' + 'text-left bg-clip-padding last:border-r-0 last:rounded-r-full last:w-full' const NORMAL_COLUMN_CSS_CLASSES = `px-cell-x py ${COLUMN_CSS_CLASSES}` /** CSS classes for every column. */ diff --git a/app/gui/src/dashboard/layouts/AssetsTable.tsx b/app/gui/src/dashboard/layouts/AssetsTable.tsx index 198d513ef280..b47fd1c1f209 100644 --- a/app/gui/src/dashboard/layouts/AssetsTable.tsx +++ b/app/gui/src/dashboard/layouts/AssetsTable.tsx @@ -65,7 +65,6 @@ import { useEventCallback } from '#/hooks/eventCallbackHooks' import { useIntersectionRatio } from '#/hooks/intersectionHooks' import { useOpenProject } from '#/hooks/projectHooks' import { useToastAndLog } from '#/hooks/toastAndLogHooks' -import useOnScroll from '#/hooks/useOnScroll' import type * as assetSearchBar from '#/layouts/AssetSearchBar' import * as eventListProvider from '#/layouts/AssetsTable/EventListProvider' import AssetsTableContextMenu from '#/layouts/AssetsTableContextMenu' @@ -197,13 +196,6 @@ const MINIMUM_DROPZONE_INTERSECTION_RATIO = 0.5 const ROW_HEIGHT_PX = 38 /** The size of the loading spinner. */ const LOADING_SPINNER_SIZE_PX = 36 -/** - * The number of pixels the header bar should shrink when the column selector is visible, - * assuming 0 icons are visible in the column selector. - */ -const COLUMNS_SELECTOR_BASE_WIDTH_PX = 4 -/** The number of pixels the header bar should shrink per collapsed column. */ -const COLUMNS_SELECTOR_ICON_WIDTH_PX = 28 const SUGGESTIONS_FOR_NO: assetSearchBar.Suggestion[] = [ { @@ -827,7 +819,6 @@ export default function AssetsTable(props: AssetsTableProps) { /** Events sent when the asset list was still loading. */ const queuedAssetListEventsRef = useRef([]) const rootRef = useRef(null) - const cleanupRootRef = useRef(() => {}) const mainDropzoneRef = useRef(null) const lastSelectedIdsRef = useRef | null>(null) const headerRowRef = useRef(null) @@ -2233,26 +2224,6 @@ export default function AssetsTable(props: AssetsTableProps) { } }, [hidden]) - // This is required to prevent the table body from overlapping the table header, because - // the table header is transparent. - const updateClipPath = useOnScroll(() => { - if (bodyRef.current != null && rootRef.current != null) { - bodyRef.current.style.clipPath = `inset(${rootRef.current.scrollTop}px 0 0 0)` - } - if ( - backend.type === BackendType.remote && - rootRef.current != null && - headerRowRef.current != null - ) { - const shrinkBy = - COLUMNS_SELECTOR_BASE_WIDTH_PX + COLUMNS_SELECTOR_ICON_WIDTH_PX * hiddenColumns.length - const rightOffset = rootRef.current.clientWidth + rootRef.current.scrollLeft - shrinkBy - headerRowRef.current.style.clipPath = `polygon(0 0, ${rightOffset}px 0, ${rightOffset}px 100%, 0 100%)` - } - }, [backend.type, hiddenColumns.length]) - - const updateClipPathObserver = useMemo(() => new ResizeObserver(updateClipPath), [updateClipPath]) - useEffect( () => inputBindings.attach( @@ -2605,7 +2576,7 @@ export default function AssetsTable(props: AssetsTableProps) { ) const headerRow = ( - + {columns.map((column) => { // This is a React component, even though it does not contain JSX. const Heading = COLUMN_HEADING[column] @@ -2701,8 +2672,8 @@ export default function AssetsTable(props: AssetsTableProps) { } }} > - - {headerRow} +
+ {headerRow} {itemRows} @@ -2802,21 +2773,8 @@ export default function AssetsTable(props: AssetsTableProps) { {(innerProps) => (
()(innerProps, { - ref: (value) => { - rootRef.current = value - cleanupRootRef.current() - if (value) { - updateClipPathObserver.observe(value) - cleanupRootRef.current = () => { - updateClipPathObserver.unobserve(value) - } - } else { - cleanupRootRef.current = () => {} - } - }, className: 'flex-1 overflow-auto container-size w-full h-full', onKeyDown, - onScroll: updateClipPath, onBlur: (event) => { if ( event.relatedTarget instanceof HTMLElement && @@ -2838,6 +2796,7 @@ export default function AssetsTable(props: AssetsTableProps) { onDragEnd: () => { setIsDraggingFiles(false) }, + ref: rootRef, })} > {!hidden && hiddenContextMenu} diff --git a/app/gui/src/dashboard/layouts/CategorySwitcher/Category.ts b/app/gui/src/dashboard/layouts/CategorySwitcher/Category.ts index a0c929032c12..11196c8672d1 100644 --- a/app/gui/src/dashboard/layouts/CategorySwitcher/Category.ts +++ b/app/gui/src/dashboard/layouts/CategorySwitcher/Category.ts @@ -109,14 +109,48 @@ export const CATEGORY_TO_FILTER_BY: Readonly() + /** Whether the category is only accessible from the cloud. */ export function isCloudCategory(category: Category): category is AnyCloudCategory { - return ANY_CLOUD_CATEGORY_SCHEMA.safeParse(category).success + const cached = CATEGORY_CACHE.get(category.type) + + if (cached != null) { + return cached === CategoryCacheType.cloud + } + + const result = ANY_CLOUD_CATEGORY_SCHEMA.safeParse(category) + CATEGORY_CACHE.set( + category.type, + result.success ? CategoryCacheType.cloud : CategoryCacheType.local, + ) + + return result.success } /** Whether the category is only accessible locally. */ export function isLocalCategory(category: Category): category is AnyLocalCategory { - return ANY_LOCAL_CATEGORY_SCHEMA.safeParse(category).success + const cached = CATEGORY_CACHE.get(category.type) + + if (cached != null) { + return cached === CategoryCacheType.local + } + + const result = ANY_LOCAL_CATEGORY_SCHEMA.safeParse(category) + CATEGORY_CACHE.set( + category.type, + result.success ? CategoryCacheType.local : CategoryCacheType.cloud, + ) + return result.success } /** Whether the given categories are equal. */ diff --git a/app/gui/src/dashboard/tailwind.css b/app/gui/src/dashboard/tailwind.css index 5d1a1cfe993b..2fbfa57880cd 100644 --- a/app/gui/src/dashboard/tailwind.css +++ b/app/gui/src/dashboard/tailwind.css @@ -20,6 +20,12 @@ --color-invert-opacity: 100%; --color-invert: rgb(var(--color-invert-rgb) / var(--color-invert-opacity)); + --color-dashboard-background-rgb: 239 234 228; + --color-dashboard-background-opacity: 100%; + --color-dashboard-background: rgb( + var(--color-dashboard-background-rgb) / var(--color-dashboard-background-opacity) + ); + --top-bar-height: 3rem; --row-height: 2rem; --table-row-height: 2.3125rem; diff --git a/app/gui/tailwind.config.js b/app/gui/tailwind.config.js index c8e761d8a1d9..8872e4a8be3e 100644 --- a/app/gui/tailwind.config.js +++ b/app/gui/tailwind.config.js @@ -20,6 +20,9 @@ export default /** @satisfies {import('tailwindcss').Config} */ ({ // This should be named "regular". primary: 'rgb(var(--color-primary-rgb) / var(--color-primary-opacity))', invert: 'rgb(var(--color-invert-rgb) / var(--color-invert-opacity))', + background: 'rgb(var(--color-background-rgb) / var(--color-background-opacity))', + dashboard: + 'rgb(var(--color-dashboard-background-rgb) / var(--color-dashboard-background-opacity))', accent: 'rgb(var(--color-accent-rgb) / 100%)', danger: 'rgb(var(--color-danger-rgb) / 100%)', 'accent-dark': '#3e9152',