Standalone objects support #221
Conversation
| @@ -148,6 +148,23 @@ public object CreateObject(Type objectType) | |||
| return result; | |||
| } | |||
|
|
|||
| public void Add<T>(T obj) where T : RealmObject | |||
There was a problem hiding this comment.
Needs to check if the object is already managed and throw an exception.
| /// 8: ldfld <backingField> | ||
| /// 9: ret | ||
| /// This is roughly equivalent to: | ||
| /// if (!base.IsManaged) return this.<backingField>; |
There was a problem hiding this comment.
Hmmm, I"m not so sure about this as a philosophy because we're supposed to be moving towards optimising the data access and this is adding another couple of conditionals on every single property read and write.
I will think about it more but this is certainly good enough for the MVP and very cleanly written!
There was a problem hiding this comment.
Yeah. The alternatives, I guess, are:
- we update the accessor with runtime code gen
- we make the accessors indirect, either by introducing an interface or with delegates.
#1 would be optimal and maybe that's what we should do when we can but it's obviously not an option on iOS.
As for #2, I guess we should test it but my gut feeling is that an indirect call is more or less as expensive as one easily predicted branch. But I don't have any data to back that up.
There was a problem hiding this comment.
Remember we've already had discussions and planned issues about collapsing the calls further by weaving in code to directly call the wrapper code, rather than going through RealmObject.SetValue so there's a lot more to be done in this field. That's another reason I'm willing to let it go for now - we have much bigger improvements to make.
There was a problem hiding this comment.
While I see the value in splitting GetValue/SetValue into specialized methods based on types so as to avoid the if/else cascade, I'm not sure whether further manual inlining is needed. The JIT should be smart enough to inline the check in get_IsManaged and the other thing is a branch.
I guess I can check how Xamarin's AOT will handle this, but from my experience with LLVM, indirectly calling a delegate is going to be slower than the current approach. Then again, the entirety of Cocoa is indirect calls, and that's certainly fast enough.
However, I think that we won't be able to drive down the number of operations in accessors to an absolute minimum, because at some point at least the setters would have to contain some logic that implements INotifyPropertyChanged and INotifyDataErrorInfo.
There was a problem hiding this comment.
Your point about INotifyPropertyChanged is one reason we've not proceeded any further with the collapsing of the stack - we need to get a lot more real-world use including people wanting notifications and more reactive apps.
|
Er, what should happen when a standalone object is added to a managed object via a relationship? Should I try to handle that case as well? |
| @@ -65,7 +65,7 @@ | |||
| </ItemGroup> | |||
| <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" /> | |||
| <PropertyGroup> | |||
| <PostBuildEvent Condition="'Windows_NT'=='$(os)'">xcopy/Y $(TargetPath) $(SolutionDir)Tools</PostBuildEvent> | |||
| <PostBuildEvent Condition="'Unix'=='$(os)'">cp $(TargetPath) $(SolutionDir)Tools</PostBuildEvent> | |||
| <PostBuildEvent Condition="'Windows_NT'=='$(os)'">xcopy /Y "$(TargetPath)" "$(SolutionDir)Tools"</PostBuildEvent> | |||
There was a problem hiding this comment.
I have a strong suspicion that I've been burned by conditional PostBuildEvents and Xamarin Studio before. Either the condition didn't work or it was truncated by using Xamarin Studio.
Please look at the <Copy> elements by opening the csproj in a text editor - they are trivial to copy and we know they are robust between the two IDEs.
…m-dotnet into yg-standalone-objects # By Thomas Goyne (46) and others # Via Ari Lazier (7) and others * 'yg-standalone-objects' of https://github.com/realm/realm-dotnet: (91 commits) Implicitly attach standalone objects to a realm when adding to a relationship Replace the conditional PostBuildEvents that copy the Weaver with a Copy task Rename Add to Attach Fix woven test as per code review, so we test if the destination classes in a `RealmList` are Fixed iteration and added test TimHasTwoIterableDogs Polishing final bits of RealmList clearing and enumeration #110 implementing relationships - finishing lists (toMany relationships) LinkList relationships working to be able to Add using a SharedLinkViewRef to hang on to them Adding related objects - complete the Add stub - fix bug in wrappers/table_cs.cpp where thought should return null for empty lists (NO) - uncomment test Setup that tries to add a related object #110 implementing relationships - finishing lists (toMany relationships) Declaration and creation working, checking size for empty relationship working Eliminate a query copy in Results::filter() Use make_optional() Fix typo in comment Expose the object class name from Results Switch to throwing exceptions with all of the info needed to build error messages Fix compilation with GCC 4.9 Port most of RLMResults's functionality to realm::Results Add results.{hpp,cpp} from realm-js Move Realm::Config's default constructor out-of-line Minor formatting changes and remove checking FullName.StartsWith, in response to PR ...
|
👍 now that I fixed one trivial error building against NUnitLite |
…m-dotnet into yg-standalone-objects
|
@AndyDentFree please take a look at 01f44dd - I had to make some changes to make the relationship stuff run on my machine. I'm not 100% sure about the changes I made, but it seems to work now (tests pass). |
|
Alright, I've gotten rid of all the Actually I'd rather not have to touch the wrappers build process in this PR, or even change in generic constraints of |
|
I'm going to merge this just to land the feature, but it'll probably be a good idea to look into the wrappers build and fix that properly separately. |
Standalone objects support
Woven properties on RealmObject classes can use their backing fields if their instances aren't attached to a realm.
Adding an object to a realm will copy the values of its backing fields to the row in the table.