SlowRenderListener holds references to Activity

[breedx-splk]

See this thread for additional context.

So there could be a theoretical scenario where the SlowRenderListener is unregistered from the application before it can clear its map and shut down its delegated window listeners. In theory, if the SlowRenderingInstrumentation is not uninstalled, then the reference to the SlowRenderListener would be held, and thereby the references to all the Activity instances being watched would also be held.

We all agree that unnecessarily holding references to an Activity is bad practice, and can potentially cause a heavy Activity instance to not be GC'd after use. We keep a reference to an Activity in order to get access to its Window instance, so that we can register/unregister frame metrics listeners for it.

In practice, this is not supposed to happen. There shouldn't be a case/way for the activity lifecycle listener to be unregistered, other than via uninstall(). Could it happen though? Should we guard against this?

One way to do this is with a WeakHashMap that utilizes WeakReference<Activity> keys. This is fine, but requires some additional work because the current implementation uses a ConcurrentMap for thread-memory safety and concurrent access concerns. Upstream instrumentation has an internal WeakConcurrentMap that we shouldn't use, and that's copied from a 3rd party source anyway.

Another option could be to not hold the Activity at all, but rather, its Window instance. In fact, there could be a problematic scenario right now when the Activity changes its Window, because we might later try and unregister listeners on the wrong Window! Eek. We'd still need to consider weak references for the Window maybe...but this is certainly a smaller footprint than the entire Activity.

Discussions to follow.

[LikeTheSalad]

One way to do this is with a WeakHashMap that utilizes WeakReference<Activity> keys. This is fine, but requires some additional work because the current implementation uses a ConcurrentMap for thread-memory safety and concurrent access concerns.

We could use this lib: https://github.com/raphw/weak-lock-free - It's lightweight and it was created by the author of ByteBuddy, which we already rely a lot on for other stuff in this repo, so it should be a good option for us.

[breedx-splk]

One way to do this is with a WeakHashMap that utilizes WeakReference<Activity> keys. This is fine, but requires some additional work because the current implementation uses a ConcurrentMap for thread-memory safety and concurrent access concerns.

We could use this lib: https://github.com/raphw/weak-lock-free - It's lightweight and it was created by the author of ByteBuddy, which we already rely a lot on for other stuff in this repo, so it should be a good option for us.

I think that's the same one that the upstream agent took their implementation from. :)