Skip to content

Commit

Permalink
perf: improve blink when feature is updated on reearth/core (#429)
Browse files Browse the repository at this point in the history
perf: improve blink when feature is updated
  • Loading branch information
keiya01 committed Feb 6, 2023
1 parent 2a2af14 commit c10a671
Show file tree
Hide file tree
Showing 2 changed files with 192 additions and 2 deletions.
153 changes: 153 additions & 0 deletions src/core/mantle/atoms/compute.test.ts
Expand Up @@ -457,6 +457,159 @@ test("computeAtom with cache", async () => {
expect(fetchDataMock).toBeCalledTimes(4);
});

test("computeAtom only value", async () => {
const data: TestData = {
test_id: "xxx",
type: "geojson",
value: {
type: "Feature",
geometry: {
type: "Point",
coordinates: [1, 1],
},
},
};
const features: Feature[] = [
{ type: "feature", id: "xxx", geometry: { type: "Point", coordinates: [1, 1] } },
];
const features2: Feature[] = [
{ type: "feature", id: "zzz", geometry: { type: "Point", coordinates: [100, 100] } },
];
const fetchDataMock = vi.spyOn(DataCache, "fetchData");
fetchDataMock.mockImplementation(async data => {
return [
{
type: "feature",
id: (data as any).test_id || "",
geometry: data.value?.geometry || { type: "Point", coordinates: [0, 0] },
properties: undefined,
range: undefined,
},
];
});
const internalFeatures: Feature[] = [
{ type: "feature", id: features[0].id, geometry: { type: "Point", coordinates: [1, 1] } },
];
const internalFeatures2: Feature[] = [
{ type: "feature", id: features2[0].id, geometry: { type: "Point", coordinates: [100, 100] } },
];
const { result } = renderHook(() => {
const atoms = useMemo(() => computeAtom(doubleKeyCacheAtom<string, string, Feature[]>()), []);
const [result, set] = useAtom(atoms);
return { result, set };
});

expect(result.current.result).toBeUndefined();

// Set data.value
await act(async () => {
await waitFor(() =>
result.current.set({
type: "setLayer",
layer: {
id: "xxx",
type: "simple",
data,
},
}),
);
});

expect(result.current.result).toEqual({
id: "xxx",
layer: {
id: "xxx",
type: "simple",
data,
},
status: "ready",
features: toComputedFeature(features),
originalFeatures: [...features],
});

expect(fetchDataMock).toBeCalledTimes(1);

await act(async () => {
await waitFor(() =>
result.current.set({
type: "setLayer",
layer: {
id: "xxx",
type: "simple",
data,
marker: {
imageColor: "black",
},
},
}),
);
});

expect(result.current.result).toEqual({
id: "xxx",
layer: {
id: "xxx",
type: "simple",
data,
marker: {
imageColor: "black",
},
},
status: "ready",
features: toComputedFeature(internalFeatures),
originalFeatures: [...internalFeatures],
});

// Should not fetch if value is not changed
expect(fetchDataMock).toBeCalledTimes(1);

const changedData = {
...data,
test_id: "zzz",
value: {
...data.value,
geometry: {
type: "Point",
coordinates: [100, 100],
},
},
};

await act(async () => {
await waitFor(() =>
result.current.set({
type: "setLayer",
layer: {
id: "xxx",
type: "simple",
data: changedData,
marker: {
imageColor: "black",
},
},
}),
);
});

expect(result.current.result).toEqual({
id: "xxx",
layer: {
id: "xxx",
type: "simple",
data: changedData,
marker: {
imageColor: "black",
},
},
status: "ready",
features: toComputedFeature(internalFeatures2),
originalFeatures: [...internalFeatures2],
});

// Should fetch if value is changed
expect(fetchDataMock).toBeCalledTimes(2);
});

vi.mock("../evaluator", (): { evalLayer: typeof evalLayer } => ({
evalLayer: async (layer: LayerSimple, ctx: EvalContext) => {
if (!layer.data) return { layer: {}, features: undefined };
Expand Down
41 changes: 39 additions & 2 deletions src/core/mantle/atoms/compute.ts
@@ -1,6 +1,7 @@
import { atom } from "jotai";
import { merge, pick } from "lodash-es";
import { merge, pick, isEqual } from "lodash-es";

import { processGeoJSON } from "../data/geojson";
import { evalLayer, EvalResult } from "../evaluator";
import type {
ComputedFeature,
Expand Down Expand Up @@ -85,20 +86,52 @@ export function computeAtom(cache?: typeof globalDataFeaturesCache) {

set(layerStatus, "fetching");

const shouldFetch = (prevFeatures: Feature[] | undefined) => {
if (currentLayer.type !== "simple") {
return false;
}

// `data.url` should be cached on dataAtoms, so we can return true in this line.
if (currentLayer.data?.url || !currentLayer.data?.value) {
return true;
}

// Only support geojson for specifying direct feature.
if (currentLayer.data?.type !== "geojson") {
return false;
}

const curFeatures = processGeoJSON(currentLayer.data.value);
if (curFeatures.length === prevFeatures?.length) {
return !curFeatures.every((cur, i) => {
const prev = prevFeatures[i];
return isEqual({ ...cur, id: "" }, { ...prev, id: "" });
});
}

return true;
};

// Used for a simple layer.
// It retrieves all features for the layer stored in the cache,
// but attempts to retrieve data from the network if the main feature is not yet in the cache.
const getAllFeatures = async (data: Data) => {
const getterAll = get(dataAtoms.getAll);
const allFeatures = getterAll(layerId, data);

const flattenAllFeatures = allFeatures?.flat();

// Ignore cache if data is embedded
if (
allFeatures &&
// Check if data has cache key
DATA_CACHE_KEYS.every(k => !!data[k])
) {
return allFeatures.flat();
return flattenAllFeatures;
}

if (!shouldFetch(flattenAllFeatures)) {
return flattenAllFeatures;
}

await set(dataAtoms.fetch, { data, layerId });
Expand All @@ -115,6 +148,10 @@ export function computeAtom(cache?: typeof globalDataFeaturesCache) {
// Ignore cache if data is embedded
if (c) return c;

if (!shouldFetch(c)) {
return c;
}

await set(dataAtoms.fetch, { data, range, layerId });
return getter(layerId, data, range);
};
Expand Down

0 comments on commit c10a671

Please sign in to comment.