Recoil: Performance issues when resetting a modestly sized atomFamily

[kysley]

I have a atomFamily, wordState, that contains 250+ members on average. I keep track of the 'ids' in a separate atom, wordListState.

The user needs to be able to reset the members of wordState back to default. I implemented this using a useRecoilCallback which loops through the wordListState, resetting each atom individually.

Ideally, this would be an 'instant' operation, however it takes anywhere from 100ms-500ms+ to complete.

Basically, wordState contains each letter of a word. Each letter is an object with some meta information which determines how it is rendered. When the user wants to reset, we need wordListState to have a new list of words, and each atom in the wordState atomFamily to be reset.

You can find a minimal reproduction here, with the update time logged to the console.

This might not be the most efficient way to get the job done either, would be open to optimizations as well!

[manyopensource]

Why do you need this:

for (let i = words.length; i--; i >= 0) {
    reset(wordState(i));
}

[kysley]

wordState can contain duplicate words/entries (intended) so we use the array index.
I found this comment by @drarmstr in #263 and took that approach.

If you're maintaining the set of items in itemListState, then you can iterate over each item to reset it in itemState family. Since using hooks conditionally or in a loop is problematic, you can use useRecoilCallback()

[manyopensource]

I don't understand what problem do you trying to solve by this approach!

[kysley]

Let me try to give some more context!

wordListState -> Array of words which are available
wordState -> Array of objects for a given word index.

// The word "hey" becomes
[
  {
    id: '0',
    letter: 'h'
    // ...
  },
  {
    id: '1',
    letter: 'e'
    // ...
  },
  {
    id: '2',
    letter: 'y'
    // ...
  },
]

When building a typing test app, each letter of a word contains its own data. This is kept in wordState. For example wordState(0) contains the letters for "recoil". Each letter in "recoil" is an object with some metadata.

We use the words index in the wordListState array because "recoil" may appear multiple times in the list of words to be typed.

When the user wants to reset the typing test, we need to flush out the contents of wordState. Which is what I'm having difficulties with!

[EricPKerr]

It looks like it's the await snapshot.getPromise(wordListState); that's taking a long time, not the reset calls

[manyopensource]

@EricPKerr I checked and it's really the reset calls that cause the issue!

[EricPKerr]

Apologies - ran it again and you're correct. Not sure what I had checked before.

[manyopensource]

I've changed the minimal reproduction to fix the issue:

import React from "react";
import "./styles.css";
import "./state";
import { useRecoilCallback, useRecoilValue, useRecoilState } from "recoil";
import { wordListState, wordState, populateWords } from "./state";
import Word from "./Word";

export default function App() {
  const [wordList, setWordList] = useRecoilState(wordListState);
  const resetWordState = useRecoilCallback(
    ({ reset, snapshot, set }) => async () => {
      const before = performance.now();

      for (let i = wordList.length; i--; i >= 0) {
        reset(wordState(i));
      }

      // Typically this would be data from an api call
      setWordList(populateWords());

      const after = performance.now();
      console.log("reset duration -> " + (after - before) + "ms");
    }
  );

  const words = useRecoilValue(wordListState);

  return (
    <div className="App">
      <button onClick={() => resetWordState()}>reset word list</button>
      <div>
        {words.map((word, i) => (
          <Word index={`${word}-${i}`} i={i} />
        ))}
      </div>
    </div>
  );
}

[kysley]

@manyopensource Wow! very interesting. Thanks for digging into it, really appreciate it. Do you have any idea why it behaves this way?

[kysley]

May be worth noting that @EricPKerr was partially correct here. As long as we read wordListState from outside of the useRecoilCallback, it seems to update in an acceptable time frame (4-6ms).

Here is a slightly modified repro from above:

import React from "react";
import "./styles.css";
import "./state";
import { useRecoilCallback, useRecoilValue, useRecoilState } from "recoil";
import { wordListState, wordState, populateWords } from "./state";
import Word from "./Word";

export default function App() {
  const wordList = useRecoilValue(wordListState);
  const resetWordState = useRecoilCallback(
    ({ reset, snapshot, set }) => async () => {
      const before = performance.now();

      // const wordListStateSnapshot = await snapshot.getPromise(wordListState);

      // using wordListStateSnapshot.length instead of wordList.length will cause recoil to buckle here.
      for (let i = wordList.length; i--; i >= 0) {
        reset(wordState(i));
      }

      // Typically this would be data from an api call
      set(wordListState, populateWords());

      const after = performance.now();
      console.log("reset duration -> " + (after - before) + "ms");
    }
  );

  return (
    <div className="App">
      <button onClick={() => resetWordState()}>reset word list</button>
      <div>
        {wordList.map((word, i) => (
          <Word index={`${word}-${i}`} i={i} />
        ))}
      </div>
    </div>
  );
}

If you uncomment the snapshot to get the length of wordListState, then recoil seems to buckle. It seems to have nothing to do with setting wordListState inside the useRecoilCallback.

[manyopensource]

Yes, he was absolutely right! You don't need the line const words = await snapshot.getPromise(wordListState);

[Harrisonl]

Yep I ran to a similar issue where an atom with 20 - 30 objects, was taking ~1 second when using await snapshot.getPromise(atom) found that instead using a set function to get the current value (set(atom, (current) => ....)) worked much better. Not sure if this has any stale data implications but in my case it wasn't the end of the world if that was the case

[drarmstr]

Yep I ran to a similar issue where an atom with 20 - 30 objects, was taking ~1 second when using await snapshot.getPromise(atom) found that instead using a set function to get the current value (set(atom, (current) => ....)) worked much better. Not sure if this has any stale data implications but in my case it wasn't the end of the world if that was the case

Using an updater function with set in order to get the current value is actually the recommended way to ensure you are working with the latest value for the current React tree state. Looking at the snapshot with either getPromise() or getLoadable() (which should be a bit faster) will provide the currently committed React state when the callback was initiated.

[drarmstr]

This performance should be improved with 0.1.1, please let us know if you continue to have an issue.