hermes icon indicating copy to clipboard operation
hermes copied to clipboard

Support FinalizationRegistry in Hermes

Open cyrillicw opened this issue 1 year ago • 15 comments

Problem

Is FinalizationRegistry support planned to be added to Hermes? It would be convenient to have finalizers for managing native resources in React Native, and is supported by most of modern JS engines

cyrillicw avatar Jun 19 '24 16:06 cyrillicw

It is blocked on React Native implementing an event loop that calls drainMicrotasks(). Similarly, Hermes has had an implementation of WeakRef for years, but to this day it can't be used for the same reason.

The RN folks are working on a new event loop. Once that lands and become standards, we can implement FinalizationRegistry.

tmikov avatar Jun 19 '24 18:06 tmikov

@tmikov Thank you for your answer! According to this issue https://github.com/facebook/react-native/issues/42742 they are going to support the new event loop and WeakRefs in the next React Native version 0.75. Can we expect that after that FinalizationRegistry might be implemented?

cyrillicw avatar Jun 20 '24 07:06 cyrillicw

Thankfully FinalizationRegistry can be simulated using WeakRef.

Here is an implementation which is working on RN 0.75 on New Architecture:

https://gist.github.com/cray0000/abecb1ca71fd28a1d8efff2be9e0f6c5

It tries to cleanup stuff every 10 seconds.

cray0000 avatar Sep 20 '24 13:09 cray0000

@cray0000 your implementation seems good to me, though obviously it wouldn't scale very well with many objects. One minor nit: what happens if finalize() throws an exception?

FWIW, now that the new architecture will be the default, we are discussing implementing FinalizationRegistry natively.

tmikov avatar Sep 20 '24 19:09 tmikov

@tmikov good point.

I did initially wrap its call into setTimeout(0) as a hack to just ignore any sync errors during finalize. But then it became harder to debug user-space code when there are multiple finalize`s which have a similar error in them and they all fire roughly at the same time.

And I didn't research the actual FinalizationRegistry standard to understand what the correct behavior is in such situations so I simplified the control flow and just did let it crash synchronously.

But if you think that adding the FinalizationRegistry to Hermes and having RN actually turn it on will take a long time (half a year or more?) then I guess it makes sense to publish this as a library and make it a full-featured mock implementation which follows the standard.

I guess my main idea is that it's finally possible to implement an easy to use cache structures in the standard RN and Expo projects. And I just threw this in for the fellow state management libraries authors as a reference implementation - that the main thing which we needed is already there - it's the WeakRef. And the FinalizationRegistry does not actually matter that much - the destructor logic can be ran by simply checking if the WeakRef is alive.

Having a native implementation is waaaay better of course. It would be a huge win for the RN libraries ecosystem if you could make it happen. Please 🙏

cray0000 avatar Sep 20 '24 21:09 cray0000

@cray0000 I wonder whether as a stopgap we could ship this implementation as a built-in polyfill?

tmikov avatar Sep 20 '24 21:09 tmikov

@tmikov well, there is no point in asking me, I'd definitely say YES since I heavily depend on having an easy to use destructors for caching :)

And I remember from reading the WeakRef issue on RN repo that Apollo and/or Prisma also really wanted the WeakRef/FinalizationRegistry for their caching system too.

I think that even if it's a user-space implementation, sub-optimal from the performance perspective, having it available is still better than not having it available. I'd guess that for the library authors having a consistent APIs across web and RN and no memory leaks is probably a good trade to make for some performance drop compared to an ideal native implementation.

cray0000 avatar Sep 20 '24 21:09 cray0000

Cool! I think we will do that as the first step. Meanwhile we will work on a "proper" native implementation. Which one gets into the next release is a matter of timing, but at least one certainly will.

tmikov avatar Sep 20 '24 22:09 tmikov

My team would love to see native support for FinalizationRegistry as well. We provide client libraries to customers and we have prominent customers that are shipping applications with ReactNative.

In some scenarios, the users of our library are expected to manage a large collection of objects and to control the lifetime of those objects. If they release those objects consistently, then it helps to keep the overall memory usage of our system lower over time. Without something like FinalizationRegistry, our only recourse is to expose a dispose() method to them and cross our fingers that they use it. And for some of our objects, a dispose() method is not even possible and we must resort to a top-level dispose(object) function instead (which is even less discoverable). All that is to say - we are having to choose between "better performance" and "better API" for our users. FinalizationRegistry would allow users to skip the manual disposal and still keep our memory consumption low, on average.

@tmikov - Would you be able to provide an estimate - either now or when you've planned it - as to when this work might be completed? It will help us decide whether we can wait for your implementation of FinalizationRegistry to drop (either the polyfill or the real deal), or if we should begin by building our own WeakRef-based solution first as a stopgap. ReactNative is perhaps the last of our supported environments that does not implement FinalizationRegistry, so we're very excited that you are working on it. Thank you!

@cray0000 - Your WeakRef-based implementation of FinalizationRegistry is a clever solution, nice work.

noencke avatar Oct 08 '24 19:10 noencke

@noencke planning to add a polyfill to Hermes as soon as I have some time.

tmikov avatar Oct 10 '24 03:10 tmikov

Doesn't FinalizationRegistry already work in RN 76? :)

mrousavy avatar Dec 06 '24 17:12 mrousavy

At least I use it in react-native-mmkv and I think it works, lol.

mrousavy avatar Dec 06 '24 17:12 mrousavy

@mrousavy this is quite surprising to me. Where did it come from? How is it implemented? A polyfill like the one we discussed is certainly possible, but it is not very efficient.

Are you sure that you are not executing the fallback here: https://github.com/mrousavy/react-native-mmkv/blob/02a00aeeb1931deba457ad87e4b9adf7cd1c1d21/package/src/MemoryWarningListener.ts#L23-L27

tmikov avatar Dec 07 '24 02:12 tmikov

@tmikov @mrousavy

Did hermes get a usable weakref in a recent release?

spendres avatar Feb 13 '25 01:02 spendres

@spendres Hermes has had a usable weakref for years, however it needs to be enabled by RN. I believe it is enabled in new-architecture apps.

tmikov avatar Feb 13 '25 03:02 tmikov

@tmikov we just hit a serious issue in Skia where Skia destructors are invoked on a Hades thread, which causes an immediate crash because currently in Skia each thread has its own GPU context so you try to destroy a GPU resource in the wrong context. FinalizationRegistry would be a solution to that problem for us.

wcandillon avatar Sep 14 '25 08:09 wcandillon

@wcandillon you are right - FinalizationRegistry is a clean solution to this, albeit a little heavyweight.

For now I recommend the approach I described here: https://github.com/facebook/hermes/issues/1780#issuecomment-3289847609: an explicit queue drained in a microtask handler.

tmikov avatar Sep 14 '25 20:09 tmikov