Skip to content

Commit

Permalink
Use virtualized table for job sets table (#4116)
Browse files Browse the repository at this point in the history 
We [previously](a57e7f2) 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.
  • Loading branch information
@mauriceyap
mauriceyap authored Jan 3, 2025
1 parent 26c69c6 commit 1ddd71b
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 80 deletions.
2 changes: 1 addition & 1 deletion internal/lookout/ui/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
165 changes: 93 additions & 72 deletions internal/lookout/ui/src/components/job-sets/JobSetTable.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useState } from "react"
import { forwardRef, useCallback, useEffect, useState } from "react"

import {
Alert,
Expand All @@ -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"
Expand All @@ -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<string>
}

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<JobSet, TableContext> = {
Scroller: forwardRef<HTMLDivElement>((props, ref) => <TableContainer {...props} ref={ref} />),
Table: (props) => <StyledTable {...props} size="small" />,
TableHead: forwardRef<HTMLTableSectionElement>((props, ref) => <TableHead {...props} ref={ref} />),
TableRow: ({ context, ...props }) => (
<TableRow {...props} hover selected={context?.selectedJobSetIds.has(props.item.jobSetId)} />
),
TableBody: forwardRef<HTMLTableSectionElement>((props, ref) => <TableBody {...props} ref={ref} />),
}

interface JobSetTableProps {
queue: string
Expand Down Expand Up @@ -89,82 +115,77 @@ export default function JobSetTable({
}

return (
<TableContainer>
<Table size="small">
<TableHead>
<TableRow>
<TableVirtuoso<JobSet, TableContext>
data={jobSets}
components={VirtuosoTableComponents}
context={{ selectedJobSetIds: new Set(selectedJobSets.keys()) }}
fixedHeaderContent={() => (
<TableRow>
<TableHeaderRowCell padding="checkbox">
<Checkbox
color="primary"
inputProps={{
"aria-label": "select all job sets",
}}
indeterminate={0 < selectedJobSets.size && selectedJobSets.size < jobSets.length}
checked={selectedJobSets.size === jobSets.length}
onChange={(_, checked) => (checked ? onSelectAllClick() : onDeselectAllClick())}
/>
</TableHeaderRowCell>
<TableHeaderRowCell>Job Set</TableHeaderRowCell>
<TableHeaderRowCell>
<TableSortLabel active direction={newestFirst ? "desc" : "asc"} onClick={() => onOrderChange(!newestFirst)}>
Submission time
<Box component="span" sx={visuallyHidden}>
{newestFirst ? "sorted descending" : "sorted ascending"}
</Box>
</TableSortLabel>
</TableHeaderRowCell>
{JOB_STATES_TO_DISPLAY.map(([jobState]) => {
const Icon = jobStateIcons[jobState]
return (
<TableHeaderRowMinWidthCell key={jobState} align="right">
<Stack direction="row" spacing={1} alignItems="center" display="inline">
<span>{formatJobState(jobState)}</span>
<Icon fontSize="inherit" color={jobStateColors[jobState]} />
</Stack>
</TableHeaderRowMinWidthCell>
)
})}
</TableRow>
)}
itemContent={(jobSetIndex, { jobSetId, latestSubmissionTime, ...jobSetRest }) => {
const rowSelected = selectedJobSets.has(jobSetId)
return (
<>
<TableCell padding="checkbox">
<Checkbox
color="primary"
inputProps={{
"aria-label": "select all job sets",
"aria-label": `select job set ${jobSetId}`,
}}
indeterminate={0 < selectedJobSets.size && selectedJobSets.size < jobSets.length}
checked={selectedJobSets.size === jobSets.length}
onChange={(_, checked) => (checked ? onSelectAllClick() : onDeselectAllClick())}
checked={rowSelected}
onChange={(_, checked) =>
shiftKeyPressed ? onShiftSelectJobSet(jobSetIndex, checked) : onSelectJobSet(jobSetIndex, checked)
}
/>
</TableCell>
<TableCell>Job Set</TableCell>
<TableCell>
<TableSortLabel
active
direction={newestFirst ? "desc" : "asc"}
onClick={() => onOrderChange(!newestFirst)}
>
Submission time
<Box component="span" sx={visuallyHidden}>
{newestFirst ? "sorted descending" : "sorted ascending"}
</Box>
</TableSortLabel>
</TableCell>
{JOB_STATES_TO_DISPLAY.map(([jobState]) => {
const Icon = jobStateIcons[jobState]
return (
<MinWidthTableCell key={jobState} align="right">
<Stack direction="row" spacing={1} alignItems="center" display="inline">
<span>{formatJobState(jobState)}</span>
<Icon fontSize="inherit" color={jobStateColors[jobState]} />
</Stack>
</MinWidthTableCell>
)
})}
</TableRow>
</TableHead>
<TableBody>
{jobSets.map(({ jobSetId, latestSubmissionTime, ...jobSetRest }, jobSetIndex) => {
const rowSelected = selectedJobSets.has(jobSetId)
return (
<TableRow key={jobSetId} hover selected={rowSelected}>
<TableCell padding="checkbox">
<Checkbox
color="primary"
inputProps={{
"aria-label": `select job set ${jobSetId}`,
}}
checked={rowSelected}
onChange={(_, checked) =>
shiftKeyPressed ? onShiftSelectJobSet(jobSetIndex, checked) : onSelectJobSet(jobSetIndex, checked)
}
<MaxWidthTableCell>{jobSetId}</MaxWidthTableCell>
<NoWrapTableCell>{latestSubmissionTime}</NoWrapTableCell>
{JOB_STATES_TO_DISPLAY.map(([jobState, jobSetKey]) => (
<MinWidthTableCell key={jobState} align="right">
<div>
<JobStateCountChip
state={jobState}
count={jobSetRest[jobSetKey]}
onClick={() => onJobSetStateClick(jobSetIndex, jobState)}
/>
</TableCell>
<TableCell>
<Truncate lines={1}>{jobSetId}</Truncate>
</TableCell>
<TableCell>{latestSubmissionTime}</TableCell>
{JOB_STATES_TO_DISPLAY.map(([jobState, jobSetKey]) => (
<MinWidthTableCell key={jobState} align="right">
<JobStateCountChip
state={jobState}
count={jobSetRest[jobSetKey]}
onClick={() => onJobSetStateClick(jobSetIndex, jobState)}
/>
</MinWidthTableCell>
))}
</TableRow>
)
})}
</TableBody>
</Table>
</TableContainer>
</div>
</MinWidthTableCell>
))}
</>
)
}}
/>
)
}
6 changes: 4 additions & 2 deletions internal/lookout/ui/src/components/lookoutV2/JobStateCountChip.tsx
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -11,7 +13,7 @@ export const JobStateCountChip = ({ state, count, onClick }: JobStateCountChipPr
const label = count.toString()

return count > 0 ? (
<Chip label={label} color={jobStateColors[state]} clickable component="button" onClick={onClick} variant="filled" />
<StyledChip label={label} color={jobStateColors[state]} clickable onClick={onClick} variant="filled" size="small" />
) : (
<>{label}</>
)
Expand Down
10 changes: 5 additions & 5 deletions internal/lookout/ui/yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected]":
version "1.7.2"
resolved "https://registry.yarnpkg.com/@remix-run/router/-/router-1.7.2.tgz#cba1cf0a04bc04cb66027c51fa600e9cbc388bc8"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 1ddd71b

Please sign in to comment.