Skip to content

Commit

Permalink
App performance enhancements (#5022)
Browse files Browse the repository at this point in the history
* fix spotlight cleanup

* remove modal navigation from router transactions

* cache gathered paths

* e2e updates

* update selection test

* lint, add comments

* event listener cleanup

* freeze tensorflow dep

* add < > shortcuts in help modal

* add test for shortcuts

* details

* clear frame stream lru cache

* fix shortcutToHelpItems

* add destroy items

* lint

* weak map resets and item deletions

* add max stream sizing configurations

* linting

* bugs

* clear stream cache on mouseleave

* better spotlight heuristics

* cleanup

* soft edit

* constants tweaks

* modal nav enhancements

* rm bytes sizing

* better aspect ratio thresholds

* tweaks

* config docs

* wait to stream for videos

* lru looker cache

* cleanup

* isReusable fixes

* cleanup

* move hook, fix e2e

* increase lru cache

* update screenshots

* rm python settings, add to options action

---------

Co-authored-by: Br2850 <bdbristol@gmail.com>
Co-authored-by: Sashank Aryal <sashank2012@gmail.com>
  • Loading branch information
3 people authored Nov 5, 2024
1 parent 991d620 commit 83f9ede
Show file tree
Hide file tree
Showing 60 changed files with 933 additions and 668 deletions.
15 changes: 14 additions & 1 deletion app/packages/app/src/routing/RouterContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { Queries } from "../makeRoutes";
import type RouteDefinition from "./RouteDefinition";
import type { LocationState, MatchPathResult } from "./matchPath";

import { viewsAreEqual } from "@fiftyone/state";
import { NotFoundError, Resource, isNotebook } from "@fiftyone/utilities";
import { createBrowserHistory, createMemoryHistory } from "history";
import React from "react";
Expand Down Expand Up @@ -183,13 +184,25 @@ export const createRouter = <T extends OperationType>(
};
};

const SKIP_EVENTS = new Set(["modal", "slice"]);
const SKIP_EVENTS = new Set(["modal", "slice", "spaces"]);

const makeGetEntryResource = <T extends OperationType>() => {
let currentLocation: FiftyOneLocation;
let currentResource: Resource<Entry<T>>;

const isReusable = (location: FiftyOneLocation) => {
if (location.pathname !== currentLocation?.pathname) {
return false;
}

if (location.search !== currentLocation?.search) {
return false;
}

if (!viewsAreEqual(location.state.view, currentLocation?.state.view)) {
return false;
}

if (currentLocation) {
return (
SKIP_EVENTS.has(location.state.event || "") ||
Expand Down
2 changes: 1 addition & 1 deletion app/packages/app/src/routing/matchPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const compilePath = (path: string) =>
});

export type LocationState<T extends OperationType = OperationType> = {
event?: "modal" | "slice";
event?: "modal" | "slice" | "spaces";
fieldVisibility?: State.FieldVisibilityStage;
groupSlice?: string;
modalSelector?: ModalSelector;
Expand Down
4 changes: 2 additions & 2 deletions app/packages/app/src/useEvents/useSetSpaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ const useSetSpaces: EventHandlerHook = ({ router }) => {
(payload) => {
setter("sessionSpaces", payload.spaces);
const state = router.history.location.state as LocationState;
router.history.replace(
router.replace(
resolveURL({
currentPathname: router.history.location.pathname,
currentSearch: router.history.location.search,
extra: {
workspace: payload.spaces._name ?? null,
},
}),
{ ...state, workspace: payload.spaces }
{ ...state, event: "spaces", workspace: payload.spaces }
);
},
[router, setter]
Expand Down
4 changes: 2 additions & 2 deletions app/packages/app/src/useWriters/onSetSessionSpaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ const onSetSessionSpaces: RegisteredWriter<"sessionSpaces"> =
({ environment, router, subscription }) =>
(spaces) => {
const state = router.history.location.state as LocationState;
router.history.replace(
router.replace(
resolveURL({
currentPathname: router.history.location.pathname,
currentSearch: router.history.location.search,
extra: {
workspace: spaces._name || null,
},
}),
{ ...state, workspace: spaces._name || null }
{ ...state, event: "spaces", workspace: spaces._name || null }
);

commitMutation<setSpacesMutation>(environment, {
Expand Down
8 changes: 7 additions & 1 deletion app/packages/core/src/components/Actions/ActionsRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import {
} from "@fiftyone/components";
import { FrameLooker, ImageLooker, VideoLooker } from "@fiftyone/looker";
import {
OperatorPlacements,
OperatorPlacementWithErrorBoundary,
OperatorPlacements,
types,
useOperatorBrowser,
useOperatorPlacements,
Expand Down Expand Up @@ -277,6 +277,12 @@ const Selected = ({
refresh?.();
}, [samples.size, refresh]);

useEffect(() => {
return () => {
setLoading(false);
};
}, []);

if (samples.size < 1 && !modal) {
return null;
}
Expand Down
86 changes: 85 additions & 1 deletion app/packages/core/src/components/Actions/Options.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ import {
import * as fos from "@fiftyone/state";
import {
configuredSidebarModeDefault,
frameCacheSize,
groupStatistics,
sidebarMode,
} from "@fiftyone/state";
import React, { RefObject, useMemo } from "react";
import type { RefObject } from "react";
import React, { useMemo } from "react";
import {
useRecoilState,
useRecoilValue,
Expand All @@ -20,6 +22,7 @@ import {
import { LIGHTNING_MODE, SIDEBAR_MODE } from "../../utils/links";
import Checkbox from "../Common/Checkbox";
import RadioGroup from "../Common/RadioGroup";
import { lookerGridCaching } from "../Grid/recoil";
import { Button } from "../utils";
import { ActionOption } from "./Common";
import Popout from "./Popout";
Expand Down Expand Up @@ -354,6 +357,86 @@ const ShowModalNav = () => {
);
};

const Caching = ({ modal }: { modal?: boolean }) => {
const [caching, setCaching] = useRecoilState(lookerGridCaching);
const [frameStream, setFrameStream] = useRecoilState(frameCacheSize);
const resetFrameCacheSize = useResetRecoilState(frameCacheSize);
const theme = useTheme();

return (
<>
{!modal && (
<>
<ActionOption
id="grid-caching"
text="Visualizer caching"
title={"Visualizer caching"}
style={{
background: "unset",
color: theme.text.primary,
paddingTop: 0,
paddingBottom: 0,
}}
svgStyles={{ height: "1rem", marginTop: 7.5 }}
/>
<Checkbox
name={"Cache grid visualizers"}
value={caching}
setValue={setCaching}
/>
</>
)}
{
<>
<ActionOption
id="frame-stream"
text="Frame cache size"
title={"Frame cache size"}
style={{
background: "unset",
color: theme.text.primary,
paddingTop: 0,
paddingBottom: 0,
}}
svgStyles={{ height: "1rem", marginTop: 7.5 }}
/>
<Selector
placeholder="Frame cache size"
onSelect={async (text) => {
if (text === "") {
resetFrameCacheSize();
return "1024";
}
const value = Number.parseInt(text);

if (Number.isNaN(value)) {
resetFrameCacheSize();
return "1024";
}

setFrameStream(value);

return String(value);
}}
inputStyle={{
fontSize: "1rem",
textAlign: "right",
float: "right",
width: "100%",
}}
value={String(frameStream)}
containerStyle={{
display: "flex",
justifyContent: "right",
marginBottom: "0.5rem",
}}
/>
</>
}
</>
);
};

type OptionsProps = {
modal?: boolean;
anchorRef: RefObject<HTMLElement>;
Expand All @@ -375,6 +458,7 @@ const Options = ({ modal, anchorRef }: OptionsProps) => {
{!view?.length && <Lightning />}
{!modal && <SidebarMode />}
<SortFilterResults modal={modal} />
<Caching modal={modal} />
</Popout>
);
};
Expand Down
53 changes: 28 additions & 25 deletions app/packages/core/src/components/Grid/Grid.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import styles from "./Grid.module.css";

import { freeVideos } from "@fiftyone/looker";
import type { ID } from "@fiftyone/spotlight";
import Spotlight from "@fiftyone/spotlight";
import type { Lookers } from "@fiftyone/state";
import * as fos from "@fiftyone/state";
import React, {
useEffect,
Expand All @@ -27,11 +25,9 @@ import useThreshold from "./useThreshold";

function Grid() {
const id = useMemo(() => uuid(), []);
const lookerStore = useMemo(() => new WeakMap<ID, Lookers>(), []);
const spacing = useRecoilValue(gridSpacing);

const selectSample = useRef<ReturnType<typeof useSelectSample>>();
const { pageReset, reset } = useRefreshers();
const { caching, lookerStore, pageReset, reset } = useRefreshers();
const [resizing, setResizing] = useState(false);
const threshold = useThreshold();

Expand All @@ -50,19 +46,28 @@ function Grid() {
const getFontSize = useFontSize(id);

const spotlight = useMemo(() => {
/** SPOTLIGHT REFRESHER */
reset;
/** SPOTLIGHT REFRESHER */

if (resizing) {
return undefined;
}

return new Spotlight<number, fos.Sample>({
...get(),
destroy: (id) => {
const looker = lookerStore.get(id.description);
looker?.destroy();
lookerStore.delete(id.description);
},
onItemClick: setSample,
retainItems: caching,
rowAspectRatioThreshold: threshold,
get: (next) => page(next),
render: (id, element, dimensions, soft, hide) => {
if (lookerStore.has(id)) {
const looker = lookerStore.get(id);
if (lookerStore.has(id.description)) {
const looker = lookerStore.get(id.description);
hide ? looker?.disable() : looker?.attach(element, dimensions);

return;
Expand All @@ -74,35 +79,33 @@ function Grid() {
throw new Error("bad data");
}

const init = (l) => {
l.addEventListener("selectthumbnail", ({ detail }: CustomEvent) => {
selectSample.current?.(detail);
});
lookerStore.set(id, l);
l.attach(element, dimensions);
};

if (!soft) {
init(
createLooker.current?.(
{ ...result, symbol: id },
{
fontSize: getFontSize(),
}
)
);
if (soft) {
// we are scrolling fast, skip creation
return;
}

const looker = createLooker.current?.(
{ ...result, symbol: id },
{
fontSize: getFontSize(),
}
);
looker.addEventListener("selectthumbnail", ({ detail }) =>
selectSample.current?.(detail)
);
lookerStore.set(id.description, looker);
looker.attach(element, dimensions);
},
scrollbar: true,
spacing,
});
}, [
caching,
createLooker,
get,
getFontSize,
lookerStore,
page,
records,
reset,
resizing,
setSample,
Expand Down
33 changes: 32 additions & 1 deletion app/packages/core/src/components/Grid/recoil.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,38 @@
import { atom, selector } from "recoil";
import { DefaultValue, atom, atomFamily, selector } from "recoil";

import * as fos from "@fiftyone/state";

const lookerGridCachingStore = atomFamily<boolean, string>({
key: "lookerGridCachingStore",
default: true,
effects: (id) => [
fos.getBrowserStorageEffectForKey(`looker-grid-caching-${id}`, {
valueClass: "boolean",
}),
],
});

export const lookerGridCaching = selector({
key: "lookerGridCaching",
get: ({ get }) => {
const id = get(fos.datasetId);
if (!id) {
throw new Error("no dataset");
}
return get(lookerGridCachingStore(id));
},
set: ({ get, set }, value) => {
const id = get(fos.datasetId);
if (!id) {
throw new Error("no dataset");
}
set(
lookerGridCachingStore(id),
value instanceof DefaultValue ? false : value
);
},
});

export const defaultGridZoom = selector<number>({
key: "defaultGridZoom",
get: ({ get }) => get(fos.config)?.gridZoom,
Expand Down
2 changes: 1 addition & 1 deletion app/packages/core/src/components/Grid/useFontSize.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useCallback } from "react";
import useThreshold from "./useThreshold";

const MAX = 32;
const MAX = 14;
const MIN = 10;
const SCALE_FACTOR = 0.09;

Expand Down
Loading

0 comments on commit 83f9ede

Please sign in to comment.