Memoize: Debugging stats investigations

[kacper-mikolajczak]

Problem

The cache debugging stats provided via Jason's AppInfo file, show a couple of issues related to memoization:

Actual	Desired	Action
Some of the memoized functions lack identifiers	Each function should have id to be distinguishable	Add monitoringName property to inlined functions passed to memoize
Memoized functions that have 100% cache hit ratio have avgFnTime equal to zero	avgFnTime should be equal to at least first function call time	Register first function call and add it as starting point of avgFnTime
getEmojiUnicode cache retrieval tends to be slower than actual function call	Retrieval should be faster than the actual function call	Analyze memoization for getEmojiUnicode
getLocaleDigits seems to not be used at all	The function may be underused, possibly due to a specific flow Jason performed	Reason about adding memoize stats data to analytics in order to have more, varying test samples
getUnreadReportsForUnreadIndicator retrieval/execution times are way above expected (8ms/22ms)	Retrieval/execution should be faster	Investigate the function
freezeScreenWithLazyLoading creates many unnamed empty cache entries	Cache entries should be limited to one per function	Check how we can cache it effectively and add monitoringName
getItemHeightMemoized lacks id	It should be properly referenced	Add monitoringName
cacheRetrievalTime measures the time for the cache itself but not for the entire memoize overhead	We should also track entire memoize helper overhead	Add memoizeFnTime as a new mark or change cacheRetrievalTime to be measured from function start

[kacper-mikolajczak]

After trying to follow Jason's flow and exploring different parts of the app, I've noticed:

• getLocaleDigits caches properly but it was not used in Jason's flow
• on my end, getEmojisUnicode cache retrieval was faster than the function. Let's keep it and wait until we have access to broader data
• getUnreadReportsForUnreadIndicator cache performance heavily depends on the number of reports supplied to it as a key (if there are many reports, key check will take longer). Same goes for the original function. The outcome could go either way (long retrieval process or long function) depending on the environment. I'd say let's leave the caching for now and think about a potential solution to drop caching for the function we are noticing lose in performance. This could be done in runtime as well (runtime checks slows down app so let's be extra cautious)

Cache dump

[
  {
    "id": "NumberFormatUtils",
    "stats": {
      "calls": 1813,
      "hits": 1803,
      "avgCacheRetrievalTime": 0.0023205953239618595,
      "avgFnTime": 0.06000000089406967,
      "cacheSize": 10
    }
  },
  {
    "id": "getLocaleDigits",
    "stats": {
      "calls": 183,
      "hits": 182,
      "avgCacheRetrievalTime": 0.004395604297354978,
      "avgFnTime": 0.29999999701976776,
      "cacheSize": 1
    }
  },
  {
    "id": "getEmojiUnicode",
    "stats": {
      "calls": 704,
      "hits": 685,
      "avgCacheRetrievalTime": 0.0017099554950169378,
      "avgFnTime": 0.01578947431162784,
      "cacheSize": 19
    }
  },
  {
    "id": "getUnreadReportsForUnreadIndicator",
    "stats": {
      "calls": 36,
      "hits": 24,
      "avgCacheRetrievalTime": 2.592046599053531,
      "avgFnTime": 0.6083333355685075,
      "cacheSize": 12
    }
  }
]

[kacper-mikolajczak]

Update avgFnTime calculation
Memoized functions with a 100% cache hit ratio have avgFnTime equal to zero, while it should be equal to at least the first function call time.

After investigating, the first call is registered properly.

What is happening is that monitoring was enabled after initial function call was made, so the function call timing could not be registered and all subsequent calls were intercepted by the cache, so we ended up with zero value for fnTime.

[kacper-mikolajczak]

Action points from the table status update:

1. ✅ Add debug names
2. ✅ avgFnTime (context)
3. ✅ getEmojiUnicode (context)
4. ✅ ❓ getLocaleDigits - Discuss adding memoize stats to the analytics
5. ❓ getUnreadReportsForUnreadIndicator - Can we drop caching in the runtime?
6. ✅ freezeScreenWithLazyLoading - It adds duplicates to cache but still memoizes component properly, so let's keep it
7. ✅ getItemHeightMemoized
8. ✅ cacheRetrievalTime (context)

[melvin-bot]

This issue has not been updated in over 15 days. @mountiny, @kacper-mikolajczak eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[mountiny]

@kacper-mikolajczak is this all done? can we close?

[kacper-mikolajczak]

@mountiny Affirmative! 🫡