[pending payment] [$250] [LOW] [Performance] Opening a transaction thread can be slow to load sometimes

[mountiny]

Coming from here

Problem

One issue that I've consistently noticed as an approver is that opening a transaction thread seems to be much slower than opening a chat or an expense report. It doesn't seem to be the API call that is slow as I'm seeing OpenReport respond in a few hundred milliseconds. It seems to be something on the client that causes a delay before we even call OpenReport. Is this something that someone could look into?

Logs:
logs-2024-05-27 17_37_19.380.txt

JSON trace from Chrome:
In the Slack post

Screen.Recording.2024-05-27.at.1.36.37.PM.mp4

Solution

Profile and investigate opening the transaction thread and why it's taking so much longer even if the API request is fast.

You can:

1. Create a Workspace as Alice
2. Invite another member to it (Bob)
3. Go to the Workspace settings
4. More Features
5. Enable Workflows
6. Setup approvals
7. Set Alice as the Approver on the workspace
8. Sign in as Bob
9. Create many requests from the workspace
10. As Alice, navigate to the expense report
11. Open the transaction thread
12. Profile opening the transaction thread

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01d2d0fcf90d743949
• Upwork Job ID: 1795399434607542272
• Last Price Increase: 2024-05-28
• Automatic offers:
• aimane-chnaif | Reviewer | 102511992

Issue Owner

Current Issue Owner: @joekaufmanexpensify

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01d2d0fcf90d743949

[melvin-bot]

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[mountiny]

cc @puneetlath

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif (External)

[mountiny]

@TMisiukiewicz is investigating this today and tomorrow

[joekaufmanexpensify]

Sounds good! @TMisiukiewicz could you please comment so I can assign you?

[brunovjk]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Opening transaction threads can sometimes be slow, especially the first time a thread is opened. While subsequent openings are faster due to caching, there is still room for improvement in overall performance.

What is the root cause of that problem?

The root cause appears to be twofold:

1. Initial Data Fetching and Processing: The first-time opening of a transaction thread involves fetching and processing various data points (report actions, personal details, policy data) from the API and initializing them in Onyx. This process, particularly the reportActionsExist function and the way it interacts with the allReportActions data structure, seems to be the most significant bottleneck in the first-time open scenario.

2. Onyx Updates: The construction and application of Onyx updates, even in subsequent openings, can still contribute to some overhead. While the API call itself is relatively fast in both scenarios, the client-side processing of the data and updates to the Onyx store could potentially be optimized.

What changes do you think we should make in order to solve the problem?

1. Optimize reportActionsExist:

   • Investigate the implementation of reportActionsExist and the allReportActions data structure.
   • Consider using lazy loading to fetch report actions only when needed for a specific report, rather than loading all of them upfront.
   • Explore the possibility of using targeted Onyx updates instead of subscribing to all report actions changes, to reduce unnecessary updates.
   • If allReportActions becomes a performance bottleneck with a large number of reports, evaluate alternative data structures that might be more efficient for storing and accessing report actions.

2. Streamline Onyx Updates:

   • Analyze the Onyx updates being performed in both the first-time and subsequent opens.
   • Look for opportunities to batch updates or minimize unnecessary updates to reduce overhead.
   • Consider using more efficient update mechanisms if available.

3. Lazy Loading of Report Data:

   • If the initial API response contains a large amount of data that's not immediately needed for rendering, implement lazy loading.
   • This would involve fetching only the necessary data upfront and loading the rest in the background, improving initial loading time.

What alternative solutions did you explore? (Optional)

• Preloading Report Data: Preload report data based on usage patterns or predictions to make the first-time open experience smoother. However, carefully consider potential impacts on network usage and storage.

• Caching API Responses: While Onyx is currently used for caching, explore alternative caching mechanisms like React Query or SWR. These might offer additional features and flexibility that could further improve performance.

[brunovjk]

I was investigating this issue and have put together a proposal based on my findings. Now that I see that a Callstack member will be assign, I'm happy to step back. However, I wanted to share my proposal in case it's helpful.

Please let me know if you'd like me to remove it :D

Thanks!

[charles-liang]

@brunovjk I agree with your proposal. I using React performance tests, and the page rendering time is not very long. There is a slight delay when parsing all_report whiling all_report larger, but it does not exceed 1 second. The real issue lies with the server response. Optimizing only the frontend without modifying the backend will have limited effectiveness.

[mountiny]

The backend from our findings is not the bottleneck, as you can see in the video, the response of OpenReport was fast. there is something in frontend which is causing the transaction threads slower

@brunovjk thank you for the proposal, @TMisiukiewicz can work with you as well if they find that you are on a good path

[rinej]

hello, I am Tomek from Callstack, I am going to take it over from @TMisiukiewicz :)

[melvin-bot]

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[rinej]

We investigated a trace from the slack thread and observed that formatWithUTCTimeZone is called 2,897 times. This function is invoked whenever getTransactionDetails is called. It seems likely that getTransactionDetails is being triggered for multiple transactions or that the component is rerendered multiple times for some reason.

Optimising formatWithUTCTimeZone might help reduce the time, but it's even more important to identify why it is being called so many times. I will post the update as far as we have more details.

[joekaufmanexpensify]

Sounds good. Thanks for the update!

[joekaufmanexpensify]

@rinej is there an ETA on a PR here?

[joekaufmanexpensify]

Not overdue

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.84-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Opening a transaction thread can be slow to load sometimes #43384

If no regressions arise, payment will be issued on 2024-06-24. 🎊

For reference, here are some details about the assignees on this issue:

• @rinej does not require payment (Contractor)
• @aimane-chnaif requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.85-7 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Opening a transaction thread can be slow to load sometimes #43384

If no regressions arise, payment will be issued on 2024-06-28. 🎊

For reference, here are some details about the assignees on this issue:

• @rinej does not require payment (Contractor)
• @aimane-chnaif requires payment automatic offer (Reviewer)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@aimane-chnaif] The PR that introduced the bug has been identified. Link to the PR:
• [@aimane-chnaif] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@aimane-chnaif] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@aimane-chnaif] Determine if we should create a regression test for this bug.
• [@aimane-chnaif] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[joekaufmanexpensify]

@aimane-chnaif mind completing checklist here so we can prep to issue payment?

[aimane-chnaif]

This is not regression but performance improvement. I don't think regression test is needed.

[joekaufmanexpensify]

Sounds good!

[joekaufmanexpensify]

Only payment here is $250 to @aimane-chnaif for C+ via Upwork

[joekaufmanexpensify]

@aimane-chnaif please accept our offer and then I'll pay you!

[joekaufmanexpensify]

Bumped in slack

[joekaufmanexpensify]

@aimane-chnaif asked that I set this to weekly for now, as he can't accept the Upwork offer until next week.

[aimane-chnaif]

Offer expired. Can you please send new offer?

[joekaufmanexpensify]

New job here.

[joekaufmanexpensify]

@aimane-chnaif offer sent!

[aimane-chnaif]

Accepted. Thanks

[joekaufmanexpensify]

@aimane-chnaif $250 sent and contract ended!

[joekaufmanexpensify]

Upwork job closed.

[joekaufmanexpensify]

All set, thanks everyone!