[Image][Performance] Better caching for iOS images

[Szymon20000]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

What performance issue do we need to solve?

Better caching for iOS images

What is the impact of this on end-users?

Previously seen avatars/assets will show up faster with suggested change.

List any benchmarks that show the severity of the issue

Proposed solution (if any)

Use FastImage for Avatars and images that are par of the messages.
Avatar.js uses Image from react-native
react-native-render-html - that as far as I know is used for rendering attachment (among the other things) uses Image from 'react-native' if the change with avatars will give us good results it's worth to test patching that library as well. So that it uses fast-image.

List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Upwork URL: https://www.upwork.com/jobs/~013a8690a2c460efd2

View all open jobs on Upwork

Reported by: @parasharrajat originally on this issue.

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019ac837ee43fe141c
• Upwork Job ID: 1620162574495588352
• Last Price Increase: 2023-01-30

[melvin-bot]

📣 @Szymon20000! 📣
Please report bugs or suggest features in the #expensify-open-source Slack channel, don't directly open issues in this repo!
Instructions here to join the channel 📖

[melvin-bot]

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[roryabraham]

Switching this to weekly

[roryabraham]

@Szymon20000 I saw you put up a draft PR but you weren't sure about the change because right now FastImage isn't compatible with Fabric. Overall I think we should prioritize Fabric compatibility over using FastImage.

Looking into FastImage some more, it seems like one of the main things it handles is persistent image caching. However, that functionality feels potentially in-scope for our persistent storage library Onyx. Going to add this issue to the image tracking issue here

[roryabraham]

Any new thoughts here @Szymon20000?

[mallenexpensify]

Been a minute.... let's see...
@roryabraham , should Alex Beaman be assigned to cuz he's leading
[Tracking] [Polish] Image-related feature request and improvements #10894

If so, I reckon I should be too since I'm the the BugZero (BZ) team person for that tracking issue.

Also... @roryabraham and/or @Szymon20000 , is this actively being worked on? If so, any update?

[roryabraham]

should Alex Beaman be assigned to cuz he's leading

Yes, I think so.

Also... @roryabraham and/or @Szymon20000 , is this actively being worked on? If so, any update?

Definitely not by me – I don't think there's been any active development on this.

[mallenexpensify]

Bumped to weekly cuz on hold

[Beamanator]

Hmm ok well I've been OOO for the week 1 & 2 updates so haven't reached out yet. It looks like there still hasn't been movement on the upstream PR (DylanVann/react-native-fast-image#953) so I say we can try to reach out to see if the maintainer has time to look at the PR this week - and then if there's still no movement by EOW we fork & work on our own version

[Beamanator]

Ok reached out, I'll post back here if I get a response

[mananjadhav]

@Beamanator @mallenexpensify This might be ready for payout by tomorrow, so before we do that I want to clarify how do we handle the payment for this one. I reviewed two PRs for this issue:

1. [Image][Performance] Fast Image with Auth Token Header Caching #12648 - Was reverted as it had a regression
2. [Image][Performance] Fast Image with Auth Token Header Caching (revert follow-up) #13304 - Merged and deployed to production on Jan 20, 2023.

Both the PRs were extremely complex and tedious to review. @Beamanator had also mentioned about the pay bump for the first PR review here

and overall both the PR codes are a lot different. Can we please increase payout for this one to accommodate the complexity and number of PRs.

[mallenexpensify]

Thanks @mananjadhav, this is definitely a tricky one. I'm going to start an internal discussion, hang tight plz...

[mallenexpensify]

@mananjadhav after internal discussion we believe that $4k, in total, is a fair amount, based on the complexity and also because of the regression. Do you agree @mananjadhav ?
@thomas-coldwell is with Margelo and his compensation is paid outside of our standard process.

[mananjadhav]

Do you agree @mananjadhav ?

Thanks @mallenexpensify. Agreed it is fair :)

[mananjadhav]

@mallenexpensify Quick bump on this one.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~019ac837ee43fe141c

[melvin-bot]

Current assignee @mananjadhav is eligible for the Internal assigner, not assigning anyone new.

[mallenexpensify]

@mananjadhav can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~019ac837ee43fe141c

[mananjadhav]

Accepted @mallenexpensify

[mallenexpensify]

@mananjadhav paid $4000.

I'm guessing this doesn't need regression test steps added. @Beamanator can you confirm? And close if so? Thx

[parasharrajat]

@mallenexpensify the reporting bonus contract is still unpaid for me.

[mallenexpensify]

Thanks @parasharrajat , I missed it as I paid Manan on a different Upwork job.
All set!

[Beamanator]

@mallenexpensify good question, I believe you're right, I think we can close this out - also @trjExpensify it looks like you added the upstream PR link to the issue title, but I made this issue to track that PR here: #14382

I think this is better so we can close out this very large issue, thoughts?

[trjExpensify]

Ah yeah, totally. That was before we went the fork route. Make sense. We good to close this out then?

[Beamanator]

Aah that makes sense. Ya I'm happy to let @mallenexpensify close it, not sure if all the payments are done

[mallenexpensify]

All paid!