Optimize getTransactionsSections to reduce redundant computation#60630
Conversation
9f24a42 to
303967b
Compare
|
@rushatgabhane Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
reviewing |
|
@rushatgabhane please lets try to prioritize this one thanks! |
There was a problem hiding this comment.
There is a false cache hit after taking an action like Approve on a report
Screen.Recording.2025-04-25.at.04.47.25.mov
There was a problem hiding this comment.
Fixed by adding report state and action to the cache key — should update properly now.
303967b to
0d1b5fa
Compare
|
@rushatgabhane ready for you again, thanks! |
Reviewer Checklist
Screenshots/VideosAndroid: NativeWhatsApp.Video.2025-04-28.at.19.53.44.mp4Android: mWeb ChromeiOS: NativeScreen.Recording.2025-04-28.at.18.32.50.moviOS: mWeb SafariScreen.Recording.2025-04-28.at.18.41.30.movMacOS: Chrome / SafariScreen.Recording.2025-04-28.at.18.19.20.movMacOS: Desktop |
|
Posted in the
|
|
@mallenexpensify umm not sure. Anyone could work |
|
Thanks Rushat, reassigned the issue and @marcaaron was the lucky one |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/marcaaron in version: 9.1.38-0 🚀
|
|
Thanks for handling Marc! |
|
Seems like there are some issues with the caching approach here though so reverting here #61145 |
|
🚀 Deployed to production by https://github.com/thienlnam in version: 9.1.38-4 🚀
|
Explanation of Change
Problem
Switching between bottom tabs on native devices is sluggish — even for small accounts — and the performance degrades significantly for users with a large amount of data.
After profiling the app, I identified that he
getTransactionsSections()function was being called frequently and reprocessed the same transaction data every time, even when the underlying data hadn’t changed.Solution
To address this, I introduced a local cache to store already-processed transaction objects and avoid redundant computation. The new
processedTransactionsCachemethod stores fully formattedTransactionListItemTypeobjects. It uses a hash of the transaction key combined with formatting flags as its cache key. This ensures that if a transaction has already been processed with the same inputs, we can reuse the existing result instead of recomputing it.I also extracted the formatting logic into two reusable helper functions:
processTransaction()andgetProcessedTransaction(). This not only centralizes and simplifies the formatting logic, but also makes it easier to extend these optimizations to other areas of the app — such asgetReportSections().Fixed Issues
$ #60007
PROPOSAL:
Tests
This change is part of a set of performance optimizations. It introduces caching to avoid redundant transaction processing. While the improvement may not be immediately noticeable, it helps reduce unnecessary computation when navigating the Reports tab.
Offline tests
Same as in Tests
QA Steps
Same as in Tests
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop