Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Improvements #408

Merged
merged 35 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2521034
Add selector for attribute values in a row
Sep 24, 2024
f08aed3
Use row attribute value selector to improve AttributeBar performance
Sep 24, 2024
9fc352b
Updates to AttributeBar comments & style
Sep 24, 2024
8cedadb
Exclude element sidebar from DOM unless open
Sep 24, 2024
2963aa8
Use callbacks for CollapseAllButton's functions
Sep 27, 2024
c005960
Memoize getting intersections in the elementsSelector
Oct 1, 2024
0cf5771
Use recoil to memoize row computations
Oct 1, 2024
5c12dc7
Clean up unused imports for previous commit
Oct 1, 2024
5fd3107
Bugfix for previous: get alttext gen working again
Oct 1, 2024
3273e06
Use selectors to memoize AttributeDropdown computations
Oct 1, 2024
8059928
Add why-did-you-render package to dev mode
Oct 3, 2024
bc81b09
Prevent AttributeBars from re-rendering on hover
Oct 12, 2024
a7937f7
Memoize props sent to VegaLite in DensityPlot to avoid unnecessary re…
Oct 12, 2024
0788948
Check attribute values in AttributeBar memo comparison func
Oct 12, 2024
5c01982
Remove unnecessary useRef for string prop
Oct 12, 2024
232cb7e
Memoize element vis signalListeners to reduce unnecessary re-renders
Oct 15, 2024
b69ffb5
Memoize VegaLite props in ElementVis
Oct 15, 2024
a3f6461
Bugfix: re-export RenderRow type as it is used elsewhere
Oct 15, 2024
e586615
Bugfix: add an additional null check to attValuesSelector; remove log…
Oct 15, 2024
6841b04
Split density plots & memoize internally to minimize expensive re-ren…
Oct 15, 2024
a98ef43
Bugfix: handle potentially null data (bug only appears in prod build …
Oct 15, 2024
c848a62
Bugfix for prev: null check & typecheck data before checking for 'set…
Oct 15, 2024
6be89e1
Bugfix for previous: tired brain no do logical operators
Oct 15, 2024
643e830
Move provenance state converter from root to app
Oct 17, 2024
57e7e35
Add upset config atom in App & sync it with the provenance state
Oct 17, 2024
87194c5
Revamp how multinet states are imported; move import & ProvenanceCont…
Oct 18, 2024
02af7c2
Bugfix: ensure ProvenanceVis never tries to find a nonexistent curren…
Oct 18, 2024
ab6866a
Change App's rowsSelector to be truly responsive & not take UpsetConf…
Oct 18, 2024
fad6046
Use structuredClone instead of json stringification in ElementVis
Oct 18, 2024
2d9fda8
Docs update for index.tsx
Oct 18, 2024
9b38c8a
Bugfix: wrong URL while setting up tests
Oct 18, 2024
2d8db46
Organize App useEffects
Oct 21, 2024
4b767ba
Update DataTable to import rows directly from selector & fix datatabl…
Oct 21, 2024
90b2d00
Add spinner while loading provenance (also fixes console warnings)
Oct 21, 2024
50fc39e
Merge branch 'main' into 334-performance
NateLanza Oct 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
"doc": "typedoc --options typedoc.json"
},
"devDependencies": {
"@types/node": "^22.7.4",
"@typescript-eslint/eslint-plugin": "^6.3.0",
"@typescript-eslint/parser": "^6.3.0",
"@welldone-software/why-did-you-render": "^8.0.3",
"eslint": "^8.6.0",
"eslint-config-airbnb": "^19.0.4",
"eslint-plugin-import": "^2.25.4",
Expand All @@ -43,9 +45,9 @@
"@playwright/test": "^1.15.0",
"@vitejs/plugin-react": "^4.0.4",
"eslint-config-react-app": "^7.0.1",
"react": "^18.0.2",
"vite": "^4.4.9",
"vite-plugin-dts": "^3.5.1",
"vite-tsconfig-paths": "^4.2.0",
"react": "^18.0.2"
"vite-tsconfig-paths": "^4.2.0"
}
}
50 changes: 50 additions & 0 deletions packages/app/src/atoms/selectors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { getRows, Item, Rows, UpsetConfig } from "@visdesignlab/upset2-core";
import { selector, selectorFamily } from "recoil";
import { dataSelector } from "./dataAtom";

/**
* Gets all rows in the plot
*/
export const rowsSelector = selectorFamily<Rows, UpsetConfig>({
key: 'plot-rows',
get: (config: UpsetConfig) => ({ get }) => getRows(get(dataSelector), config),
})

/**
* Gets all items from the CoreUpsetData
*/
export const itemsSelector = selector<Item[]>({
key: 'data-items',
get: ({ get }) => Object.values(get(dataSelector)?.items ?? {}),
})

/**
* Counts the number of items that have a given attribute.
* null, undefined, NaN, and '' values are not counted.
* @param {string} att Attribute name to count items for
* @returns {number} Count of items that have a truthy value for this attribute
*/
export const attributeValuesCount = selectorFamily<number, string>({
key: 'attribute-count',
get: (att: string) => ({ get }) => Object.values(
get(itemsSelector),
).filter(
(item) => !!item[att],
).length,
});

/**
* Selector to count values for multiple attributes
* @param {string[]} atts Array of attribute names to count values for
* @returns {Record<string, number>} An object where keys are attribute names and values are their respective counts
*/
export const countValuesForAttributes = selectorFamily<Record<string, number>, string[]>({
key: 'multi-att-count',
get: (atts: string[]) => ({ get }) => {
const counts: Record<string, number> = {};
atts.forEach((att) => {
counts[att] = get(attributeValuesCount(att));
});
return counts;
},
});
59 changes: 13 additions & 46 deletions packages/app/src/components/AttributeDropdown.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {
Button,
Box,
import {
Menu,
Checkbox,
FormControlLabel,
Expand All @@ -19,33 +17,8 @@ import { ProvenanceContext } from "./Root"
import { dataSelector } from "../atoms/dataAtom";
import { useRecoilValue } from "recoil";
import { useState } from "react";
import { CoreUpsetData, DefaultConfig } from "@visdesignlab/upset2-core";

/**
* Get the count of items that have a specific attribute.
* Does not count items with null or undefined values for this attribute.
* @param attribute - The attribute to count.
* @param data - The data object containing the items.
* @returns The count of items with the specified attribute value.
*/
const getAttributeItemCount = (attribute: string, data: CoreUpsetData) => {
if (DefaultConfig.visibleAttributes.includes(attribute)) {
return '';
}
let count = 0;

Object.values(data.items).forEach((item) => {
Object.entries(item).forEach(([key, val]) => {
if (key === attribute) {
if (val) {
count++;
}
}
})
})

return count;
}
import { DefaultConfig } from "@visdesignlab/upset2-core";
import { countValuesForAttributes } from "../atoms/selectors";

/**
* Dropdown component for selecting attributes.
Expand All @@ -66,18 +39,12 @@ export const AttributeDropdown = (props: {anchorEl: HTMLElement, close: () => vo
);

const [ searchTerm, setSearchTerm ] = useState<string>("");

const attributeItemCount: { [attr: string]: number | string } = {};

const attributes = data ? [...DefaultConfig.visibleAttributes, ...data.attributeColumns]: [...DefaultConfig.visibleAttributes];
const attributes = useMemo(
() => data ? [...DefaultConfig.visibleAttributes, ...data.attributeColumns]: [...DefaultConfig.visibleAttributes],
[data]
);
const attributeCounts = useRecoilValue(countValuesForAttributes(attributes));


if (data) {
attributes.forEach((attr) => {
attributeItemCount[attr] = getAttributeItemCount(attr,data);
})
}

/**
* Handle checkbox toggle: add or remove the attribute from the visible attributes
* and update the provenance state and plot.
Expand Down Expand Up @@ -116,18 +83,18 @@ export const AttributeDropdown = (props: {anchorEl: HTMLElement, close: () => vo
* Get the rows to display in the table.
* @returns The filtered rows based on the search term.
*/
const getRows = () => {
const rows = useMemo(() => {
if (data === undefined || data === null) {
return []
}
return attributes.map((attr, index) => {
return {
id: index,
attribute: attr,
itemCount: getAttributeItemCount(attr,data)
itemCount: attributeCounts[attr]
}
}).filter((row) => row.attribute.toLowerCase().includes(searchTerm.toLowerCase()))
}
}, [data, attributes, searchTerm, attributeCounts]);

return (
<Menu
Expand Down Expand Up @@ -156,13 +123,13 @@ export const AttributeDropdown = (props: {anchorEl: HTMLElement, close: () => vo
</TableRow>
</TableHead>
<TableBody>
{ getRows().map((row) => {
{rows.map((row) => {
return (
<TableRow key={row.id}>
<TableCell>
<FormControlLabel checked={checked.includes(row.attribute)} control={<Checkbox />} label={row.attribute} onChange={(e) => handleToggle(e.target)}/>
</TableCell>
<TableCell><Typography>{row.itemCount}</Typography></TableCell>
<TableCell><Typography>{row.itemCount > 0 ? row.itemCount : ''}</Typography></TableCell>
</TableRow>
)
})
Expand Down
7 changes: 5 additions & 2 deletions packages/app/src/components/Body.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { AltText, Upset, getAltTextConfig } from '@visdesignlab/upset2-react';
import { UpsetConfig, getRows } from '@visdesignlab/upset2-core';
import { UpsetConfig } from '@visdesignlab/upset2-core';
import { useRecoilValue, useRecoilState } from 'recoil';
import { encodedDataAtom } from '../atoms/dataAtom';
import { doesHaveSavedQueryParam, queryParamAtom, saveQueryParam } from '../atoms/queryParamAtom';
Expand All @@ -13,6 +13,7 @@ import { loadingAtom } from '../atoms/loadingAtom';
import { Backdrop, CircularProgress } from '@mui/material';
import { updateMultinetSession } from '../api/session';
import { generateAltText } from '../api/generateAltText';
import { rowsSelector } from '../atoms/selectors';

type Props = {
data: any;
Expand All @@ -27,6 +28,7 @@ export const Body = ({ data, config }: Props) => {
const [ isElementSidebarOpen, setIsElementSidebarOpen ] = useRecoilState(elementSidebarAtom);
const [ isAltTextSidebarOpen, setIsAltTextSidebarOpen ] = useRecoilState(altTextSidebarAtom);
const loading = useRecoilValue(loadingAtom);
const rows = useRecoilValue(rowsSelector(provObject.provenance.getState()));

const provVis = {
open: isProvVisOpen,
Expand Down Expand Up @@ -59,7 +61,8 @@ export const Body = ({ data, config }: Props) => {
*/
async function getAltText(): Promise<AltText> {
const state = provObject.provenance.getState();
const config = getAltTextConfig(state, data, getRows(data, state));
// Rows must be cloned to avoid a recoil error triggered far down in this call chain when a function writes rows
const config = getAltTextConfig(state, data, structuredClone(rows));

if (config.firstAggregateBy !== "None") {
throw new Error("Alt text generation is not yet supported for aggregated plots. To generate an alt text, set aggregation to 'None' in the left sidebar.");
Expand Down
8 changes: 5 additions & 3 deletions packages/app/src/components/Header/index.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { exportState, getAccessibleData, downloadSVG } from '@visdesignlab/upset2-react';
import { Column, getRows } from '@visdesignlab/upset2-core';
import { Column } from '@visdesignlab/upset2-core';
import { UserSpec } from 'multinet';
import RedoIcon from '@mui/icons-material/Redo';
import UndoIcon from '@mui/icons-material/Undo';
Expand All @@ -24,6 +24,7 @@ import { loadingAtom } from '../../atoms/loadingAtom';
import { getMultinetDataUrl } from '../../api/getMultinetDataUrl';
import { getUserInfo } from '../../api/getUserInfo';
import { oAuth } from '../../api/auth';
import { rowsSelector } from '../../atoms/selectors';

const Header = ({ data }: { data: any }) => {
const { workspace } = useRecoilValue(queryParamAtom);
Expand All @@ -34,6 +35,7 @@ const Header = ({ data }: { data: any }) => {
const setLoading = useSetRecoilState(loadingAtom);

const { provenance } = useContext(ProvenanceContext);
const rows = useRecoilValue(rowsSelector(provenance.getState()))

const [ attributeDialog, setAttributeDialog ] = useState(false);
const [ showImportModal, setShowImportModal ] = useState(false);
Expand Down Expand Up @@ -117,7 +119,7 @@ const Header = ({ data }: { data: any }) => {
await Promise.all([
localforage.clear(),
localforage.setItem('data', data),
localforage.setItem('rows', getAccessibleData(getRows(data, provenance.getState()), true)),
localforage.setItem('rows', getAccessibleData(rows, true)),
localforage.setItem('visibleSets', visibleSets),
localforage.setItem('hiddenSets', hiddenSets.map((set: Column) => set.name))
]);
Expand Down Expand Up @@ -272,7 +274,7 @@ const Header = ({ data }: { data: any }) => {
<MenuItem onClick={() => exportState(provenance)} color="inherit" aria-label="UpSet JSON state file download">
Export State
</MenuItem>
<MenuItem onClick={() => exportState(provenance, data, getRows(data, provenance.getState()))} aria-label="Download UpSet JSON state file with table data included">
<MenuItem onClick={() => exportState(provenance, data, rows)} aria-label="Download UpSet JSON state file with table data included">
Export State + Data
</MenuItem>
<MenuItem onClick={() => downloadSVG()} aria-label="SVG Download of this upset plot">
Expand Down
11 changes: 11 additions & 0 deletions packages/app/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,17 @@ import { client_id } from './api/env';
import { readSharedLoginCookie, writeSharedLoginCookie, invalidateSharedLoginCookie } from 'multinet';
import localforage from 'localforage';

// Not quite recommended but the only way I could get why-did-you-render to work with vite
// from https://github.com/welldone-software/why-did-you-render/issues/243#issuecomment-1112542230
if (process.env.NODE_ENV === 'development') {
// Yes, ts complains, but it works
const { default: whyDidYouRender } = await import('@welldone-software/why-did-you-render');
whyDidYouRender(React, {
logOnDifferentValues: true,
trackAllPureComponents: true,
});
}

// import reportWebVitals from './reportWebVitals';

localforage.config({
Expand Down
2 changes: 1 addition & 1 deletion packages/app/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"compilerOptions": {
"target": "es5",
"types": ["@emotion/react/types/css-prop"],
"types": ["@emotion/react/types/css-prop", "node"],
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
Expand Down
31 changes: 13 additions & 18 deletions packages/core/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,18 @@ import {
Row, Rows, Sets, UpsetConfig,
} from './types';

/**
* Maps Row IDs to Row objects
*/
export type RowMap = Record<string, Row>;

/**
* Calculates the first aggregation for the given data and state.
* @param data - The data object containing items, sets, and attribute columns.
* @param state - The UpsetConfig object containing the configuration state.
* @returns The result of the first aggregation.
*/
export const firstAggRR = (data: any, state: UpsetConfig) => {
const firstAggRR = (data: any, state: UpsetConfig) => {
const subsets = getSubsets(data.items, data.sets, state.visibleSets, data.attributeColumns);
return firstAggregation(
subsets,
Expand All @@ -34,7 +39,7 @@ export const firstAggRR = (data: any, state: UpsetConfig) => {
* @param state - The configuration state for the aggregation.
* @returns The second-level aggregation result.
*/
export const secondAggRR = (data: any, state: UpsetConfig) => {
const secondAggRR = (data: any, state: UpsetConfig) => {
const rr = firstAggRR(data, state);

if (areRowsAggregates(rr)) {
Expand All @@ -60,7 +65,9 @@ export const secondAggRR = (data: any, state: UpsetConfig) => {
* @param state - The state configuration containing the visible sets and sorting options.
* @returns The sorted rows based on the RR and the provided sorting options.
*/
export const sortByRR = (data: any, state: UpsetConfig) => {
const sortByRR = (data: any, state: UpsetConfig) => {
if (!data || typeof data !== 'object' || !Object.hasOwn(data, 'sets')) return { order: [], values: {} };

const vSets: Sets = Object.fromEntries(Object.entries(data.sets as Sets).filter(([name, _set]) => state.visibleSets.includes(name)));
const rr = secondAggRR(data, state);

Expand All @@ -74,7 +81,7 @@ export const sortByRR = (data: any, state: UpsetConfig) => {
* @param state - The state object containing the Upset configuration.
* @returns The filtered rows based on the RR algorithm and the provided filters.
*/
export const filterRR = (data: any, state: UpsetConfig) => {
const filterRR = (data: any, state: UpsetConfig) => {
const rr = sortByRR(data, state);

return filterRows(rr, state.filters);
Expand Down Expand Up @@ -142,7 +149,7 @@ export const flattenedRows = (data: any, state: UpsetConfig) => {
* @param state - The UpsetConfig state.
* @returns An object containing only the rows.
*/
export const flattenedOnlyRows = (data: any, state: UpsetConfig) => {
export function flattenedOnlyRows(data: any, state: UpsetConfig): RowMap {
const rows = flattenedRows(data, state);
const onlyRows: { [key: string]: Row } = {};

Expand All @@ -151,16 +158,4 @@ export const flattenedOnlyRows = (data: any, state: UpsetConfig) => {
});

return onlyRows;
};

/**
* Calculates the number of rows in the data based on the provided state.
*
* @param data - The data to calculate the rows count from.
* @param state - The state object containing the configuration for the calculation.
* @returns The number of rows in the data.
*/
export const rowsCount = (data: any, state: UpsetConfig) => {
const rr = flattenedRows(data, state);
return rr.length;
};
}
5 changes: 5 additions & 0 deletions packages/upset/src/atoms/attributeAtom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ export const attributeAtom = atom<string[]>({
default: [],
});

/**
* Gets all non-NaN values for a given attribute
* @param {string} attribute Attribute name
* @returns {number[]} All numeric (!Number.isNaN) values for this attribute
*/
export const attributeValuesSelector = selectorFamily<number[], string>({
key: 'attribute-values',
get:
Expand Down
Loading
Loading