Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread handoff for RealmObject and RealmResults / Frozen state #1208

Closed
cmelchior opened this issue Jun 11, 2015 · 36 comments
Closed

Thread handoff for RealmObject and RealmResults / Frozen state #1208

cmelchior opened this issue Jun 11, 2015 · 36 comments

Comments

@cmelchior
Copy link
Contributor

cmelchior commented Jun 11, 2015

It should be possible to move objects and query results between threads seamlessly. The underlying concepts introduced in #896 should be useable for this as well.

@sawcioo
Copy link

sawcioo commented Jun 22, 2015

Any progress?

@cmelchior
Copy link
Contributor Author

@sawcioo Yes, are working on #896 first which will introduce async query capabilities. Then this will follow afterwards.

@bmunkholm bmunkholm added P2 and removed P1 labels Sep 1, 2015
@cmelchior
Copy link
Contributor Author

Following the discussion on last weeks offsite-meeting. We gravitated towards the following pattern:

Foo obj = realm.where(Foo.class).findFirst();
Foo immutableCopy = obj.freeze(); Create a copy that is frozen to the current version. Any modification will throw.
new Thread(new Runnable() {
  @Override
  public void run() {
    immutableCopy.getBar(); // Will work
    obj.getBar(); // Will throw RealmException
  }
}).start();

Adding a .freeze() to RealmObject/RealmResults/RealmList will make it possible to break the normal Realm contract of always being up date. Which can make sense. Any RxJava support would then also only return frozen objects:

Observable<RealmResults<Foo>> foo = realm.where(Foo.class).findAll().observable():
foo.subscribeOn(Schedulers.io())
  .observeOn(AndroidSchedulers.mainThread()
  .subscribe(new Action1<RealmResults<Foo>>() {
            @Override
            public void call(RealmResults<Foo> results) {
                results.get(0); // Would also work without throwing Thread exception
        }
);

We need the new snapshot concept from core though to not run out of memory really really fast.

@cmelchior cmelchior added the Blocked This issue is blocked by another issue label Sep 23, 2015
@osama-lionheart
Copy link

Thanks for the updates @cmelchior!

but what exactly would be "frozen" when we call freeze? only that object fields, or any linked objects as well??

in other words, are you taking a snapshot of the entire object graph that can be linked to from the frozen object?

@cmelchior
Copy link
Contributor Author

It would give you a completely consistent view of all linked data as well. This is actually what is already happening today across threads. The freeze() method just creates the necessary plumbing internally for moving stuff across threads in a safe manner as well as having the nice benefit that it is really explicit that your data are no longer "up to date".

@jpiche
Copy link

jpiche commented Sep 23, 2015

freeze makes a lot of sense, but will there be a corresponding method to easily re-query the RealmResults for an unfrozen version? Also, would there be a method on RealmResults like isFrozen to check if a list is frozen or not?

@cmelchior
Copy link
Contributor Author

Depending on what you mean by requerying the RealmResults. You wouldn't be able to "unfreeze" a RealmResults to make it live-update again I think. That would just open up a host of potential pitfalls. Immutability should really be choice at the beginning of some event-handling, not something you can turn on/off as you wish.

Something like isFrozen() might make sense, although you should really know when interacting with an object if it is immutable or not. If you don't, you have a very high risk of having consistency issues in your app.

Do you have a particular use cases in mind? We haven't set anything set in stone yet, so any feedback on potential use cases are very welcome as that might effect how the API is designed.

Thinking about this a bit more then it would probably make more sense to not let .freeze() return a copy, but just freeze the existing object as the other approach looks like it could the be cause for potential race conditions in the code.

@jpiche
Copy link

jpiche commented Sep 23, 2015

@cmelchior I don't have a specific use case in mind. And you're absolutely right about consistency issues; my thinking was more about being about to do a sanity check.

Also, instead of having isFrozen(), .freeze() could return a type of FrozenRealmResults (or some name) which extends RealmResults. My thinking here is if you know a method will use the results on a different thread, it would provide type safety, ensuring on the compiler level that .freeze() was called.

@cmelchior
Copy link
Contributor Author

Some sort of type-safety might actually not be a bad idea. I am not sure that approach is possible with single objects as we would have to generate classes visible to the user though the annotation processor, which is a pretty painful process. It is worth a second though.

@astigsen
Copy link

astigsen commented Sep 23, 2015 via email

@craig-enquos
Copy link

Any update on if the freeze() implementation is going ahead? That type of approach would certainly make my life easier at the moment. Thanks.

@cmelchior
Copy link
Contributor Author

@craig-enquos Yes, if not with this exact API then in some way the functionality will be provided. We don't have any timeline yet though.

@cmelchior cmelchior mentioned this issue Dec 17, 2015
12 tasks
@cmelchior cmelchior changed the title Thread handoff for RealmObject and RealmResults Thread handoff for RealmObject and RealmResults / Frozen object Apr 14, 2016
@cmelchior cmelchior changed the title Thread handoff for RealmObject and RealmResults / Frozen object Thread handoff for RealmObject and RealmResults / Frozen state Apr 14, 2016
@rajuashok
Copy link

What's the status on this? Is there still no way to use subscribeOn(Schedulers.io()) with observeOn(AndroidSchedulers.mainTHread())?

@MiralDesai
Copy link

+1 would love to see this.

@Zhuinden
Copy link
Contributor

@cmelchior I do wonder though, realm results that are no longer used still update themselves on REALM_CHANGED as long as the weak reference to them still exists, right?

Frozen state would make RealmResults more CPU-friendly when the view/activity/fragment that created them no longer exists, because they wouldn't be updating a results that's no longer needed by anyone.

Just a random thought.

@beeender
Copy link
Contributor

beeender commented Jul 13, 2016

@Zhuinden

I do wonder though, realm results that are no longer used still update themselves on REALM_CHANGED as long as the weak reference to them still exists, right?

Yes!

But IMO, the easy way to make it more CPU-friendly it just call removeChangeListener(). The use of WeakReference for RealmResults is just a compromise that most users don't want to call removeChangeListener() and we still need a way to know the listeners are not needed.

@Zhuinden
Copy link
Contributor

@beeender wait a second, removeChangeListener() doesn't help this, considering the WeakReference to the RealmResults is retained by the handler controller

static <E extends RealmModel> RealmResults<E> createFromTableOrView(BaseRealm realm, TableOrView table, Class<E> clazz) {
    RealmResults<E> realmResults = new RealmResults<E>(realm, table, clazz);
    realm.handlerController.addToRealmResults(realmResults); // this line
    return realmResults;
}

@beeender
Copy link
Contributor

@Zhuinden I misunderstood your point, sorry...

Yeah, as long as they are not GCed they are still get updated when db changes. It is not difficult to supply a API to close the RealmResults (release native memory, no more updates and much easier to implement than frozen state) I am just not quite sure if people like it :)

@Zhuinden
Copy link
Contributor

@beeender Well in cases when you replace a RealmResults with a new RealmResults because your filter condition is based on input from the user (realm.where(Blah.class).contains(input).findAllSorted("blaah", Sort.ASCENDING)) then the ability to completely kill the previous RealmResults that nobody cares about is quite useful :)

@Zhuinden
Copy link
Contributor

@beeender I'm asking primarily due to the points exactly described in this issue just now #3181

  1. How many realm queries are active (not released)

B) barring GC and finalizer lag

@beeender
Copy link
Contributor

beeender commented Jul 13, 2016

  1. How many realm queries are active (not released)

Yes. I think a RealmResults.close() would be helpful on that. BTW, RealmQuery won't be updated automatically, it is just for store the query conditions. RealmResults is the one might cause extra CPU in this case. @realm/java What do you think about RealmResults.close()?

@cmelchior
Copy link
Contributor Author

I'm not inherently against it, but then again, adding an optional close method for the sake of an arguably very limited performance gain doesn't feel totally right to me. Especially not since talked about finding a way of getting rid of the on on the Realm instance. I would probably want some more evidence that it is a problem before adding it.

@Qubitium
Copy link

I want Realmresult.close(). =)

My reason is my believe that devs knows much more about context than the library and should have much control of allocation/deallocation as possible, to a point without having the library/API feel like C code code where everything is free() this and free() that. I think some closeable() interface in realm can co-exist with finalizer and future phantom queues. Best of both worlds.

@ArthurSav
Copy link

A very common case for me

getRealm().where(Venue.class)
                .findAllAsync() //5k venues
                .asObservable()
                 .map(venues -> venues.subList(0, 2000)) // a results.toList() method would be much appreciated
                .map(venues -> getRealm().copyFromRealm(venues)) //big performance hit
                .doOnNext(venues -> jumpThread(venues)) //Thread crash from realm
                .subscribe();

I would start realm from the thread i'm processing the results but it's not that easy and it messes up with my structure.

We need to be able to jump threads with the results. Although, copyFromRealm is a solution, it's has a big performance hit when you have large results.

@Zhuinden
Copy link
Contributor

Zhuinden commented Jul 22, 2016

@ArthurSav Well if you use Realm as intended

        RealmRecyclerViewAdapter adapter = //... use recycler adapter which handles change events naturally
    getRealm().where(Venue.class)
       .findAllAsync() //5k venues // use on UI thread, `async` method is already async
       .asObservable()
        //.map(venues -> venues.subList(0, 2000)) // you already get a RealmResults<T> here, just index from 0...,min(1999, venues.size()-1)
        //.map(venues -> getRealm().copyFromRealm(venues)) //big performance hit // zero copy
        .filter(RealmResults::isLoaded()) // only load Results if they're loaded
        //.doOnNext(venues -> jumpThread(venues)) //don't jump thread, call on UI thread
        .subscribe(venues -> adapter.updateData(venues));

In short

        getRealm().where(Venue.class)
                .findAllAsync()
                .asObservable()
                .filter(RealmResults::isLoaded())
                .subscribe(venues -> adapter.updateData(venues));

Then you won't have this problem. See my RecyclerView example.

@ArthurSav
Copy link

ArthurSav commented Jul 22, 2016

@Zhuinden I don't see how you example is relevant.
Venues go into into another parser which is not in the UI thread, nothing to do with recyclerview or listview.

@Zhuinden
Copy link
Contributor

Zhuinden commented Jul 22, 2016

@ArthurSav then I don't see why the query is not a synchronous query on a background thread then, directly throwing the RealmResults into the parser with map().

Don't forget, Realm is zero-copy only if you don't explicitly copy everything from it. If you rip out the entire database into memory, that's expected to take a while.

@cmelchior
Copy link
Contributor Author

With the new changes in Core6 (#6107). This feature should now be possible.

We need the following changes:

Suggested public API changes:

  • Add RealmResults.freeze().
  • Add RealmList.freeze().
  • Add RealmObject.freeze() and RealmObject.isFrozen()
  • Add RealmModel.freeze() and RealmModel.isFrozen()
  • Throw exception if trying to add a changelistener to a frozen object.
  • Transactions should throw meaningful error on frozen Realms (e.g. `results.getRealm().beginTransaction())
  • Trying to refresh a frozen Realm should throw.
  • Core already have support for tracking how many different versions are active. We should make this public and crash Realm if too many active versions exists. This will help developers catching version leaks which are now more likely to happen (and can thus cause file size explosions).
  • The RxJava implementations should return frozen objects as the default behavior. This is a breaking change.

We need to determine the semantics for results.freeze().getRealm().beginTransaction(), i.e. can a frozen object still access a mutable Realm or should the entire Realm be considered different? This implies that calling freeze on an object actually makes a copy (which probably makes a lot of sense)

Internal changes:

  • We need to disable thread checks in Java on frozen objects.
  • We need to be able to disable thread checks in Object Store for Realm/Results/List. Java currently doesn't use the OS accessors for property access, so we can probably leave this out for now.
  • We need an API in OS for creating a frozen Realm transaction. This probably means changes to to OS caching.
  • I talked to Core about caching fixed version read transactions, so instead of upper layers creating many different transactions for the same version, Core can hand out cached versions and automatically release them. This is mostly a performance optimization.

@RealmBot RealmBot closed this as completed Dec 3, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests