Skip to content

Commit

Permalink
Addtional tiny performance follow-up (#11460)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
MrFlashAccount authored Nov 1, 2024
1 parent 5f52547 commit 7487a3b
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 49 deletions.
2 changes: 1 addition & 1 deletion app/gui/src/dashboard/components/dashboard/AssetRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const COLUMN_SHOW_TEXT_ID: Readonly<Record<Column, text.TextId>> = {
} 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. */
Expand Down
49 changes: 4 additions & 45 deletions app/gui/src/dashboard/layouts/AssetsTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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[] = [
{
Expand Down Expand Up @@ -827,7 +819,6 @@ export default function AssetsTable(props: AssetsTableProps) {
/** Events sent when the asset list was still loading. */
const queuedAssetListEventsRef = useRef<AssetListEvent[]>([])
const rootRef = useRef<HTMLDivElement | null>(null)
const cleanupRootRef = useRef(() => {})
const mainDropzoneRef = useRef<HTMLButtonElement | null>(null)
const lastSelectedIdsRef = useRef<AssetId | ReadonlySet<AssetId> | null>(null)
const headerRowRef = useRef<HTMLTableRowElement>(null)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -2605,7 +2576,7 @@ export default function AssetsTable(props: AssetsTableProps) {
)

const headerRow = (
<tr ref={headerRowRef} className="sticky top-[1px] text-sm font-semibold">
<tr ref={headerRowRef} className="rounded-none text-sm font-semibold">
{columns.map((column) => {
// This is a React component, even though it does not contain JSX.
const Heading = COLUMN_HEADING[column]
Expand Down Expand Up @@ -2701,8 +2672,8 @@ export default function AssetsTable(props: AssetsTableProps) {
}
}}
>
<table className="table-fixed border-collapse rounded-rows">
<thead>{headerRow}</thead>
<table className="isolate table-fixed border-collapse rounded-rows">
<thead className="sticky top-0 z-1 bg-dashboard">{headerRow}</thead>
<tbody ref={bodyRef}>
{itemRows}
<tr className="hidden h-row first:table-row">
Expand Down Expand Up @@ -2802,21 +2773,8 @@ export default function AssetsTable(props: AssetsTableProps) {
{(innerProps) => (
<div
{...mergeProps<JSX.IntrinsicElements['div']>()(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 &&
Expand All @@ -2838,6 +2796,7 @@ export default function AssetsTable(props: AssetsTableProps) {
onDragEnd: () => {
setIsDraggingFiles(false)
},
ref: rootRef,
})}
>
{!hidden && hiddenContextMenu}
Expand Down
38 changes: 36 additions & 2 deletions app/gui/src/dashboard/layouts/CategorySwitcher/Category.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,48 @@ export const CATEGORY_TO_FILTER_BY: Readonly<Record<Category['type'], FilterBy |
'local-directory': FilterBy.active,
}

/**
* The type of the cached value for a category.
* We use const enums because they compile to numeric values and they are faster than strings.
*/
const enum CategoryCacheType {
cloud = 0,
local = 1,
}

const CATEGORY_CACHE = new Map<Category['type'], CategoryCacheType>()

/** 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. */
Expand Down
6 changes: 6 additions & 0 deletions app/gui/src/dashboard/tailwind.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions app/gui/tailwind.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down

0 comments on commit 7487a3b

Please sign in to comment.