Reduce alloc

[Eideren]

Description This PR changes a couple of data types to structs to reduces the amount of garbage generated, as those types only communicate with the C++ layer they should be safe to replace with lightweight structs. Changes The types changed are : CollisionObjectWrapper, ManifoldPoint, LocalConvexResult, LocalRayResult and LocalShapeInfo. I have also introduced a new type called 'NonZeroIntPtr' which helps in filling the features that where included with the implementation of those classes but cannot be ported to structs.

---

I need some clarifications though

• Would this line dispose of the data created from this one with this PR ?
• What is BulletObjectTracker's use, it just register classes into an hashset and removes them from it on dispose, so it's only there to avoid them being collected by the GC, right or am I missing something ?

---

The second commit updates the syntax used within /test, I couldn't quite get it to run to validate this PR though. If you really can't find the time to test this out I'll try to figure it out on my own.

[AndresTraks]

There are a few points to consider.

• LocalConvexResult/LocalRayResult could be allocated by the user or by Bullet internally. For example, Bullet allocates the LocalRayResult on the stack here before calling AddSingleResultUnmanaged. In this case, btCollisionWorld_LocalRayResult_delete should not be called, because Bullet will free the memory itself. On the other hand, when the LocalRayResult public constructor is used to initialize the object, then IsUserOwned is set to true (in the current version of the code), and then btCollisionWorld_LocalRayResult_delete should be used.
• IDisposable structs are difficult to use and to reason about, because you could free and zero out a pointer in one copy of the struct and still have the pointer accessible in another copy of the struct. This is a problem especially since LocalConvexResult/LocalRayResult are public. Another problem is that you cannot define a destructor in a struct, which is required to properly clean up unmanaged resources when Dispose does not get called (e.g. in a fault condition or forgetting to call Dispose).
• CollisionObjectWrapper and ManifoldPoint objects could actually be good candidates for structs, because those are typically only allocated by Bullet, so they wouldn't ever need to be disposed. I'll have to think through the use cases.
• In AddSingleResultUnmanaged, even though the memory for the LocalConvexResult/LocalRayResult objects should not be freed, it is still a good idea to call Dispose (in a using block) like you did. That is because the Dispose method should call GC.SuppressFinalize to prevent the destructor from being called by the garbage collector, which is good for performance.
• The ideal approach would probably be to keep a single LocalConvexResult/LocalRayResult object to use every time that AddSingleResultUnmanaged is called and then just swap out the Native pointer. This would work in a single threaded situation, but I haven't checked if e.g. the multithreaded collision world might call addSingleResult in parallel.
• I'm thinking that it is better to not set pointers to zero at object disposal, but to use a boolean variable like isDisposed instead. That way the native pointer can be used as a very fast GetHashCode() implementation (just return the pointer), which is good when putting BulletSharp objects in a dictionary. In that case, setting the pointer to zero would change the hash, which would break the dictionary.

BulletObjectTracker is a work in progress thing to make sure that BulletSharp objects are disposed properly. Eventually it should also keep track of object dependencies to warn when objects are disposed in some wrong order that makes Bullet crash. The BULLET_OBJECT_TRACKING flag is currently set in the repository during development, but I'll remove it eventually.

I think I'll try implementing the single LocalConvexResult/LocalRayResult instance for all AddSingleResultUnmanaged calls, but I'm not sure when I'll get to it. I do see the need to improve performance there.

[Eideren]

Edit: See below instead.

[Eideren]

Result types (LocalConvexResult, LocalRayResult, ManifoldPoint, etc.) give access to write operation onto bullet-managed memory, users are also allowed to create structs of those types, what are the use cases for both of those ?

If we just marshall bullet's ptrs as new c# structures and clear the pointer right away we won't have to deal with forgetting to clear them, there won't be any concurrency issue, and when users instantiate structures of those types we also won't have to create and manage any bullet pointer as it will never interact with bullet anyway, right ? Again, I'm only talking about 'result' types but I feel like we're just making this more complex than it needs to be ?

[AndresTraks]

You're right, it looks like LocalConvexResult, LocalRayResult and LocalShapeInfo are not passed into any native Bullet method and users never need to create those objects. So the classes do not have to wrap unmanaged objects and they do not require destructors or the IDisposable interface. I turned them into structs here: https://github.com/AndresTraks/BulletSharpPInvoke/commit/d9d1f231b7dc09ef5cda76a5f16ee0939eb27acd

It would be possible to initialize all members of the structs with values from the unmanaged objects and forget the native pointer, but that's probably unnecessary work. And it's not likely that someone would accidentally copy a LocalConvexResult/LocalRayResult and try to access the native memory from outside of the AddSingleResult method.

I'll look at CollisionObjectWrapper and ManifoldPoint later.

[Eideren]

Thanks, I'll just close this one off then since your commit took care of most of the issues !