LegendApp / legend-state

Legend-State is a super fast and powerful state library that enables fine-grained reactivity and easy automatic persistence
https://legendapp.com/open-source/state/
MIT License
3.06k stars 87 forks source link

Conceptual Batching Question #290

Closed realPrimoh closed 6 months ago

realPrimoh commented 6 months ago

Hi,

I have a question about batching.

I have a function that looks like this:

export async function getBodyweightsLegend(daysBack: number) {
    for (let i = 0; i <= daysBack; i++) {
        const startOfDay = dayjs().subtract(i, 'day').startOf('day');
        const endOfDay = dayjs().subtract(i - 1, 'day').startOf('day');

        const bodyweight = getSomeDataFromExternalStore();

        const dateKey = startOfDay.format('YYYY-MM-DD');
        const quantity = bodyweight.quantity;
        if (quantity > 0) {
            bodyweights$.set((prev) => {
                const newMap = new Map(prev);
                newMap.set(dateKey, quantity);
                return newMap;
            });
        }
    }
}

I usually call this upon app startup in a useEffect. I have maybe ~8-10 of these functions for different metrics that are all called in a Promise.all(). The app now is super laggy upon startup presumably because there are excessive re-renders all being done at once.

How should this function (and all the similar functions) be batched? Should I wrap the .set in a batch or the entire for loop in the beginBatch(); endBatch(); calls?

Thank you!

jmeistrich commented 6 months ago

One thing that might be slowing it down is cloning the Maps. Legend-State doesn’t require immutables and you can interact with the observable Map just like a regular Map, so you can just set the key directly on the observable Map. That might even be all you need to improve the performance.

bodyweights$.set(dateKey, quantity)

Wrapping the whole loop in a batch would be good as long as it’s all synchronous, to make it trigger subscribers only once, either with beginBatch/endBatch or wrap the whole thing in batch(() => { for(…) }

But if you have 8-10 of these all running async then that might trigger re-render 8-10 different times. So if it’s still slow you might want to do it in two parts, await all the calculations with a Promise.all and then apply the calculations onto the observables in a batch. That might add too much complexity and end up being slower or use more memory for storing temporary values, but it could speed it up.

Another idea since these are all running on mount, if you want them to all complete at the same time, is to have a single const state$ = useObservable() that contains all of the data. In the useEffect you can build up all the calculations asynchronously into a local allTheData object and then at the end after everything is complete, state$.set(allTheData) to re-render one time with the full data.

realPrimoh commented 6 months ago

This is very helpful, thanks! Removing the map cloning and adding some batching basically fixed the issue.