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

Adds Realm.copyFromRealm() #1849

Merged
merged 1 commit into from Dec 3, 2015
Merged

Adds Realm.copyFromRealm() #1849

merged 1 commit into from Dec 3, 2015

Conversation

cmelchior
Copy link
Contributor

Fixes #931

Adds 4 new methods that makes it possible to make detached deep copies of managed RealmObjects:

RealmObject objectCopy = Realm.copyFromRealm(RealmObject object);
RealmObject objectCopy = Realm.copyFromRealm(RealmObject object, int maxDepth);
List<RealmObject> listCopy = Realm.copyFromRealm(Iterable<RealmObject> objects);
List<RealmObject> listCopy = Realm.copyFromRealm(Iterable<RealmObject> objects, int maxDepth);

As discussed this breaks Realms zero-copy and consistency guarantees, but enables architectural patterns currently not supported by Realm.

@cmelchior
Copy link
Contributor Author

@realm/java

@saket
Copy link

saket commented Nov 27, 2015

Interesting! How does maxDepth work here?

@cmelchior cmelchior mentioned this pull request Nov 27, 2015
12 tasks
@@ -31,8 +31,11 @@
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.util.Types;
import javax.rmi.CORBA.Util;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this import seems to be unused

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ups, will fix

@cmelchior
Copy link
Contributor Author

It is there to prevent copying to much data. Realm is an Object database so you might construct your data model in such a way that you can access the entire graph. This could be potentially very memory intensive to copy, so being able to break the graph makes this scenario easier to control so you don't run out of memory.

See e.g this unit test: https://github.com/realm/realm-java/pull/1849/files#diff-b11f4583c757aa2c5f52d4fb9e4ac750R2348

@saket
Copy link

saket commented Nov 27, 2015

Looks good. Thank you! I've been cloning into raw-objects manually in my code.

@zaki50
Copy link
Contributor

zaki50 commented Nov 28, 2015

Current implementation searches and duplicates the object graph by depth-first search.
This causes reconstruction of sub graph if the same object appears more than once and depth is smaller than before.

To avoid this reconstruction, we can use the breadth-first search since the depth monotonically increases.

This is just an optimization and might not be needed at first release.

@cmelchior
Copy link
Contributor Author

Yes, it is depth first currently. Mostly because it was easier to implement and also because I think this feature will mostly be used on leaf nodes, which means we won't have to repopulate objects often (pure speculation though).

So as far as I can see we have the following trade-offs:

Breath-first

  • + Don't have to repopulate cached objects that are encountered at lower depths
  • - Will always have to maintain an extra queue that consists of a tuple of (realmObject, standalone) for all references at each depth.

Depth-first:

  • + No need for extra datastrutucures = less memory pressure.
  • - Will have to repopulate objects encountered at lower depths.

Given that I wouldn't anticipate this being used on either many or deep objects i opted for simplicity, but I'm happy to hear arguments for changing it?

@@ -1,3 +1,6 @@
0.87.0
* Added Realm.copyFromRealm() for creating detached copies of Realm data.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"data" -> "objects"

@beeender
Copy link
Contributor

beeender commented Dec 1, 2015

Nice! 👍

@zaki50
Copy link
Contributor

zaki50 commented Dec 1, 2015

Depth-first is OK as first release. We should keep simple if possible.
And if we will find the use-case which requires Breath-first, we can introduce Breath-first implementation at any time.

👍

@kneth
Copy link
Member

kneth commented Dec 1, 2015

Even though I do have reservations about diverging from the zero-copy principle I think it looks good.

cmelchior added a commit that referenced this pull request Dec 3, 2015
@cmelchior cmelchior merged commit 3eb80dd into master Dec 3, 2015
@cmelchior cmelchior removed the Review label Dec 3, 2015
@cmelchior cmelchior deleted the cm/copyfromrealm branch December 3, 2015 15:46
@saket
Copy link

saket commented Dec 3, 2015

Is it possible to start using 0.87 so that I can use this feature?

@cmelchior
Copy link
Contributor Author

Yes, you can use our SNAPSHOT releases : https://github.com/realm/realm-java#using-snapshots

@saket
Copy link

saket commented Dec 3, 2015

Ah yes, thanks!

@borkasm
Copy link

borkasm commented Apr 29, 2016

I wonder why this functionality maintained only in Java. Just run into the situation where it would be nice to have it in Swift.

@cmelchior
Copy link
Contributor Author

cmelchior commented Apr 29, 2016

The Android and Swift communities have different paradigms, patterns and libraries that are commonly used. So while we strive to have feature parity between Java / Swift / Objective-C there will always be small differences.

If it is a feature you would like implemented I would suggest raising it as an issue for Realm Swift. They already have an issue tracking it here: realm/realm-swift#3381

@borkasm
Copy link

borkasm commented Apr 29, 2016

Thanks, I think I will find a simple workaround this time.

@zhihuitang
Copy link

thanks @cmelchior, nice job

@othreecodes
Copy link

Awesome!!!! thank you for this!

@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants