Search Unity

Help Wanted Collider.Create performance

Discussion in 'DOTS Physics' started by Shinyclef, Feb 11, 2021.

  1. Shinyclef

    Shinyclef

    Joined:
    Nov 20, 2013
    Posts:
    439
    Hello,

    Background: I'm procedurally generating content, I have thousands of box colliders I would like to create, most of them will be static. They are being generated so I can create compound colliders in chunks of 64x64 tiles.

    upload_2021-2-11_16-49-11.png

    Using profiler markers, I have determined that this line right here is causing the vast majority of time taken by by my job:
    Code (CSharp):
    1. createBoxColliderProfilingMarker.Begin();
    2. BlobAssetReference<Collider> boxCollider = BoxCollider.Create(boxGeometry);
    3. createBoxColliderProfilingMarker.End();
    upload_2021-2-11_16-51-24.png

    Most of the time is on memory allocation.
    I do understand that I can use the collider more than once when I create a CompoundCollider.ColliderBlobInstance. This would require some code complication as I will have many different collider shapes and sizes. I am still considering approaches to do this as a last resort.

    Is there any other advice on reducing the performance hit of creating so many colliders? It is by far the slowest part of my procedural generation.

    Thanks :)
     
    Antypodish and steveeHavok like this.
  2. steveeHavok

    steveeHavok

    Joined:
    Mar 19, 2019
    Posts:
    480
    caveat emptor: I haven't tried any of this!

    The BoxCollider has 2 parts to the setup (immutable and mutable data). The first immutable data is the edge and face links of a convex hull making up the box and this is the part that does all the memory allocation. The second mutable data is the collider specific BoxGeometry. So rather than calling BoxCollider.Create you could instead Clone another BoxCollider (one single mem alloc and copy rather than several), and then set the BoxGeometry to update the mutable data only.

    Want you probably really want would be some custom shape that only generated BoxColliders when necessary (a bit little the Mesh and Terrain Colliders only generate PolygonColliders when necessary), but that is a big kettle of fish that I shouldn't have even mentioned ....
     
    Shinyclef likes this.
  3. Shinyclef

    Shinyclef

    Joined:
    Nov 20, 2013
    Posts:
    439
    I found the sample where the sphere collider geometry was being updated via pointers. I have managed to get a proof of concept working where I first create a collider with 'default' (incorrect) geometry, and immediately after use the unsafe context to edit the BoxGeometry. This worked! So then I thought, ok, maybe I can pool up a few thousand colliders in the background to ease the hit. But your suggestion of cloning is very intriguing and would be way nicer!

    But alas I must admit I do not know how to clone! Are you referring to Collider.Clone(), which returns a BlobAssetReference<Collider>()? If so that method does not specify how many clones. Wouldn't I still need to clone many times, where each clone needs a mem alloc? I must be missing something.

    (Your kettle of fish is going over my head so I am trying simple box cloning first haha).

    Edit: Ah is it so that the Cloned collider would reference the same instance of immutable data thus preventing the additional alloc?
     
  4. Shinyclef

    Shinyclef

    Joined:
    Nov 20, 2013
    Posts:
    439
    I tried Collider.Clone().
    It looks like it is still doing the mem allocs. However the updating of the BoxGeometry via pointers is working.

    Code (CSharp):
    1. createBoxColliderProfilingMarker.Begin();
    2. BlobAssetReference<Collider> collider = colliderTemplate.Value.Clone();
    3. createBoxColliderProfilingMarker.End();
    4.                        
    5. unsafe
    6. {
    7.     BoxCollider* bcPtr = (BoxCollider*)collider.GetUnsafePtr();
    8.     BoxGeometry boxGeom = bcPtr->Geometry;
    9.     boxGeom.Size = size;
    10.     bcPtr->Geometry = boxGeom;
    11. }

    upload_2021-2-12_2-24-4.png
     
  5. steveeHavok

    steveeHavok

    Joined:
    Mar 19, 2019
    Posts:
    480
    No you aren't missing anything. Clone on the BoxCollider would only do one mem alloc rather than a bunch, but looks like cloning at that level doesn't help. You could clone the CompoundCollider (which would clone all the children in one go). That would assume the type and memory size of each child remains the same, which in your boxes only case is fine. You would need to update the Bounding Volume Hierarchy for the Compound as well though and that would mean tweaking the Unity Physics code.
    I can see what is involved there. It might be a small change and could help this use-case (and voxel landscapes).
     
  6. Shinyclef

    Shinyclef

    Joined:
    Nov 20, 2013
    Posts:
    439
    There is a different amout of colliders per compound however. So not that simple... But you're giving me new ideas. Maybe I could clone a native array of colliders or something?

    Quick feature request actually, could you provide the option to bulk create colliders, perhaps passing in many geometry structs, to do one giant mem alloc at once? Similar to the mesh api?

    Also, if I am destroying and creating terrain chunks often? Am I going to have a memory leak this way? I'm thinking maybe I should be reusing/pooling these colliders some how to avoid the constant allocs?

    Also you mentioned that the part that does the mem alloc, the immutable part is not related to the geometry. In that case, why can't all box colliders share the same immutable part to prevent many allocs? I guess there is a reason...
     
  7. Shinyclef

    Shinyclef

    Joined:
    Nov 20, 2013
    Posts:
    439
    I think I've got the best outcome...

    Same amount of colliders per compound collider. Completely avoided mem alloc, saving roughly 8ms per job.
    upload_2021-2-13_15-37-15.png

    I started by creating a lot of colliders in OnCreate. I create the max amount of colliders one of my compound colliders could possibly use, multiplied by thread count. This is my colliderPool.
    Code (CSharp):
    1. private void SetupColliderPool()
    2. {
    3.     int threadCount = JobsUtility.JobWorkerCount + 1;
    4.     int total = threadCount * PooledCollidersPerThread;
    5.  
    6.     colliderPool = new NativeArray<BlobAssetReference<Collider>>(total, Allocator.Persistent);
    7.     NativeArray<BlobAssetReference<Collider>> pool = colliderPool;
    8.  
    9.     // this takes roughly 100ms regardless of thread count.
    10.     Parallel.For(0, threadCount, t =>
    11.     {
    12.         int start = t * PooledCollidersPerThread;
    13.         for (int i = start; i < start + PooledCollidersPerThread; i++)
    14.         {
    15.             pool[i] = BoxCollider.Create(new BoxGeometry() { Orientation = quaternion.identity, Size = 0f });
    16.         }
    17.     });
    18. }
    Next, in the job where I will be creating a compound collider, I create a slice of the colliderPool for the thread so it does not compete with any other threads for the pooled colliders it will work with. I use the unsafe context to change the geometry of pooled colliders, and use them to construct a compound collider. I could modify this so that if no pooled colliders are left, I expand the pool, or create new colliders, but for now I don't need to do so.

    Code (CSharp):
    1. createBoxColliderProfilingMarker.Begin();
    2.  
    3. var poolSlice = new NativeSlice<BlobAssetReference<Collider>>(pool, (nativeThreadIndex - 1) * PooledCollidersPerThread, PooledCollidersPerThread);
    4. for (int i = 0; i < count; i++)
    5. {
    6.     BlobAssetReference<Collider> collider = poolSlice[i];
    7.     unsafe
    8.     {
    9.         BoxCollider* bcPtr = (BoxCollider*)collider.GetUnsafePtr();
    10.         BoxGeometry boxGeom = bcPtr->Geometry;
    11.         boxGeom.Size = colliderGenData[i].Size;
    12.         bcPtr->Geometry = boxGeom;
    13.     }
    14.  
    15.     compoundChildren[i] = new CompoundCollider.ColliderBlobInstance()
    16.     {
    17.         CompoundFromChild = new RigidTransform(quaternion.identity, colliderGenData[i].Center),
    18.         Collider = collider
    19.     };
    20. }
    21.  
    22. createBoxColliderProfilingMarker.End();
    So in summary, every time I create the compound collider I am reusing the same pooled box colliders. This results in no memory allocations for box colliders (apart from once in OnCreate). I still have a mem alloc for the compound collider but that seems to be no issue.

    It seems to be working just fine. Is there any reason why this might come back to bite me... or a reason why this is not recommended? If not, do you think we could get access to change the geometry without needing the unsafe context?

    Thanks for your help :)
     
    Last edited: Feb 13, 2021
    argibaltzi likes this.
  8. argibaltzi

    argibaltzi

    Joined:
    Nov 13, 2014
    Posts:
    164
    Does each box collider need to be unique in the world?
    For example if all my voxel boxes are (1.0f, 1.0f, 1.0f), can i just make 1 box collider and use that in all compound colliders?
     
  9. Shinyclef

    Shinyclef

    Joined:
    Nov 20, 2013
    Posts:
    439
    That's not something I have tested. Give it a try :).
     
  10. argibaltzi

    argibaltzi

    Joined:
    Nov 13, 2014
    Posts:
    164
    you are making me work extra hours
     
  11. argibaltzi

    argibaltzi

    Joined:
    Nov 13, 2014
    Posts:
    164
    I did a little test with 4 boxes and 1 compound collider and it seemed to work as expected in the simulation, which is good news. Not sure if this approach can cause problems down the road. Perhaps someone from havok can give some advice on using "master box colliders" with compound colliders

    SphereCasts on compound colliders and GetLeaf might not work properly? but if you dont need to get the Leaf this could be a nice optimization

    Code (CSharp):
    1. // COLLIDER
    2.         float3 size = new Unity.Mathematics.float3(1, 1, 1);
    3.         float3 center = new Unity.Mathematics.float3(0, 0, 0);
    4.  
    5.         BlobAssetReference<Unity.Physics.Collider> boxCollider = PhysicsAssistant.CreateBoxCollider(center, size, 0, PhysicsMaterialRef);
    6.  
    7.  
    8.         NativeArray<Unity.Physics.CompoundCollider.ColliderBlobInstance> Blobs = new NativeArray<Unity.Physics.CompoundCollider.ColliderBlobInstance>(MaxAxis, Allocator.Temp);
    9.  
    10.         for (int y = 0; y < MaxAxis; ++y)
    11.         {
    12.             Blobs[y] = new Unity.Physics.CompoundCollider.ColliderBlobInstance()
    13.             {
    14.                 Collider = boxCollider,
    15.                 CompoundFromChild = new Unity.Mathematics.RigidTransform(
    16.                 Quaternion.identity,
    17.                 new Unity.Mathematics.float3(0, 1 * y + 0.5f, 0))
    18.             };
    19.         }
    20.  
    21.         BlobAssetReference<Unity.Physics.Collider> MyCollider = Unity.Physics.CompoundCollider.Create(Blobs);
    22.         Blobs.Dispose();
     
    Shinyclef likes this.
  12. milos85miki

    milos85miki

    Joined:
    Nov 29, 2019
    Posts:
    197
    So glad to see a productive discussion here!

    It's totally ok to reuse colliders, we even expose it in editor via "Force Unique" checkbox in Physics Shape component.

    Unfortunately, you currently cannot cast a Collider into a BoxCollider without pointers and unsafe code. Don't worry, though, it will be fine. :)
    There are internal discussions around unsafe methods in our API and how to make them more user-friendly (safe).
     
  13. slushieboy99

    slushieboy99

    Joined:
    Aug 29, 2014
    Posts:
    31
    Hi all, I'm trying to figure out a faster way to do mesh collider generation and a lot of stuff in this thread has got me thinking. Does anyone know if there is a possible implementation of this for a compound collider of polygon colliders?
     
unityunity