[C-1322] Android and general performance improvements by sliptype · Pull Request #2149 · AudiusProject/audius-client

sliptype · 2022-10-14T23:12:10Z

Description

Sorry for the big pr 😅

use useProxySelector for all notifications to prevent rerenders when navigating
Reduce rerenders of skeleton by memoizing the animation and starting the animation from the onLayout callback
Increase max listeners of remote config EventEmitter to ignore warning about possible memory leak
Reduce DynamicImage rerenders by memoizing styles that are passed in and only using a single onLayout callback to set the size for both images

This may cause issues if the sizes of the 2 images are different but I don't think we have that use case
Reduce rerenders when playing from a lineup by pushing the play toggling logic into the lineup sagas. This allows each of the tile components to not have to know about the current playing state unless it is the currently playing track

This is a fairly big change and also result in us firing analytics from inside the lineup sagas. Would love thoughts here!

@dylanjeffers I reverted the navigation changes for now and will address in next pr because I was seeing the double navigation problem. Will try to use getNavigator to fix that

Dragons

Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

How will this change be monitored?

For features that are critical or could fail silently please describe the monitoring/alerting being added.

Feature Flags

Are all new features properly feature flagged? Describe added feature flags.

This reverts commit 0309301.

dylanjeffers

awesome work, stoked to see the improvements in dynamic image and skeleton in particular. big lol that we were Stylesheet.createing 100s of times every page. Notif work is great too

dylanjeffers · 2022-10-14T23:16:02Z

        <TrackItem
          key={track.uid}
-          active={playingUid === track.uid}
+          uid={track.uid}


so the main idea here is that we don't rerender every tile when playinguid changes yeah?

Yes, this line is specific to the items in the collection tile but the other changes in the pr achieve what you said

dylanjeffers · 2022-10-14T23:17:28Z

  onLoad,
  style
 }: LineupTileArtProps) => {
-  const styles = useThemedStyles(createStyles)


dylanjeffers · 2022-10-14T23:18:52Z

 }

 export const LineupTileCoSign = ({ coSign }: Props) => {
-  const trackTileStyles = useThemedStyles(createTrackTileStyles)


we should prob look to remove useThemeStyles everywhere at some point

I'm down, there are a lot of uses though

yeah the whole sign-up flow could use a perf facelift 🗡️

dylanjeffers · 2022-10-14T23:19:29Z

-  const styles = useThemedStyles(createStyles)
-  const trackTileStyles = useThemedStyles(createTrackTileStyles)
+  const styles = useStyles()
+  const trackTileStyles = useTrackTileStyles()


this should prob help a ton

dylanjeffers · 2022-10-14T23:20:19Z

 * It's rerendering because bottomTab render function rerenders a lot.
 */
-export const NowPlayingDrawer = memo(function NowPlayngDrawer(
+export const NowPlayingDrawer = memo(function NowPlayingDrawer(


dylanjeffers · 2022-10-14T23:20:32Z

      {user && track ? (
        <>
-          <Pressable onPress={onPressTitle}>
+          <TouchableOpacity onPress={onPressTitle}>


what's the idea here? just better looking ui?

Yeah it felt weird to have the pressable things with no feedback

dylanjeffers · 2022-10-14T23:21:09Z

-      backgroundColor: themeColors.skeleton
-    }
-  })
+const useStyles = makeStyles(({ palette }) => ({


wow... we didn't have this for skeleton, lmao rippp

dylanjeffers · 2022-10-14T23:22:08Z

-          setShimmerWidth(width)
-          setShimmerPos(new Animated.Value(-0.75 * width))
+          shimmerPos.setValue(-0.75 * width)
+          shimmer.start()


smart so does this prevent a rerender or just speed up when we start to shimmer?

Prevents a render bc we aren't setting width in state

dylanjeffers · 2022-10-14T23:22:35Z

-    getNotificationEntity(state, notification)
+  const entity = useProxySelector(
+    (state) => getNotificationEntity(state, notification),
+    [notification]


so it is the case that "notification" object is stable?

I believe so, this is how the other notif types are doing it

dylanjeffers · 2022-10-14T23:23:23Z

  yield put(queueActions.pause({}))
 }

+function* togglePlay(lineupActions, lineupSelector, prefix, action) {


this makes sense to me

@sliptype lol rip i was doing the exact same thing in my branch 💀. but the good news is we both thought this was a good idea

Oh noooo, sorry I should have communicated that

audius-infra · 2022-10-14T23:32:35Z

Preview this change https://demo.audius.co/sk-android-perf

audius-infra · 2022-10-15T00:41:55Z

Preview this change https://demo.audius.co/sk-android-perf

[5cd5d68] [C-1322] Android and general performance improvements (#2149) Sebastian Klingler [f6ef033] [C-1341] Add notifications bottom tab screen (#2148) Dylan Jeffers [9ddfe72] [C-1335] Upgrade and share typescript version (#2145) Dylan Jeffers [b701102] [C-1336] Implement account drawer (#2146) Dylan Jeffers [8587b3f] Add structured data and update head (#2131) Joseph Lee [40ef3b7] [C-1317] Improve client startup tooling (#2140) Raymond Jacobson [e09c6f9] Update new tables to allow drag to add to playlist (#2134) Kyle Shanks [4633581] Bump ios (#2144) Sebastian Klingler [f331426] Slow down $AUDIO page Stripe/Coinbase button animation speed (#2143) Marcus Pasell [44916a5] [C-1291] Add base layout for navbar overhaul (#2139) Dylan Jeffers [e85955c] Prepend audius url to share url (#2142) Sebastian Klingler [068917e] Use Hermes for Android app C-1329 (#2130) nicoback2 [98f845c] [C-1327] Fix deep linking for tracks with diacritics (#2137) Sebastian Klingler [0ec460f] [PAY-685] Update Stripe button to use approved brand assets (#2138) Marcus Pasell [1b1a377] Refactor bottom-tab-bar (#2136) Dylan Jeffers [9dbb11e] fix play bar buttons target (#2135) nicoback2 [9daba05] [C-1303][C-1304] Fix misc. profile layout issues (#2129) Raymond Jacobson [04eb0c9] [C-1272] Fix Search UI jank (#2133) Sebastian Klingler [feaa6f0] Bump react-screens to 3.17.0 to avoid crash on android (#2132) Sebastian Klingler [db996e8] Remove old table components and rename new table components (#2126) Kyle Shanks [92a2c7e] [C-1319] Fix native profile collectibles (#2127) Dylan Jeffers [7a83f29] [C-1282] Remove followUser social action in profile reducer (#2121) Dylan Jeffers [26de098] [C-1309] Fix playlists/collectibles not loading (#2125) Dylan Jeffers [78dee89] Fix getState undefined when opening reopening app on android (#2123) Sebastian Klingler [ef69b70] [C-1318] Revert "[C-1184] Fetch lineups after coming back online (#2083)" (#2122) Andrew Mendelsohn [42efd1b] [PAY-457] Add Geofilter for Coinbase, Stripe (#2118) Marcus Pasell [e413d9a] Bring back unhealthySlotDiffPlays (#2119) Dylan Jeffers [fb96ea4] Fix favorites page filtering on old tables (#2120) Kyle Shanks [cf73f53] Fix android build (#2113) Sebastian Klingler [ebaa5d8] [C-1040] Remove extraneous mobile logs (#2107) Dylan Jeffers [bc7e7b0] bump libs in web and common (#2117) nicoback2 [dc16dcb] bump libs 1.0.12 (#2116) nicoback2 [c737535] Undo change to prevent cache update when no collectibles (#2114) Sebastian Klingler [b2764ba] Keep screen awake when confirming tip (#2103) nicoback2 [4b39ec1] fix pw clipping on mobile (#2102) nicoback2

sliptype added 10 commits October 13, 2022 14:23

useProxySelector for mileston notification

fd03d39

Reduce navigation context rerenders

0309301

useProxySelector for remaining notifs

d47a0c0

Reduce rerenders of skeleton

c8f1912

Convert lineup tiles to useStyles

d98045e

Increase max listeners to ignore memory leak error

9696788

Reduce DynamicImage rerenders

9ddcd99

Change waudio balance to a console.log

03c67af

Reduce rerenders when playing from a lineup

3c68557

Revert "Reduce navigation context rerenders"

3de14db

This reverts commit 0309301.

sliptype requested review from dylanjeffers and nicoback2 October 14, 2022 23:12

pull-request-size Bot added the size/XL label Oct 14, 2022

Clean up lint

f0f746e

dylanjeffers approved these changes Oct 14, 2022

View reviewed changes

Fix lint

3c82466

sliptype merged commit 5cd5d68 into main Oct 15, 2022

sliptype deleted the sk-android-perf branch October 15, 2022 00:41

AudiusProject deleted a comment from linear Bot Sep 11, 2023

Uh oh!

Conversation

sliptype commented Oct 14, 2022

Description

Dragons

How Has This Been Tested?

How will this change be monitored?

Feature Flags

Uh oh!

dylanjeffers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

audius-infra commented Oct 14, 2022

Uh oh!

audius-infra commented Oct 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants