From 2e972a10c06ae04d0e0a5ae0d675d21dc7c02087 Mon Sep 17 00:00:00 2001 From: Maurice Yap Date: Fri, 3 Jan 2025 14:15:08 +0000 Subject: [PATCH] Use virtualized table for job sets table We [previously](https://github.com/armadaproject/armada/commit/a57e7f2622a4efb8e70b24415ec75046ecd5824d) made the job sets table a normal, non-virtualized table. This was to simplify the logic, remove the third-party library, `react-virtualized`, which is no longer well-maintained, and done on the assumption that the page would not have to display enough rows of data to cause it any performance-related trouble. We have observed in some environments that the latter assumption is not correct, and have noticed significant performance degradation with very large datasets. This commit makes the table a virtualised one again, but using the well-maintained Virtuoso library. It also stops truncating the job set ID as users have indicated it is often useful to see the whole thing, even if it is long. --- internal/lookout/ui/package.json | 2 +- .../src/components/job-sets/JobSetTable.tsx | 165 ++++++++++-------- .../lookoutV2/JobStateCountChip.tsx | 6 +- internal/lookout/ui/yarn.lock | 10 +- 4 files changed, 103 insertions(+), 80 deletions(-) diff --git a/internal/lookout/ui/package.json b/internal/lookout/ui/package.json index 7878909b465..1dc8345ff74 100644 --- a/internal/lookout/ui/package.json +++ b/internal/lookout/ui/package.json @@ -29,7 +29,6 @@ "@mui/icons-material": "^6.1.10", "@mui/lab": "^6.0.0-beta.18", "@mui/material": "^6.1.10", - "@re-dev/react-truncate": "^0.4.3", "@tanstack/react-query": "^5.62.3", "@tanstack/react-table": "^8.7.0", "date-fns": "^2.29.3", @@ -45,6 +44,7 @@ "react": "^18", "react-dom": "^18", "react-router-dom": "6.14.2", + "react-virtuoso": "^4.12.3", "semver": "6.3.1", "tough-cookie": "^4.1.3", "use-debounce": "^10.0.0", diff --git a/internal/lookout/ui/src/components/job-sets/JobSetTable.tsx b/internal/lookout/ui/src/components/job-sets/JobSetTable.tsx index 59755ff4e68..0dbe358363b 100644 --- a/internal/lookout/ui/src/components/job-sets/JobSetTable.tsx +++ b/internal/lookout/ui/src/components/job-sets/JobSetTable.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from "react" +import { forwardRef, useCallback, useEffect, useState } from "react" import { Alert, @@ -15,7 +15,7 @@ import { TableSortLabel, } from "@mui/material" import { visuallyHidden } from "@mui/utils" -import { Truncate } from "@re-dev/react-truncate" +import { TableVirtuoso, TableComponents } from "react-virtuoso" import { JobState, jobStateColors, jobStateIcons } from "../../models/lookoutV2Models" import { JobSet } from "../../services/JobService" @@ -31,7 +31,33 @@ const JOB_STATES_TO_DISPLAY = [ [JobState.Cancelled, "jobsCancelled"], ] as const -const MinWidthTableCell = styled(TableCell)({ width: "0%", textWrap: "nowrap" }) +interface TableContext { + selectedJobSetIds: Set +} + +const NoWrapTableCell = styled(TableCell)({ textWrap: "nowrap" }) +const MinWidthTableCell = styled(NoWrapTableCell)({ width: "0%" }) +const MaxWidthTableCell = styled(TableCell)({ width: "100%" }) + +const StyledTable = styled(Table)({ + borderCollapse: "separate", +}) + +const TableHeaderRowCell = styled(TableCell)(({ theme }) => ({ background: theme.palette.background.paper })) + +const TableHeaderRowMinWidthCell = styled(MinWidthTableCell)(({ theme }) => ({ + background: theme.palette.background.paper, +})) + +const VirtuosoTableComponents: TableComponents = { + Scroller: forwardRef((props, ref) => ), + Table: (props) => , + TableHead: forwardRef((props, ref) => ), + TableRow: ({ context, ...props }) => ( + + ), + TableBody: forwardRef((props, ref) => ), +} interface JobSetTableProps { queue: string @@ -89,82 +115,77 @@ export default function JobSetTable({ } return ( - - - - + + data={jobSets} + components={VirtuosoTableComponents} + context={{ selectedJobSetIds: new Set(selectedJobSets.keys()) }} + fixedHeaderContent={() => ( + + + (checked ? onSelectAllClick() : onDeselectAllClick())} + /> + + Job Set + + onOrderChange(!newestFirst)}> + Submission time + + {newestFirst ? "sorted descending" : "sorted ascending"} + + + + {JOB_STATES_TO_DISPLAY.map(([jobState]) => { + const Icon = jobStateIcons[jobState] + return ( + + + {formatJobState(jobState)} + + + + ) + })} + + )} + itemContent={(jobSetIndex, { jobSetId, latestSubmissionTime, ...jobSetRest }) => { + const rowSelected = selectedJobSets.has(jobSetId) + return ( + <> (checked ? onSelectAllClick() : onDeselectAllClick())} + checked={rowSelected} + onChange={(_, checked) => + shiftKeyPressed ? onShiftSelectJobSet(jobSetIndex, checked) : onSelectJobSet(jobSetIndex, checked) + } /> - Job Set - - onOrderChange(!newestFirst)} - > - Submission time - - {newestFirst ? "sorted descending" : "sorted ascending"} - - - - {JOB_STATES_TO_DISPLAY.map(([jobState]) => { - const Icon = jobStateIcons[jobState] - return ( - - - {formatJobState(jobState)} - - - - ) - })} - - - - {jobSets.map(({ jobSetId, latestSubmissionTime, ...jobSetRest }, jobSetIndex) => { - const rowSelected = selectedJobSets.has(jobSetId) - return ( - - - - shiftKeyPressed ? onShiftSelectJobSet(jobSetIndex, checked) : onSelectJobSet(jobSetIndex, checked) - } + {jobSetId} + {latestSubmissionTime} + {JOB_STATES_TO_DISPLAY.map(([jobState, jobSetKey]) => ( + +
+ onJobSetStateClick(jobSetIndex, jobState)} /> - - - {jobSetId} - - {latestSubmissionTime} - {JOB_STATES_TO_DISPLAY.map(([jobState, jobSetKey]) => ( - - onJobSetStateClick(jobSetIndex, jobState)} - /> - - ))} - - ) - })} - -
- + + + ))} + + ) + }} + /> ) } diff --git a/internal/lookout/ui/src/components/lookoutV2/JobStateCountChip.tsx b/internal/lookout/ui/src/components/lookoutV2/JobStateCountChip.tsx index 354f7aaa948..7c2868a0397 100644 --- a/internal/lookout/ui/src/components/lookoutV2/JobStateCountChip.tsx +++ b/internal/lookout/ui/src/components/lookoutV2/JobStateCountChip.tsx @@ -1,7 +1,9 @@ -import { Chip } from "@mui/material" +import { Chip, styled } from "@mui/material" import { JobState, jobStateColors } from "../../models/lookoutV2Models" +const StyledChip = styled(Chip)({ padding: "0 1ch" }) + export interface JobStateCountChipProps { state: JobState count: number @@ -11,7 +13,7 @@ export const JobStateCountChip = ({ state, count, onClick }: JobStateCountChipPr const label = count.toString() return count > 0 ? ( - + ) : ( <>{label} ) diff --git a/internal/lookout/ui/yarn.lock b/internal/lookout/ui/yarn.lock index 0c92373beac..6f7d6602b7f 100644 --- a/internal/lookout/ui/yarn.lock +++ b/internal/lookout/ui/yarn.lock @@ -988,11 +988,6 @@ resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.11.8.tgz#6b79032e760a0899cd4204710beede972a3a185f" integrity sha512-P1st0aksCrn9sGZhp8GMYwBnQsbvAWsZAX44oXNNvLHGqAOcoVxmjZiohstwQ7SqKnbR47akdNi+uleWD8+g6A== -"@re-dev/react-truncate@^0.4.3": - version "0.4.3" - resolved "https://registry.yarnpkg.com/@re-dev/react-truncate/-/react-truncate-0.4.3.tgz#89ed2935a30f271a80f6af8fa66c9fe471baf742" - integrity sha512-dMtHgU/uOorC5gQNtJBjIf9Zv5+lRa11+0YSTEbfD1KCyKKb0bcCjVCEVyyKUSv0/vEmiG+twQIt2eVKETaRNA== - "@remix-run/router@1.7.2": version "1.7.2" resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.7.2.tgz#cba1cf0a04bc04cb66027c51fa600e9cbc388bc8" @@ -4247,6 +4242,11 @@ react-transition-group@^4.4.5: loose-envify "^1.4.0" prop-types "^15.6.2" +react-virtuoso@^4.12.3: + version "4.12.3" + resolved "https://registry.yarnpkg.com/react-virtuoso/-/react-virtuoso-4.12.3.tgz#beecf0582b31058c5a6ed3ec58fc43fd780e5844" + integrity sha512-6X1p/sU7hecmjDZMAwN+r3go9EVjofKhwkUbVlL8lXhBZecPv9XVCkZ/kBPYOr0Mv0Vl5+Ziwgexg9Kh7+NNXQ== + react@^18: version "18.3.1" resolved "https://registry.yarnpkg.com/react/-/react-18.3.1.tgz#49ab892009c53933625bd16b2533fc754cab2891"