should android onTrimMemory() events trigger JavaScript garbage collection?

[jgreen210]

Android has an event that is fired if memory is low:

https://developer.android.com/reference/android/content/ComponentCallbacks2.html#onTrimMemory(int)

React native is listening to it here:

https://github.com/facebook/react-native/blob/v0.61.5/ReactAndroid/src/main/java/com/facebook/react/MemoryPressureRouter.java

Other code in react native can register to listen to the event. Currently, there is just one listener:

https://github.com/facebook/react-native/blob/v0.61.5/ReactAndroid/src/main/java/com/facebook/react/bridge/MemoryPressureListener.java

...that is registered from the CatalystInstance:

$ fgrep -ri handleMemoryPressure .
./ReactCommon/cxxreact/NativeToJsBridge.h:  void handleMemoryPressure(int pressureLevel);
./ReactCommon/cxxreact/JSExecutor.h:  virtual void handleMemoryPressure(__unused int pressureLevel) {}
./ReactCommon/cxxreact/NativeToJsBridge.cpp:void NativeToJsBridge::handleMemoryPressure(int pressureLevel) {
./ReactCommon/cxxreact/NativeToJsBridge.cpp:    executor->handleMemoryPressure(pressureLevel);
./ReactCommon/cxxreact/Instance.h:  void handleMemoryPressure(int pressureLevel);
./ReactCommon/cxxreact/Instance.cpp:void Instance::handleMemoryPressure(int pressureLevel) {
./ReactCommon/cxxreact/Instance.cpp:  nativeToJsBridge_->handleMemoryPressure(pressureLevel);
./ReactAndroid/src/main/java/com/facebook/react/MemoryPressureRouter.java:    // themselves in handleMemoryPressure()
./ReactAndroid/src/main/java/com/facebook/react/MemoryPressureRouter.java:      listener.handleMemoryPressure(level);
./ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java:  private native void jniHandleMemoryPressure(int level);
./ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java:  public void handleMemoryPressure(int level) {
./ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java:    jniHandleMemoryPressure(level);
./ReactAndroid/src/main/java/com/facebook/react/bridge/MemoryPressureListener.java:  void handleMemoryPressure(int level);
./ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.h:  void handleMemoryPressure(int pressureLevel);
./ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp:    makeNativeMethod("jniHandleMemoryPressure", CatalystInstanceImpl::handleMemoryPressure),
./ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp:void CatalystInstanceImpl::handleMemoryPressure(int pressureLevel) {
./ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp:  instance_->handleMemoryPressure(pressureLevel);

The CatalystInstance implementation is:

https://github.com/facebook/react-native/blob/v0.61.5/ReactAndroid/src/main/java/com/facebook/react/bridge/CatalystInstanceImpl.java#L506

This delegates the event via:

ReactCommon/cxxreact/Instance.cpp
ReactCommon/cxxreact/NativeToJsBridge.cpp

...finally calling:

https://github.com/facebook/react-native/blob/v0.61.5/ReactCommon/cxxreact/JSExecutor.h#L106

virtual void handleMemoryPressure(__unused int pressureLevel) {}

I don't see anything overriding this in the react-native, hermes, https://www.npmjs.com/package/jsc-android or JavaScriptCore repos, so (just from this code inspection) it looks like the onTrimMemory() event is ignored.

I think it would be useful to connect it up to JavaScript garbage collection. E.g. it would stop garbage-collectable js blob handles from holding on to BlobModule.java HashMap entries and could avoid the java OutOfMemoryErrors that are theoretically still possible even after this fix:

766f470

Hermes has a method for triggering garbage collection:

https://github.com/facebook/hermes/blob/v0.2.1/API/hermes/hermes.cpp#L489

...implementing a collectGarbage() in the jsi API:

https://github.com/facebook/react-native/blob/v0.61.5/ReactCommon/jsi/jsi/instrumentation.h#L47

Would need to find a suitable onTrimMemory() level. Would maybe need to make sure that use correct thread to trigger garbage collection.

React Native version:

react: 16.9.0 => 16.9.0
react-native: 0.61.5 => 0.61.5

[jgreen210]

Without some rate limiting, this could cause an excessive number of js collections if the java heap is full for a reason unrelated to js handles to java objects.

It seems (see comments in the above BlobModule leak issues) that JavaScriptCore has some tested heuristics for triggering js garbage collections.

[stale]

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

[stale]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.