Cache system color scheme, fix memoised ChatToJsx.

This commit improves performance considerably when there's lots of Text. - useColorScheme seems to cause unnecessary performance lag, and is called everytime Text is invoked. It is now only called once globally for the entire application, and passed via React context. - Memoising ChatToJsx properly reduces CPU load from chat parsing. ChatToJsx is no longer improperly memoised by default, and the item renderer in ChatScreen is memoised correctly instead.
retrixe · Nov 20, 2022 · 49ae5c4 · 49ae5c4
1 parent aed52ce
commit 49ae5c4
Show file tree

Hide file tree

Showing 4 changed files with 104 additions and 77 deletions.
diff --git a/src/App.tsx b/src/App.tsx
@@ -27,6 +27,7 @@ import SettingsContext, {
   Settings
 } from './context/settingsContext'
 import ServersContext, { Servers } from './context/serversContext'
+import { ColorSchemeContext } from './context/useDarkMode'
 import DisconnectDialog from './components/DisconnectDialog'
 import ChatScreen from './screens/chat/ChatScreen'
 import ServerScreen from './screens/ServerScreen'
@@ -92,7 +93,6 @@ const HomeScreen = ({ navigation }: { navigation: HomeNavigationProp }) => {
 }
 
 const App = () => {
-  const colorScheme = useColorScheme()
   const [connection, setConnection] = React.useState<Connection | undefined>()
   const [disconnect, setDisconnect] = React.useState<Disconnect | undefined>()
 
@@ -109,6 +109,8 @@ const App = () => {
     setAccountsStore(JSON.stringify(newAccounts))
   const setServers = (newServers: Servers) =>
     setServersStore(JSON.stringify(newServers))
+
+  const colorScheme = useColorScheme()
   const systemDefault = colorScheme === null ? true : colorScheme === 'dark'
   const darkMode =
     settings.darkMode === null ? systemDefault : settings.darkMode
@@ -138,31 +140,33 @@ const App = () => {
         <SettingsContext.Provider value={{ settings, setSettings }}>
           <ServersContext.Provider value={{ servers, setServers }}>
             <AccountsContext.Provider value={{ accounts, setAccounts }}>
-              {disconnect && <DisconnectDialog />}
-              <NavigationContainer theme={darkMode ? DarkTheme : undefined}>
-                <StatusBar
-                  backgroundColor={darkMode ? '#242424' : '#ffffff'}
-                  barStyle={darkMode ? 'light-content' : 'dark-content'}
-                />
-                <Stacks.Navigator
-                  id='StackNavigator'
-                  initialRouteName='Home'
-                  screenOptions={{
-                    headerShown: false,
-                    animation: 'slide_from_right'
-                  }}
-                >
-                  <Stacks.Screen name='Home' component={HomeScreen} />
-                  <Stacks.Screen
-                    name='Chat'
-                    component={ChatScreen}
-                    getId={({ params }) => {
-                      return (params as { serverName: string }).serverName
-                    }}
+              <ColorSchemeContext.Provider value={darkMode}>
+                {disconnect && <DisconnectDialog />}
+                <NavigationContainer theme={darkMode ? DarkTheme : undefined}>
+                  <StatusBar
+                    backgroundColor={darkMode ? '#242424' : '#ffffff'}
+                    barStyle={darkMode ? 'light-content' : 'dark-content'}
                   />
-                  <Stacks.Screen name='Settings' component={SettingScreen} />
-                </Stacks.Navigator>
-              </NavigationContainer>
+                  <Stacks.Navigator
+                    id='StackNavigator'
+                    initialRouteName='Home'
+                    screenOptions={{
+                      headerShown: false,
+                      animation: 'slide_from_right'
+                    }}
+                  >
+                    <Stacks.Screen name='Home' component={HomeScreen} />
+                    <Stacks.Screen
+                      name='Chat'
+                      component={ChatScreen}
+                      getId={({ params }) => {
+                        return (params as { serverName: string }).serverName
+                      }}
+                    />
+                    <Stacks.Screen name='Settings' component={SettingScreen} />
+                  </Stacks.Navigator>
+                </NavigationContainer>
+              </ColorSchemeContext.Provider>
             </AccountsContext.Provider>
           </ServersContext.Provider>
         </SettingsContext.Provider>

diff --git a/src/context/useDarkMode.ts b/src/context/useDarkMode.ts
@@ -1,12 +1,7 @@
-import { useContext } from 'react'
-import { useColorScheme } from 'react-native'
-import SettingsContext from '../context/settingsContext'
+import { createContext, useContext } from 'react'
 
-const useDarkMode = () => {
-  const colorScheme = useColorScheme()
-  const { settings } = useContext(SettingsContext)
-  const systemDefault = colorScheme === null ? true : colorScheme === 'dark'
-  return settings.darkMode === null ? systemDefault : settings.darkMode
-}
+export const ColorSchemeContext = createContext<boolean>(false)
+
+const useDarkMode = () => useContext(ColorSchemeContext)
 
 export default useDarkMode
diff --git a/src/minecraft/chatToJsx.tsx b/src/minecraft/chatToJsx.tsx
@@ -234,8 +234,8 @@ const parseChatToJsx = (
   )
 }
 
-// React Component-ised.
-const ChatToJsxNonMemo = (props: {
+// React Component-ised. Memoise if being called often.
+export const ChatToJsx = (props: {
   chat?: MinecraftChat
   component: React.ComponentType<TextProps>
   colorMap: ColorMap
@@ -251,9 +251,6 @@ const ChatToJsxNonMemo = (props: {
     props.componentProps,
     props.trim
   )
-// Memoisation means this is only re-rendered if the props changed.
-// TODO: This might be hurting memory usage.
-export const ChatToJsx = React.memo(ChatToJsxNonMemo)
 
 export const parseValidJson = (text: string) => {
   try {

diff --git a/src/screens/chat/ChatScreen.tsx b/src/screens/chat/ChatScreen.tsx
@@ -1,4 +1,10 @@
-import React, { useContext, useEffect, useRef, useState } from 'react'
+import React, {
+  useCallback,
+  useContext,
+  useEffect,
+  useRef,
+  useState
+} from 'react'
 import {
   FlatList,
   StyleSheet,
@@ -49,36 +55,57 @@ interface Message {
   text: MinecraftChat
 }
 
-const renderItem = (colorMap: ColorMap, handleCe: (ce: ClickEvent) => void) => {
-  const ItemRenderer = ({ item }: { item: Message }) => (
-    <View style={styles.androidScaleInvert}>
-      <ChatToJsx
-        chat={item.text}
-        component={Text}
-        colorMap={colorMap}
-        clickEventHandler={handleCe}
-      />
-    </View>
-  )
-  // LOW-TODO: Performance implications? https://reactnative.dev/docs/optimizing-flatlist-configuration
-  return ItemRenderer
-}
+const ItemRenderer = (props: {
+  item: Message
+  colorMap: ColorMap
+  clickEventHandler: (event: ClickEvent) => void
+}) => (
+  <View style={styles.androidScaleInvert}>
+    <ChatToJsx
+      chat={props.item.text}
+      component={Text}
+      colorMap={props.colorMap}
+      clickEventHandler={props.clickEventHandler}
+    />
+  </View>
+)
+
+const ItemRendererMemo = React.memo(
+  ItemRenderer,
+  (prev, next) =>
+    prev.item.key === next.item.key &&
+    prev.colorMap === next.colorMap &&
+    prev.clickEventHandler === next.clickEventHandler
+)
+
 const ChatMessageList = (props: {
   messages: Message[]
   colorMap: ColorMap
   clickEventHandler: (ce: ClickEvent) => void
 }) => {
+  // If colorMap/clickEventHandler changes, this will change and cause a re-render.
+  // If messages changes, FlatList will execute this function for all messages, and
+  // ItemRendererMemo will check if props have changed instead of this useCallback.
+  const renderItem = useCallback(
+    ({ item }) => (
+      <ItemRendererMemo
+        item={item}
+        colorMap={props.colorMap}
+        clickEventHandler={props.clickEventHandler}
+      />
+    ),
+    [props.colorMap, props.clickEventHandler]
+  )
   return (
     <FlatList
       inverted={Platform.OS !== 'android'}
       data={props.messages}
       style={[styles.androidScaleInvert, styles.chatArea]}
       contentContainerStyle={styles.chatAreaScrollView}
-      renderItem={renderItem(props.colorMap, props.clickEventHandler)}
+      renderItem={renderItem}
     />
   )
 }
-const ChatMessageListMemo = React.memo(ChatMessageList) // Shallow prop compare.
 
 const handleError =
   (addMessage: (text: MinecraftChat) => void, translated: string) =>
@@ -261,6 +288,29 @@ const ChatScreen = ({ navigation, route }: Props) => {
     }
   }
 
+  const handleClickEvent = useCallback(
+    (ce: ClickEvent) => {
+      // TODO: URL prompt support.
+      if (
+        ce.action === 'open_url' &&
+        settings.webLinks &&
+        (ce.value.startsWith('https://') || ce.value.startsWith('http://'))
+      ) {
+        Linking.openURL(ce.value).catch(() =>
+          addMessage(enderChatPrefix + 'Failed to open URL!')
+        )
+      } else if (ce.action === 'copy_to_clipboard') {
+        Clipboard.setString(ce.value)
+      } else if (ce.action === 'run_command') {
+        // TODO: This should be a prompt - sendMessage(ce.value, false)
+        setMessage(ce.value)
+      } else if (ce.action === 'suggest_command') {
+        setMessage(ce.value)
+      } // No open_file/change_page handling.
+    },
+    [settings.webLinks]
+  )
+
   const title =
     route.params.serverName.length > 12
       ? route.params.serverName.substring(0, 9) + '...'
@@ -302,29 +352,10 @@ const ChatScreen = ({ navigation, route }: Props) => {
       )}
       {!loading && connection && (
         <>
-          <ChatMessageListMemo
+          <ChatMessageList
             messages={messages}
             colorMap={darkMode ? mojangColorMap : lightColorMap}
-            clickEventHandler={ce => {
-              // TODO: URL prompt support.
-              if (
-                ce.action === 'open_url' &&
-                settings.webLinks &&
-                (ce.value.startsWith('https://') ||
-                  ce.value.startsWith('http://'))
-              ) {
-                Linking.openURL(ce.value).catch(() =>
-                  addMessage(enderChatPrefix + 'Failed to open URL!')
-                )
-              } else if (ce.action === 'copy_to_clipboard') {
-                Clipboard.setString(ce.value)
-              } else if (ce.action === 'run_command') {
-                // TODO: This should be a prompt - sendMessage(ce.value, false)
-                setMessage(ce.value)
-              } else if (ce.action === 'suggest_command') {
-                setMessage(ce.value)
-              } // No open_file/change_page handling.
-            }}
+            clickEventHandler={handleClickEvent}
           />
           <View style={darkMode ? styles.textAreaDark : styles.textArea}>
             <TextField