perf: Skip a write to storage when nothing is actually changing

[kidroca]

@marcaaron

Details

PR3. Of the changes planned here: #88 (comment)

Update set to check with cache and skip writing to storage. Cache reflects what's already on disc, if we have the exact value in cache it means it didn't change and we don't need to make a write

Quite a few times we're trying to "overwrite" something that didn't change, the most notable one is modal where I would see 19 consecutive calls to write false and on top of that the value in storage is already false

If cache has a value it's always the most recent up to date value
So when we try to write a value, but we have a cached key and the exact same value in cache we can skip the write

Related Issues

Expensify/App#2667

Automated Tests

Covered by existing unit tests

Linked PRs

Benchmarks

Hash the benchmarks were performed on: Expensify/App@b2582f3

Legend

• Total: sum of all calls made by Onyx
• Last call ended at: end time of the last call made by Onyx (often this is not a "get" call, but a "set")
• Last long Onyx#get at: the last time a get call took more than 100ms to complete
• Get "currencyList" at: The time Onyx.get is called with the currencyList key. Used as a reference point as usually it's the point on Android where the "last long" call happens.

Android Before

	Run 1	Run 2	Run 3	Run 4	Run 5	Run 6	Run 7	Run 8
Total	153.08sec	156.62sec	149.23sec	149.76sec	163.39sec
Last call ended at	18.45sec	20.24sec	18.29sec	19.10sec	20.45sec
Onyx#set time	22.99sec	22.02sec	21.44sec	21.25sec	22.11sec
Last long Onyx#get at	13.84sec	14.54sec	8.38sec	8.49sec	9.62sec
Get "currencyList" at	8.36sec	8.96sec	8.38sec	8.49sec	9.62sec

• Full data: https://docs.google.com/spreadsheets/d/110nn3zjjBQ5MUqzk_Q_QJshOMX8r776BWMhOacA2G2w/edit?usp=sharing
• I've kept more than 5 Runs for the other benchmarks

Android After

	Run 1	Run 2	Run 3	Run 4	Run 5	Run 6	Run 7	Run 8
Total	110.74sec	128.28sec	108.23sec	131.17sec	129.07sec	123.80sec	132.30sec	119.27sec
Last call ended at	16.06sec	18.05sec	15.33sec	18.16sec	17.81sec	17.49sec	18.63sec	15.72sec
Onyx#set time	6.95sec	7.93sec	6.76sec	8.06sec	7.79sec	7.58sec	8.08sec	9.46sec
Last long Onyx#get at	6.49sec	7.27sec	6.37sec	7.46sec	11.52sec	7.05sec	7.66sec	6.42sec
Get "currencyList" at	6.49sec	7.27sec	6.37sec	7.46sec	7.32sec	7.05sec	7.66sec	6.42sec

• Full data: https://docs.google.com/spreadsheets/d/1wGrVXsad6ayQZVqvyUmTuTb1cmeegdE0y-IFzhLiPKU/edit?usp=sharing

iOS Before

	Run 1	Run 2	Run 3	Run 4	Run 5	Run 6	Run 7	Run 8
Total	85.36sec	75.25sec	93.82sec	86.31sec	85.49sec	87.86sec	82.34sec	82.01sec
Last call ended at	15.06sec	14.32sec	15.24sec	15.16sec	15.34sec	15.32sec	15.24sec	14.39sec
Onyx#set time	10.08sec	7.03sec	13.32sec	10.34sec	10.23sec	11.22sec	10.63sec	8.42sec
Last long Onyx#get	12.40sec	11.72sec	4.16sec	4.15sec	4.11sec	7.50sec	4.11sec	3.55sec
Get "currencyList" at	4.35sec	3.55sec	4.34sec	4.20sec	4.30sec	4.21sec	4.29sec	3.54sec

• Full Data: https://docs.google.com/spreadsheets/d/1i8Ufi-vT58xQXsdW2_1iiNsdcpY5nupANBOL0c26HF8/edit?usp=sharing

iOS After

	Run 1	Run 2	Run 3	Run 4	Run 5	Run 6	Run 7	Run 8
Total	71.91sec	72.67sec	73.90sec	92.42sec	73.13sec	71.13sec	73.40sec	72.69sec
Last call ended at	13.86sec	13.63sec	14.02sec	17.50sec	13.54sec	13.85sec	13.65sec	13.90sec
Onyx#set time	4.52sec	4.48sec	4.78sec	6.06sec	4.54sec	4.76sec	4.50sec	4.58sec
Last long Onyx#get at	2.79sec	3.02sec	2.84sec	4.53sec	11.55sec	7.19sec	11.43sec	2.87sec
Get "currencyList" at	3.10sec	3.11sec	3.38sec	4.32sec	3.08sec	3.35sec	3.09sec	3.18sec

• Full Data: https://docs.google.com/spreadsheets/d/1XlKFbf2bDga6cHG6JCa4GDeFq4OZqUlyib3gLJ0HjYo/edit?usp=sharing

[kidroca]

Note longer total time compared to previous PR due to preferredLocale
Also a bigger report is used as my app started to init on a different report and I couldn't remember the report used on prior PRs

• see: https://expensify.slack.com/archives/C01GTK53T8Q/p1627310018264000

This PR together with the preferedLocale fixes should bring Total Time spent in Android to ~60sec.
preferedLocale takes about 40-50 on the report used in this benchmark

[quinthar]

nice!!

…

On Thu, Jul 29, 2021 at 12:27 PM Peter Velkov ***@***.***> wrote: Note longer total time compared to previous PR due to *preferredLocale* Also a bigger report is used as my app started to init on a different report and I couldn't remember the report used on prior PRs - see: https://expensify.slack.com/archives/C01GTK53T8Q/p1627310018264000 This PR together with the preferedLocale fixes should bring Total Time spent in Android to ~60sec. preferedLocale takes about 40-50 on the report used in the benchmark — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#92 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAEMNUXLEUCUCZX4ONKFFA3T2GTRXANCNFSM5BHDBZBQ> .

[thienlnam]

Changes look good, leaving final review to @marcaaron