[HOLD for payment 2025-01-15] [HOLD for payment 2024-12-25] Benchmark performance bottlenecks when opening chats

[adhorodyski]

Coming from the discussion thread under https://expensify.slack.com/archives/C05LX9D6E07/p1728314452317409

Problem

Lack of solid performance benchmarks across the core app workflows, baselines for proposing performance improvements.

Solution

• Focus on 1 scenario of opening a chat
• Benchmark it in detail to get the granular results (also for each phase) so we have solid numbers for whom specifically it's slow and why. There are also analytics we can use for this to have more datapoints.
• Pinpoint the areas for improvement we see
• Map how the proposal from initial thread/nitro modules/anything other can fit to improve it

cc @hannojg

Issue Owner

Current Issue Owner: @laurenreidexpensify

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021873115691475801087
• Upwork Job ID: 1873115691475801087
• Last Price Increase: 2024-12-28

[adhorodyski]

@mountiny could you please assign this one to me & @hannojg? Thanks!

[hannojg]

Thanks! 👋

[adhorodyski]

Update
Our bottom line is to get super solid on what precisely is slow (have granular measurements) and for whom specifically.

Who
To be able to say how many people something affects (in different percentiles), we need to have something to segment by. We'll extend our performance analytics' metadata (in all envs, including e2e tests) to include the below fields. These will help us group measurements by the types of accounts. (PR: #50881)

• number of reports ✅
• number of reportActions 🔄
• number of personalDetails ✅
• number of policies 🔄
• number of transactions 🔄
• number of transactionViolations 🔄

What
To be able to pinpoint what exactly is slow, we'll implement granular metrics in code, across both App & Onyx. Here's a breakdown of all the phases we decided to focus on.

• APPLYING_UPDATES (coming over https, until we schedule notifying subscribers)
  • DATABASE_WRITE (actual save operations like multiset and multimerge)
• NOTIFYING_SUBSCRIBERS (multiple key(s)Changed triggered by scheduleNotifyCollectionSubscribers)
  • DATABASE_READ
  • REACT_PROVIDER - (useOnyx/withOnyx)
• ONYX_DATA_POSTPROCESSING (from receiving a new update (subscriber got called) until we have the final data)

How
TBC

[adhorodyski]

cc @mountiny we'd like to ask you for any feedback about this 1st batch (please invite others!). Is there an area we're missing? I want to make sure we focus on the right parts when profiling this, the exact tools (like e2e suites) will come next once we fully agree on the who&what.

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

• Extend perf analytics metadata #50881

If no regressions arise, payment will be issued on 2024-10-29. 🎊

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

• @hannojg does not require payment (Contractor)
• @adhorodyski does not require payment (Contractor)

[melvin-bot]

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

[adhorodyski]

The extended analytics setup is merged now:

• to be confirmed it's available in Firebase for newly reported events
• to be merged into local-based profiling tools (on top of react-native-performance within the Performance module?) so we can use the same parameters in eg. e2e workflows cc @hannojg, I think this is the last thing we have to do when it comes to the 'segmentation' part

[adhorodyski]

• Next part is to implement the granular metrics as listed above

also cc @hannojg, do you think these should also be reported to Firebase OR we just focus on being able to profile this behind some flag?

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

• feat: export performance data #54412

If no regressions arise, payment will be issued on 2025-01-15. 🎊

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

• @hannojg does not require payment (Contractor)
• @adhorodyski does not require payment (Contractor)
• @rayane-djouah requires payment (Needs manual offer from BZ)

[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:

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

[adhorodyski]

@muttmuure yeah that was totally I think due to this underlying bug in Hermes. I'll monitor when it's up. @mountiny the latest testflight includes upgrade, correct?

[mountiny]

Yes

[adhorodyski]

Works now

[adhorodyski]

Ok so I've found a few bugs on different environments that I can't go over quickly:

• https://staging.new.expensify.com/ on web allows me to profile, but does not seem to produce the AppInfo file
• https://new.expensify.com/ on web does not allow me to profile, Use profiling does not toggle on with a console error of [Violation] Document policy violation: js-profiling is not allowed in this document.
• TestFlight build is similar to the web staging - allows me to profile, but does not produce the AppInfo

cc @hannojg are these known issues, am I missing something? Shouldn't AppInfo actually be produced on all these targets?

[rayane-d]

• [@rayane-djouah] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.

I think no regression test is needed since this is a performance debug tool

• @rayane-djouah requires payment (Needs manual offer from BZ)

My eligibility for NewDot payments began on December 28th. I will request payment via NewDot once the onboarding process is completed.

[melvin-bot]

Payment Summary

Upwork Job

• Contributor: @hannojg is from an agency-contributor and not due payment
• Contributor: @adhorodyski is from an agency-contributor and not due payment
• ROLE: @rayane-djouah paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@laurenreidexpensify)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1873115691475801087/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[adhorodyski]

I think there are no actions needed in the scope of this task, the feature works as expected though I have some doubts it's equally accessible across all the platforms (it haven't been touched here). Should we follow up on this one as per ^? cc @mountiny

[mountiny]

@adhorodyski if you mean this #50652 (comment) yes lets try to follow up

[laurenreidexpensify]

@rayane-djouah are you onboarded yet for payment?

[rayane-d]

@laurenreidexpensify Not yet. Could you please check the status with the team?

[laurenreidexpensify]

@rayane-djouah have done, am bumping the team to get it resolved today 👍

[rayane-d]

@laurenreidexpensify Resolved! Could you kindly provide a payment summary?

[garrettmknight]

@laurenreidexpensify got the payment request in ND, just need the payment summary when you get a chance. 😄

[laurenreidexpensify]

payment summary

@rayane-d $250 C+ please request in ND

[garrettmknight]

$250 approved for @rayane-d