Rewrite the Cloning System

[AdamsLair]

As stated in issue #103, there is currently no easy way to customize cloning behavior other than taking over completely, either by overriding OnCopyTo (Components, Resources), or by implementing ICloneExplicit (other classes).

• The apparent problem is the default behavior of explicitly unwrapping only ICollection types whenever cloning Duality classes.
• The underlying problem to solve is how the CloneProvider determines which references of an object are actually references, and which refer to objects that are to be considered dependent parts of the current object.
• Introduce a new attribute that identifies a class or a field as dependent / non-autonomous object.
  • Whenever that attribute is active, the affected objects are cloned deeply instead of assigning their reference only. This is not overridable by explicit unwrapping behavior.
  • If it's applied to a class, all occurrences of it will be affected.
  • If it's applied to a field, the stored object will be affected.
  • Special case: If that stored object is an ICollection, all of it's elements will be affected.
    • This does NOT mean all of the collections fields will be affected, just the objects that are to be considered elements of the collection, i.e. objects with collection.Contains(obj) being equal to true.
    • The same holds true for collections of collections. And what about dictionaries and other key-value collections? That sounds messy.
    • Also, calling Contains is not a good idea, as it might be a "non-const" operation for some collections.
    • Better idea: Pass the effect of that field on as long as there is an unbroken chain of deeply-cloned objects from the current object to the one that was directly affected by the field attribute.

[AdamsLair]

As soon as the above attributes are in place and working, replace some of the internal implementations of ICloneExplicit with using the attribute.

[AdamsLair]

• Right now, all possibilities for the programmer to interact with cloning are from within Components and Resources. In both cases, explicit unwrapping is active and set to ICollection.
  • Maybe make this the default behavior of CloneProvider in general, to be more consistent?
  • Remove the concept / property of "explicit unwrapping" entirely from the API?
    • Nope. That would make the CloneProvider pretty useless, provided it would just assign pretty much every class instead of deepcloning.
  • Alternative: Make the root object special in that it will always be deep-cloned itself, then rely on the internal unwrapping rules (ICollection) and the above mentioned Attribute for non-autonomous objects or fields.
    • That Attribute could be extended to a general "CloneBehavior" attribute, which could specify "Clone", "Assign" or "Auto", which would be the default being equal to not having such attribute.
    • Beware: Runtime costs! Performance for cloning will decrease, if every field involves an attribute check..
• Re-Evaluate this whole Cloning topic later. The current system isn't awesome, but it's running and sufficient. Know what they say about running systems.

[AdamsLair]

• As it turns out, the current Cloning system doesn't actually work in general, but only in limited scope. See here
  1. A Scene is cloned.
  2. A Component references a GameObject that wasn't cloned yet. It will be reference-assigned, but that reference will point to the old Scene.
  3. Quick fix does a prepass in the Scene's clone method to deal with this at least for GameObject and Component referenced. Need to actually fix this at its root later.
• Cloning needs to be a two-pass process:
  1. Traverse the object graph and determine which objects need to be cloned.
  2. Traverse it again and use the information to decide which objects to clone, and which to reference.
• The system will need to be re-evaluated and probably rewritten.

[AdamsLair]

Some general thoughts on this before diving right in:

• As described, Cloning needs to be a two-step process, and it needs to be context-based. This means that the result varies based on which object is selected as the root node.
• In the first step, it will be determined which objects are to be cloned, i.e. which objects actually belong to the root in a sense of ownership. Clone instances of the to-be-cloned objects can be created directly when suitable.
• In the second step, each of the original objects will be copied to each of the clone instances.
• The crucial part will be to determine ownership of an object.
  • In general, full ownership should be assumed as this is a safe assumption for unknown objects.
  • Checks need to be performed in order to determine the (not-so) special case of an object actually being just referenced, not owned.
  • Certain objects usually exist independently. Introduce a [CloneBehavior(XY)] class attribute to make referencing the default way of handling them. Allow an assembly-level version of this attribute in order to allow configuring third-party Types.
    • GameObjects, Components, Resources, ...
  • There are always special cases. Allow applying the [CloneBehavior] attribute to fields as well to override behavior locally. Allow an optional Type parameter in case the target is actually a nested object, or one stored within a collection.
    • A Scene owns its GameObjects.
    • A GameObject owns its Components.
  • Also, some fields shouldn't be cloned at all, so there should be an equivalent to [NonSerialized] on the cloning side as well.

[AdamsLair]

Status Report: Created a "cloning" branch for this issue and began rewriting the whole Cloning system using the concept described above. Satisfied all existing Unit Tests, wrote some more and satisfied them too.

ToDo:

• Write more Unit Tests, because they clearly don't cover everything. Cloning can't be done already, no way.
• (Re-)Implement ICloneExplicit similar to ISerializeExplicit and using specialized interfaces instead of passing the CloneProvider itself.
• (Re-)Implement ICloneSurrogate similar to ISerializeSurrogate.
• Make sure all the existing surrogates work.
• Fix all the "TODO CLONING" pragma warnings.
• Remove all Cloning hacks that were previously introduced due to the problems with the old system.
• If possible, get rid of OnCopyTo completely and rely entirely on attributes. Users can still implement ICloneExplicit.
• Run performance tests and improve wherever possible.

[AdamsLair]

Status Report: Wrote a lot of Unit Tests, added support for the new ICloneExplicit interface, added support for Skip- and Identity-Fields, removed a lot of old code and fixed pragma warnings.

I've also run some first performance measurements, yielding that the new system is approximately 1.42 times slower than the old one. This definitely needs to change. The goal is to get it working at the same speed.

ToDo:

• (Re-)Implement ICloneSurrogate similar to ISerializeSurrogate.
  • Make sure all the existing surrogates work.
• Get the old ICloneExplicit use cases to work (Resources and Components)
  • However, if possible, get rid of OnCopyTo completely and rely entirely on attributes. Users can still implement ICloneExplicit when needed.
  • Ideally, it will work as good as Serialization, where custom implementations are rarely necessary.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible.
  • Clone a lot of objects. Also, bring up this test Scene by @BraveSirAndrew again and see how it's doing.
• Improve performance of the new system.
  • Cache static cloning information similar to SerializeType in ReflectionHelper
    • Fields to clone (skipped ones removed) and their flags
    • Can it be cloned by assignment?
  • Some classes can't be cloned by assignment, but all of their (non-skipped) Fields can.
    • Test whether this assumption is true. If yes, proceed.
    • Compile an Expression that assigns all their fields and use it to handle their CopyTo.

[AdamsLair]

Side Note: Investigate whether ReflectionHelper.CreateInstanceOf can be improved.

• 90% of the time, it is invoked as type.CreateInstance() ?? type.CreateInstance(true).
  • See if that can be reasonably made 100%, then replace the actual implementation with that behavior, because it would've made sense like that in the first place.
• Replace with a cached Delegate approach, using Throw-Catch only upon creation of the delegate.

[AdamsLair]

Status Report: Implemented surrogates and did some refactoring on ReflectionHelper. DelegateSurrogates are now actually much better than before, as they are treated like weak references, so object A doesn't pass on all its event subscriptions to its clone, if they happen to refer to objects outside the cloned object graph. Performance has been improved slightly.

ToDo:

• Get the old ICloneExplicit use cases to work (Resources and Components)
  • However, if possible, get rid of OnCopyTo completely and rely entirely on attributes. Users can still implement ICloneExplicit when needed.
  • Ideally, it will work as good as Serialization, where custom implementations are rarely necessary.
  • An easy way to transition to the new system would be to still implement OnCopyTo, but call AutoHandleObject from the base class. That way, a regular OnCopyTo implementation would only need to call custom post-cloning initialization stuff while all the basic cloning has already taken place.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible.
  • Clone a lot of objects. Also, bring up this test Scene by @BraveSirAndrew again and see how it's doing.
• Improve performance of the new system.
  • Cache static cloning information similar to SerializeType in ReflectionHelper
    • Fields to clone (skipped ones removed) and their flags
    • Can it be cloned by assignment?
  • Some classes can't be cloned by assignment, but all of their (non-skipped) Fields can.
    • Test whether this assumption is true. If yes, proceed.
    • Use object.MemberwiseClone to initialize their instances? Test performance!
    • Compile an Expression that assigns all their fields and use it to handle their CopyTo? Test performance!
  • Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well.

[AdamsLair]

Status Report: Implemented all old ICloneExplicit cases in first iteration. Did a lot of performance improvements - the complex object graph test case went down from 480ms to 130ms, which may actually be more than twice as fast as the old system. Which would be pretty awesome, if proven true in real-world use cases.

ToDo:

• Get the old ICloneExplicit use cases to work (Resources and Components)
  • Transform expects AddToGameObject to be called when cloning (which it isn't anymore). Either change cloning behavior so that it can do without AddToGameObject, or do explicit cloning for GameObject in order to provide the event.
  • Fix the StackOverflow crash related to UndoRedo: Create a new Transform object, clone it and parent the clone to the original. Undo everything. Redo everything. Crash on parenting action.
  • Keep eyes open for other stuff that is broken.
  • Don't forget to write a unit test for each broken case.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible.
  • Clone a lot of objects. Also, bring up this test Scene by @BraveSirAndrew again and see how it's doing.
• Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well. Maybe do a side-by-side test when cloning a Scene.

[AdamsLair]

Status Report: Fixed a lot of bugs and wrote tests for them to make sure they don't come back. Not done yet, it's still in the "crash and burn" phase from a productive standpoint. At least Prefabs don't work yet, and there are probably more bugs that haven't been discovered.

On the plus side, most of these bugs stem from the fact that the old system was a hacky bunch of special case handling, and now those special cases are gone - but some parts of the code still rely on it. So yes, the Duality world is burning right now, but it's a cleansing fire.

( In case I just scared you away from pulling updates, it all happens in the cloning branch. The master branch remains unaffected. )

ToDo:

• Keep eyes open for broken stuff and fix it.
  • Fix PrefabLink.Apply replacing itself in the GameObject with null due to cloning the PrefabLink as well. Can't skip it, because cloning an object should also clone the PrefabLink. It just needs to be skipped in this specific case - or manually reapplied in Prefab.CopyTo(GameObject)
  • Test this again and write a Unit Test for it.
  • Keep an eye on Andrews test Scene.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible. Also test updating Prefabs and Changelists.
  • Clone a lot of objects. RigidBodies and stuff.
• Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well. Maybe do a side-by-side test when cloning a Scene.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Fixed some more bugs in the cloning system and modified the API and internal routines to allow reusing existing objects in a CopyDataTo operation. This is vital in order to apply Prefabs to existing objects, since existing object hierarchies and Components will need to be used without re-instantiation.

Also found an issue with copying data to existing GameObjets: Right now, due to the fully automated GameObject cloning, no events will be fired, but this is necessary in those cases. Need to fix this using a manual ICloneExplicit implementation in GameObjects. Make sure this implementation works seamlessly with AutoHandleObject calls and doesn't look too bad.

ToDo:

• Keep eyes open for broken stuff and fix it.
  • Make sure that copying data to existing GameObjects will trigger all required events.
    • Added / Removed Components and child GameObjects
    • A custom ICloneExplicit implementation might easily solve this by using the proper API in the setup step.
  • Test this again and write a Unit Test for it.
  • Keep an eye on Andrews test Scene.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible. Also test updating Prefabs and Changelists.
  • Clone a lot of objects. RigidBodies and stuff.
• Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well. Maybe do a side-by-side test when cloning a Scene.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Fixed the GameObject issue by providing an ICloneExplicit implementation. Unlike the one from the old system, it will also remove Components and child objects when necessary, so that's kind of a free fix +1 along the way. Also improved the general ICloneSetup and ICloneOperation API to be more convenient and straightforward. All Unit Tests end successful now.

ToDo:

• Keep eyes open for broken stuff and fix it.
  • When performing CopyTo on Delegates, a special additive behavior is required because of the conceptual ownership inversion: If I register an event in object A and then copy object B onto it, I wouldn't expect my event registration to be removed, because the subscription happened outside the bounds of objects A conceptual ownership. Thus, Delegates need to be additive during CopyTo operations.
  • Since GameObject CopyTo is now able to remove Components and child objects, see whether Prefab instances can still carry additional children and Components. They likely cannot, which needs to be fixed.
  • Test this again and write a Unit Test for it.
  • Keep an eye on Andrews test Scene.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible. Also test updating Prefabs and Changelists.
  • Clone a lot of objects. RigidBodies and stuff.
• Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well. Maybe do a side-by-side test when cloning a Scene.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Resolved a Delegates ownership inversion and introduced "merge surrogates" to do it properly. When copying a delegate field from a source object to an existing target object, the target graph will keep all subscriptions it doesn't own and add target versions of all subscriptions owned by the source graph.

void Foo(Bar source, Bar target, Bar other)
{
    source.SomeEvent += source.Child.ReceiveEventA;
    source.SomeEvent += other.Child.ReceiveEventA;
    source.SomeEvent += StaticReceiveEventA;

    target.SomeEvent += target.Child.ReceiveEventB;
    target.SomeEvent += other.Child.ReceiveEventB;
    target.SomeEvent += StaticReceiveEventB;

    source.DeepCopyTo(target);

    target.FireEvent();
}

In the above code snippet, firing the target event will trigger the following receivers:

• target.Child.ReceiveEventA: Because the direct childs ReceiveEventA was part of the source graph.
• other.Child.ReceiveEventB: Because the subscription it is not part of the target graph and thus isn't overwritten.
• StaticReceiveEventB: Because static subscriptions are never owned.

The target event will not trigger the following receivers:

• source.Child.ReceiveEventA: Because source is mapped to target, this is transformed into the above target.Child.ReceiveEventA.
• target.Child.ReceiveEventB: Because this is a part of the local target graph, it will be overwritten.
• other.Child.ReceiveEventA: Because the subscription it is not part of the source graph and thus isn't copied.
• StaticReceiveEventA: Because static subscriptions are never owned.

It may look complicated at first, but it makes sense when you think about it. Object usually subscribe and unsubscribe themselves to events. Of the events owner is copied, you wouldn't expect to be suddenly subscribed to the copy as well.

ToDo:

• Write proper Prefab Unit Tests.
• Keep eyes open for broken stuff and fix it.
  • Since GameObject CopyTo is now able to remove Components and child objects, see whether Prefab instances can still carry additional children and Components. They likely cannot, which needs to be fixed.
  • Test this again and write a Unit Test for it.
  • Keep an eye on Andrews test Scene.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible. Also test updating Prefabs and Changelists.
  • Clone a lot of objects. RigidBodies and stuff.
• Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well. Maybe do a side-by-side test when cloning a Scene.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Wrote a lot of Prefab unit tests, and some more to check for specific issues. Prefabs now allow GameObjects to carry additional Components and children again.

ToDo:

• Keep eyes open for broken stuff and fix it.
• Do a lot of tests in the actual engine.
  • Editor UndoRedo is great, as it clones stuff all the time.
  • Prefab Support. Make it as complicated and nested as possible. Also test updating Prefabs and Changelists.
  • Clone a lot of objects. RigidBodies with joints and stuff.
• Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well. Maybe do a side-by-side test when cloning a Scene.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Did a lot of testing, fixed bugs and wrote unit tests. Manually tested nested Prefabs and complex RigidBody setups being cloned, all successful without further ado. So far, no issues with UndoRedo occurred. Due to the positive test results, I'm considering to merge the cloning branch into the main branch soon, but I might wait a while until distributing new NuGet packages.

ToDo:

• Keep eyes open for broken stuff and fix it.
• Test cloning performance with "real world" use cases of instantiating a complex Prefab or cloning a whole Scene as well. Maybe do a side-by-side test when cloning a Scene.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Did a side-by-side performance comparison cloning a complex Scene in the old system vs. the new one. Am horrified by the results, because the new one takes 15x the time of the old one. This needs to change before considering a merge.

ToDo:

• Fix cloning performance in real-world use cases.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Improved performance, so the new system is now at 2x the time of the old one in a real-world use case. Still not good enough. Need to improve this.

ToDo:

• Fix cloning performance in real-world use cases.
  • Make sure there is no target mapping for value types, this only uses space and lookup time. Will need to modify GetTarget callsites to handle value types properly. Right now, there are ~1000 ValueType objects mapped from a total of ~5000 in the test case. Stop that.
  • Keep an eye on ContentRef classes. They could in theory be skipped in the graph setup step and only need the copy step for Resource instance mapping. Make sure this is optimized.
    • Actually, a general optimization is possible on all fields with a directly applied local reference behavior, because this overrides all other settings and is directly applied to the field itself. This field will always be just a reference.
  • See what else there is to improve.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Removed source-target mapping from CloneProvider for value types and cleaned up some code.

ToDo:

• Fix cloning performance in real-world use cases.
  • Keep an eye on ContentRef classes. They could in theory be skipped in the graph setup step and only need the copy step for Resource instance mapping. Make sure this is optimized.
    • Actually, a general optimization is possible on all fields with a directly applied local reference behavior, because this overrides all other settings and is directly applied to the field itself. This field will always be just a reference.
  • See what else there is to improve.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Optimized ContentRef behavior without notable performance impact. Failed in various other ways to improve performance any further.

ToDo:

• Fix cloning performance in real-world use cases.
  • Investigate whether more precompiled cloning code can solve the problem.
  • Investigate whether manual cloning implementations for Duality classes can solve the problem.
  • See what else there is to improve.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Added manual cloning implementations for widely used Duality Components in order to improve performance. The CloneProvider now also uses precompiled functions to traverse an objects fields in the setup step. The new system still takes 1.5x the time, which is kind of okay, given the fact that it actually works in a lot more cases than the old one.

Can get down to about 1.2x when treating ContentRefs as plain old data, but that would break all of them when they refer to a temporary Resource within the cloned object graph, like the old system would have.

Comparing performance from the old system to the new one is kind of unfair though, because the old system didn't actually work in a lot of cases. I guess 1.5x is fine - it can still clone 25000 simple GameObjects and their 1-3 Components in under 250ms on my test system. That's actually cloning 1600 objects per frame and still having 60 FPS, which would be an insane thing to do anyway.Looking at more realistic use cases:

• Clever SavePoints without stopping the game can be done by separating a Scenes objects into static and dynamic objects. When loading the Scene, create a copy of its static objects and keep the CloneProvider and its cache. When hitting the savepoint, create a copy of the whole Scene or only its dynamic objects and rely on the already existign Cache to skip static objects and correctly map references to them. Then serialize the savepoint in a separate thread.
• Spawning a lot of objects. Instantiating a "regular" Prefab takes about 0.08 - 0.15 ms, depending on its complexity - which is ~9x the time it takes to create the same object manually from source code. This is a lot, but low enough to get away with, when not spawning huge amounts of objects at once.
• Applying Prefabs. This is a design-time and load-time only use case, so it's probably irrelevant. Doesn't take much longer than spawning the same objects anyway.

Also fixed some bugs and tested some more use cases. The new system seems to work fine so far. Getting ready to merge.

ToDo:

• Write a unit test for checking whether cloning two joint RigidBodies still works.
• Extend the CloneProvider to allow keeping its cache for the "Clever SavePoints" use case, and write a test for it.
• Maybe there is potential in splitting up the current behemoth methods into object handling and valuetype handling. The latter one would be generic and could make use of a lot of optimizations, because of a lot of nice assumptions on valuetypes.
  • Exposing both variants via the interface would allow precompiled methods and explicit implementations to invoke the correct version directly without any casting and boxing.
  • There needs to be an internal "unboxing" glue-method to handle already boxed valuetypes, cast them into their actual type and call the correct generic value handling method. Needs to be done in another precompiled method in the valuetypes CloneType.
  • Maybe add some additional precompiled methods into the mix that make use of the new generic valuetype API. Setup and Copy of a non-POD value type array like ContentRef[] could get a proper speedup.
• Keep eyes open for opportunities to improve performance.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Added more unit tests, streamlined copying value types internally. Down to 1.3x the time of the old system, which is pretty good given the fact that it actually needs to do more than twice the work in order to handle all cases correctly. Spawning objects from prefabs now takes 7x the time of manually instantiating it, instead of 9x like before.

ToDo:

• Extend the CloneProvider to allow keeping its cache for the "Clever SavePoints" use case, and write a test for it.
• Keep eyes open for opportunities to improve performance.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Added support for partial cloning, e.g. "Clever SavePoints".

ToDo:

• Find the courage to actually merge this into the master branch.
• Keep eyes open for opportunities to improve performance.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

Status Report: Merged cloning into the master branch. Observe how it behaves, then commit new packages.

ToDo:

• Keep eyes open for opportunities to improve performance.
• Keep eyes open for broken stuff and fix it.
• Publish new versions of all Duality NuGet packages. Will be necessary due to changed core API.

[AdamsLair]

New NuGet packages have been deployed. Closing this issue.