google/agera

[Question] Usages of non main thread to register an Updatable to a BaseObservable?

LouisCAD opened this issue · 5 comments

Hi,
I know BaseObservable checks for the calling thread to be a Looper thread, but I'm wondering if there are real use cases where this thread would not be the main one since using a Looper Thread properly outside an IntentService is not trivial, and using IntentServices with Agera doesn't seem obvious to me.

If anyone using BaseObservable outside of the main thread, please reply here, and explain why.

Thanks!

I think it's highly theoretical at this point. We're not using this in our project, out of maybe ~500-1000 BaseObservable being used (mainly in the form of a Repository). The pattern is mainly; Create the instance in onCreate (activity, fragment, service etc.), add updatable in onStart and remove it in onStop. There was some ideas of services running on a different looper thread at some point, but this never happened.

Maybe the non main thread support could be dropped so the synchronized blocks could be stripped in favor of a low overhead main thread check?

We also don't want to break anyone currently using BaseObservables outside the main thread who didn't watch this repository / see this issue :)

I did a quick browse through BaseObservable and, barring one restriction, there's actually a way to remove all synchronized keywords. The one restriction is the contract of immediately throwing exception when adding a duplicate updatable, or removing a non-registered updatable, from a different looper thread than the one managing the BaseObservable. Let's keep this issue open and revisit soon.

From my tests, the quickest way to check for looper match is to cache (as a member) the thread of the looper and compare it with Thread.currentThread().

This means something like this in BaseObservable constructor:

public BaseObservable() {
    final Looper currentLooper = Looper.myLooper();
    ... // Check the looper is not null.
    mThread = currentLooper.getThread();
    ...
}

And this expression would allow to check the thread is the one that created the BaseObservable:

mThread == Thread.currentThread();

This also raises the question of whether Looper thread enforcement is needed. Just checking the current Thread in the constructor could be enough, right?