`InMemoryNormalizedCache` is a memory bomb

[postmechanical]

Summary

The implementation does not register for or respond to UIApplicationDidReceiveMemoryWarningNotification nor does the Apollo SDK do so anywhere else in order to call clearCache. An Apollo client instance also cannot be instantiated without a cache instance and InMemoryNormalizedCache is the default. Given these issues this effectively makes InMemoryNormalizedCache a memory bomb that will result in any app using it being terminated due to memory pressure when used across a large enough data set unless a fetch policy is used that ignores cache.

Version

1.7.1

Steps to reproduce the behavior

Use InMemoryNormalizedCache

Logs

No response

Anything else?

No response

[calvincestari]

Hi @postmechanical - I don't believe this is a bug and it comes down to a difference in philosophy of responsibility.

In my opinion InMemoryNormalizedCache should not respond to UIApplicationDidReceiveMemoryWarningNotification and it certainly should not automatically clear the cache unless you configure it to. Furthermore you cannot configure it to do that because we do not offer that functionality. What we do provide is the ability for you to manage your cache data. If the documentation led you to believe we manage cache memory for you please let me know where it states that and I'll correct it.

[github-actions[bot]]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

[postmechanical]

@calvincestari That is not a philosophy that scales to thousands of developers seeking to deploy this code in performant and reliable apps to millions of end user devices. Happy to call it a documentation bug that could be mitigated by a warning here that users must implement their own memory pressure handling otherwise the default configuration will result in unbounded memory consumption.

I urge you, however, to consider a more end user, product, and customer focused philosophy. The Kotlin SDK maintainer's philosophy has resulted in an API that both anticipates the issue described here and provides a configuration based interface to manage it. It's not too dissimilar to NSCache's API, which is the standard tool for in-memory caching on Apple platforms.

Defaulting to something like a NullNormalizedCache or making the cache param optional here would also be an option if updating the documentation to warn against ubounded memory consumption in InMemoryNormalizedCache is not an option.

[calvincestari]

Hi Aaron, thanks for the feedback.

I want to clarify that we're not saying these features will never be offered in Apollo iOS. It's clear there is more work to be done on the cache and we acknowledge that; it's why you'll see cache improvements listed as an item on our roadmap. We have a vision for a configurable cache with TTL, better eviction mechanisms, and hybrid configuration. However, users should still be deciding how to handle memory warnings for the application, we don’t want to make that decision for you.

It is unfortunately not at the top of our list of priorities right now so until we get there I've updated the cache documentation with a note regarding that responsibility.

[postmechanical]

Much appreciated