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

[Payment due March 7] [Performance] Remove cache from the getOrderedReportIDs method #36799

Closed
mountiny opened this issue Feb 19, 2024 · 10 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Reviewing Has a PR in review Task

Comments

@mountiny
Copy link
Contributor

mountiny commented Feb 19, 2024

Coming from Slack

Problem

Whenever we want to calculate order of reports in the LHN, the getOrderedReportIDs function is called. Based on our investigations for different metrics (Report load time and Send a message), we can tell that the getOrderedReportIDs is a main bottleneck responsible for app's degrading performance.

Details can be found in the Cache part of this analysis

### Solution
In order to fix the underlying issue, we have splitted the implementation into a few atomic PRs that could be easily digested. One of them is presented below.

Followed the analysis, the overhead that stems from cache lookup when generating ordered reports does not match its possible positive outcomes on overall time execution. Analysis, as well as, measurements with possible gains of removing the cache are presented in linked issues.

@kacper-mikolajczak
Copy link
Contributor

Hi @mountiny

@mountiny mountiny added the Reviewing Has a PR in review label Feb 19, 2024
Copy link

melvin-bot bot commented Feb 26, 2024

@mountiny, @kacper-mikolajczak Whoops! This issue is 2 days overdue. Let's get this updated quick!

@mountiny
Copy link
Contributor Author

Merged

Copy link

melvin-bot bot commented Mar 5, 2024

@mountiny, @kacper-mikolajczak Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Copy link

melvin-bot bot commented Mar 5, 2024

@mountiny, @kacper-mikolajczak Whoops! This issue is 2 days overdue. Let's get this updated quick!

@jjcoffee
Copy link
Contributor

jjcoffee commented Mar 7, 2024

I am due payment for the PR review on this. Deployed to production last week so should be due payment today, I think. 🙇

@mallenexpensify mallenexpensify added the Bug Something is broken. Auto assigns a BugZero manager. label Mar 7, 2024
Copy link

melvin-bot bot commented Mar 7, 2024

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@Christinadobrzyn Christinadobrzyn changed the title [Performance] Remove cache from the getOrderedReportIDs method [Payment due March 7] [Performance] Remove cache from the getOrderedReportIDs method Mar 8, 2024
@Christinadobrzyn
Copy link
Contributor

Payouts due:

Contributor+: $500 @jjcoffee (in Upwork - offer sent - https://www.upwork.com/nx/wm/offer/101274490

Upwork job is here.

sent you an offer @jjcoffee!

@jjcoffee
Copy link
Contributor

jjcoffee commented Mar 8, 2024

@Christinadobrzyn Accepted, thanks! 🙇

@Christinadobrzyn
Copy link
Contributor

Awesome! Paid to you @jjcoffee based on this payment summary #36799 (comment)

Closing

@github-project-automation github-project-automation bot moved this from CRITICAL to Done in [#whatsnext] #quality Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Reviewing Has a PR in review Task
Projects
Development

No branches or pull requests

5 participants