Skip to content

Commit

Permalink
use more granular strategy for caching
Browse files Browse the repository at this point in the history
  • Loading branch information
sashankaryal committed Jan 11, 2025
1 parent ebd99ae commit 3975c8b
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 86 deletions.
25 changes: 24 additions & 1 deletion app/packages/core/src/components/Grid/Grid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import React, {
useRef,
useState,
} from "react";
import { useRecoilValue } from "recoil";
import { useRecoilCallback, useRecoilValue } from "recoil";
import { v4 as uuid } from "uuid";
import { QP_WAIT, QueryPerformanceToastEvent } from "../QueryPerformanceToast";
import { gridActivePathsLUT } from "../Sidebar/useShouldReloadSample";
import { gridCrop, gridSpacing, pageParameters } from "./recoil";
import useAt from "./useAt";
import useEscape from "./useEscape";
Expand Down Expand Up @@ -46,6 +47,15 @@ function Grid() {
const setSample = fos.useExpandSample(store);
const getFontSize = useFontSize(id);

const getCurrentActiveLabelFields = useRecoilCallback(
({ snapshot }) =>
() => {
return snapshot
.getLoadable(fos.activeLabelFields({ modal: false }))
.getValue();
}
);

const spotlight = useMemo(() => {
/** SPOTLIGHT REFRESHER */
reset;
Expand All @@ -61,6 +71,7 @@ function Grid() {
const looker = lookerStore.get(id.description);
looker?.destroy();
lookerStore.delete(id.description);
gridActivePathsLUT.delete(id.description);
},
detach: (id) => {
const looker = lookerStore.get(id.description);
Expand Down Expand Up @@ -101,6 +112,18 @@ function Grid() {
);
lookerStore.set(id.description, looker);
looker.attach(element, dimensions);

// initialize active paths tracker
const currentActiveLabelFields = getCurrentActiveLabelFields();
if (
currentActiveLabelFields &&
!gridActivePathsLUT.has(id.description)
) {
gridActivePathsLUT.set(
id.description,
new Set(currentActiveLabelFields)
);
}
},
scrollbar: true,
spacing,
Expand Down
21 changes: 4 additions & 17 deletions app/packages/core/src/components/Grid/useRefreshers.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { subscribe } from "@fiftyone/relay";
import * as fos from "@fiftyone/state";
import { LRUCache } from "lru-cache";
import { useEffect, useMemo, useRef } from "react";
import { useEffect, useMemo } from "react";
import uuid from "react-uuid";
import { useRecoilValue } from "recoil";
import { useOnSidebarSelectionChange } from "../Sidebar/useOnSidebarSelectionChange";
import { gridActivePathsLUT } from "../Sidebar/useShouldReloadSample";
import { gridAt, gridOffset, gridPage } from "./recoil";

const MAX_LRU_CACHE_ITEMS = 510;
Expand Down Expand Up @@ -86,28 +86,15 @@ export default function useRefreshers() {
/** LOOKER STORE REFRESHER */

return new LRUCache<string, fos.Lookers>({
dispose: (looker) => {
dispose: (looker, id) => {
looker.destroy();
gridActivePathsLUT.delete(id);
},
max: MAX_LRU_CACHE_ITEMS,
noDisposeOnSet: true,
});
}, [reset]);

const lookerStoreRef = useRef(lookerStore);
lookerStoreRef.current = lookerStore;

const gridLabelsToggleTracker = useOnSidebarSelectionChange({ modal: false });

/**
* Refresh all lookers when the labels toggle tracker changes
*/
useEffect(() => {
lookerStoreRef.current?.forEach((looker) => {
looker.refreshSample();
});
}, [gridLabelsToggleTracker]);

return {
lookerStore,
pageReset,
Expand Down
10 changes: 10 additions & 0 deletions app/packages/core/src/components/Grid/useSelect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as fos from "@fiftyone/state";
import type { LRUCache } from "lru-cache";
import { useEffect } from "react";
import { useRecoilValue } from "recoil";
import { useShouldReloadSampleOnActiveFieldsChange } from "../Sidebar/useShouldReloadSample";

export default function useSelect(
getFontSize: () => number,
Expand All @@ -12,6 +13,10 @@ export default function useSelect(
) {
const { init, deferred } = fos.useDeferrer();

const shouldRefresh = useShouldReloadSampleOnActiveFieldsChange({
modal: false,
});

const selected = useRecoilValue(fos.selectedSamples);
useEffect(() => {
deferred(() => {
Expand All @@ -29,6 +34,11 @@ export default function useSelect(
fontSize,
selected: selected.has(id.description),
});

// rerender looker if active fields have changed and have never been rendered before
if (shouldRefresh(id.description)) {
instance.refreshSample();
}
});

for (const id of store.keys()) {
Expand Down

This file was deleted.

59 changes: 59 additions & 0 deletions app/packages/core/src/components/Sidebar/useShouldReloadSample.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { activeLabelFields } from "@fiftyone/state";
import { useCallback } from "react";
import { useRecoilValue } from "recoil";

type LookerId = string;
type CachedLabels = Set<string>;

export const gridActivePathsLUT = new Map<LookerId, CachedLabels>();

export const useShouldReloadSampleOnActiveFieldsChange = ({
modal,
}: {
modal: boolean;
}) => {
const activeLabelFieldsValue = useRecoilValue(activeLabelFields({ modal }));

const shouldRefresh = useCallback(
(id: string) => {
const thisLookerActiveFields = gridActivePathsLUT.get(id);
const currentActiveLabelFields = new Set(activeLabelFieldsValue);

if (currentActiveLabelFields.size === 0) {
return false;
}

// diff the two sets, we only care about net new fields
// if there are no new fields, we don't need to update the tracker
let hasNewFields = false;

if (thisLookerActiveFields) {
for (const field of currentActiveLabelFields) {
if (!thisLookerActiveFields.has(field)) {
hasNewFields = true;
break;
}
}
} else {
hasNewFields = true;
}

if (!hasNewFields) {
return false;
}

gridActivePathsLUT.set(
id,
new Set([
...(thisLookerActiveFields ?? []),
...currentActiveLabelFields,
])
);

return true;
},
[activeLabelFieldsValue]
);

return shouldRefresh;
};
1 change: 1 addition & 0 deletions app/packages/looker/src/worker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ const processLabels = async (
activePaths.length &&
activePaths.includes(`${prefix ?? ""}${field}`)
) {
console.log(">>>painting ", `${prefix ?? ""}${field}`);
if (painterFactory[cls]) {
painterPromises.push(
painterFactory[cls](
Expand Down

0 comments on commit 3975c8b

Please sign in to comment.