Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize backendNetworks store #6409

Merged
merged 6 commits into from
Jan 23, 2025

Conversation

christianbaroni
Copy link
Member

@christianbaroni christianbaroni commented Jan 17, 2025

What changed (plus any additional context for devs)

  • The backendNetworks store methods are called thousands of times — this cuts total function execution time by ~95%+
    • Before: 3k calls in: ~97.5ms 🔴
      • (Best case — this was measuring only a few methods)
    • After: 3k calls in ~3.3ms 🟢
      • This is achieved by memoizing the store methods, which ensures derived state is only recomputed when the underlying data changes
  • Removes the duplicate worklet methods as they aren't needed — the various chain metadata records can be retrieved on the JS thread and then passed to worklets
  • Migrates the store to createQueryStore and removes the <BackendNetworks /> sync component, which existed in two places
  • Decreases fetch frequency to a max of once every 10 minutes
  • Skips store updates when the fetched data is unchanged
  • Removes the need for networks store access in the swap buy list info icons

Screen recordings / screenshots

What to test

@christianbaroni christianbaroni marked this pull request as draft January 17, 2025 22:50
@brunobar79
Copy link
Member

Launch in simulator or device for ee89095

@christianbaroni christianbaroni marked this pull request as ready for review January 18, 2025 22:10
@christianbaroni christianbaroni added the performance performance related improvements label Jan 18, 2025
@brunobar79
Copy link
Member

Launch in simulator or device for abe3268

@brunobar79
Copy link
Member

Launch in simulator or device for 7c6701e

@@ -154,7 +153,7 @@ const getInitialSliderXPosition = ({
export const SwapProvider = ({ children }: SwapProviderProps) => {
const { nativeCurrency } = useAccountSettings();

const backendNetworks = useBackendNetworksStore(state => state.backendNetworksSharedValue);
const [nativeChainAssets] = useState(useBackendNetworksStore.getState().getChainsNativeAsset());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason to not use useRef here?

@@ -97,6 +98,13 @@ export const TokenToBuyList = () => {
const { internalSelectedInputAsset, internalSelectedOutputAsset, isFetching, isQuoteStale, outputProgress, setAsset } = useSwapContext();
const { results: sections, isLoading } = useSearchCurrencyLists();

const [supportedChainsBooleanMap] = useState(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason not to use useRef here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's mostly a semantic choice, seems a bit cleaner to use useState since you can omit the setter, making it closer to naturally read only, and with useState you don't need to deal with .current.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable to me. Was just curious!

Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Insane work. Thanks for cleaning this up. Approved but left a couple small qs

Copy link
Member

@derHowie derHowie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The memoizing is cool. Seems like this file will serve as a good example of how to solve this particular issue going forward. 🫡

@brunobar79 brunobar79 added the release for release blockers and release candidate branches label Jan 23, 2025
@brunobar79 brunobar79 merged commit 2015cf9 into develop Jan 23, 2025
12 checks passed
@brunobar79 brunobar79 deleted the @christian/backend-networks-optimizations branch January 23, 2025 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance performance related improvements release for release blockers and release candidate branches
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants