[HOLD for payment 2024-03-19] [$250] [Performance] optimise sorting for nonArchivedReports

[mountiny]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number:
Reproducible in staging?:
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:

PR is a part of Callstack performance audit of the app

In this PR, hot path of sorting nonArchivedReports inside of getOrderedReportIDs was optimised to return early when dates are sufficient to be used as a comparison criterion.

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010fd35a8396dac225
• Upwork Job ID: 1765757106598993920
• Last Price Increase: 2024-03-07

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~010fd35a8396dac225

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @greg-schroeder (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot]

Upwork job price has been updated to $250

[webbdays]

How do we know only dates are required for sorted in this case.
Any variable? similar to how we check isInDefaultMode while sorting.

[melvin-bot]

📣 @webbdays! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[webbdays]

@mountiny

[webbdays]

Proposal

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

Optimise sorting for nonArchivedReports when only dates are sufficient as comparator for sorting.

What is the root cause of that problem?

N/A as this is the new feature to optimise sorting for nonArchivedReports when only dates are sufficient as comparator for sorting.

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

I have went through the code.
This is how its currently implemented
It sorts with display names when all the dates of reports are same (no order). Sorts via dates only in default mode.
For this new feature to improve sorting, we can use this if condition if (isDatesComparatorEnough) {return compareDates}
there like

if (isInDefaultMode) {
        nonArchivedReports.sort((a, b) => {
            const compareDates = a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0;
            if (isDatesComparatorEnough) {return compareDates}
            const compareDisplayNames = a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0;
            return compareDates || compareDisplayNames;
        });

What alternative solutions did you explore?

So to handle the case where dates is enough to compare, here we can use ternary operator when returning the comparator. Like this return isDatesComparatorEnough? compareDates : compareDates || compareDisplayNames;
(this is less efficient than above, Negligible improvement but just mentioning because why define another variable which is not required in that case).

Thanks for reading.

[muas19]

Proposal

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

optimize sorting for nonArchivedReports

What is the root cause of that problem?

compareDisplayNames is being computed unnecessarilly if compareDates is true. This is the bottleneck.

App/src/libs/SidebarUtils.ts

Lines 166 to 168 in f550e84

	const compareDates = a?.lastVisibleActionCreated && b?.lastVisibleActionCreated ? compareStringDates(b.lastVisibleActionCreated, a.lastVisibleActionCreated) : 0;
	const compareDisplayNames = a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0;
	return compareDates || compareDisplayNames;

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

Check compareDates in an if statement:

nonArchivedReports.sort((a, b) => {
    const compareDates =
      a?.lastVisibleActionCreated && b?.lastVisibleActionCreated
        ? compareStringDates(
            b.lastVisibleActionCreated,
            a.lastVisibleActionCreated
          )
        : 0;

//check if here
    if (compareDates) {
        return compareDates;
    }
    const compareDisplayNames =
      a?.displayName && b?.displayName
        ? a.displayName.toLowerCase().localeCompare(b.displayName.toLowerCase())
        : 0;
    return compareDisplayNames;

What alternative solutions did you explore? (Optional)

[mountiny]

Merged

[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.50-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• [NoQA] [Audit][Implementation] - optimise sorting for nonArchivedReports #37841

If no regressions arise, payment will be issued on 2024-03-19. 🎊

[melvin-bot]

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

• [@mountiny] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[melvin-bot]

Skipping the payment summary for this issue since all the assignees are employees or vendors. If this is incorrect, please manually add the payment summary SO.

[mountiny]

no further steps required here