This project has moved. For the latest updates, please go here.
7
Vote

Garbage Generated In Potential Frequently-Called Methods

description

Garbage is generated from the anonymous delegates used in the World.QueryAABB, World.Raycast, World.TestPoint and World.TestAll methods. These methods may be called many times per frame as part of the game engine update logic, potentially generating a considerable amount of garbage. The Xbox360 and WP7 platforms are particularly affected by this issue.

comments

genbox wrote Oct 21, 2012 at 12:44 AM

Do you mean garbage from the delegates themselves, or from the content they have?

The objects inside the delegates are structs, and they don't create garbage as they are removed from as soon as they fall out of scope.

JaredReed wrote Oct 21, 2012 at 1:58 AM

I didn't report the issue, but I voted for it a while back.

For me, it was garbage from the delegates being created on every call.
For example, World.RayCast() is calling ContactManager.BroadPhase.RayCast() and passing in an anonymous function. That function was being wrapped in a Func delegate. That was occurring on every call. I happened to be calling it a few times on every update call, which made the issue much worse.

I was able to get around it by making the anonymous method a real method in the class, and then by storing a member variable of a delegate pointing to that method. I also passed state to it. That seemed to take care of the memory issue for me. I did this in a few places where I was seeing the issue.

genbox wrote Oct 21, 2012 at 6:35 PM

Yes, I see. The Func are the source of the garbage, but it will only be around 32 bytes on each call, which I deemed acceptable back in the day. If one were to make 100 calls of this on each update at 60 FPS, this can escalate quite quickly.

I have fixed raycast and query already. Currently I'm using a delegate wrapper by saving the original callback, calling the wrapper and then use the original delegate call inside the wrapper.

Could you upload a patch with your code? I would like to see if you are doing anything different.

wrote Oct 22, 2012 at 6:21 AM

wrote Feb 22, 2013 at 2:02 AM

genbox wrote Jul 5, 2013 at 7:00 PM

I'm not sure how to optimize the garbage away, without also causing additional problems. For now, I will leave it alone. I'm open to suggestions on how to minimize the garbage.

wrote Feb 1, 2014 at 5:07 PM

Aranda wrote Feb 1, 2014 at 6:00 PM

I'm always up for garbage minimalization so I voted for this issue. @JaredReed are you able to post some of the code you used for avoiding the anonymouse method?

jamesford42 wrote May 26, 2014 at 7:24 AM

Make a method of your class matching the signature oh the lambda/delegate, then store the result in a member variable, along with another bool for 'got result'. Yes this is a really obtuse API for doing what should be a simple query.

jamesford42 wrote May 26, 2014 at 7:28 AM

These methods can both go away:
    public void QueryAABB(Func<Fixture, bool> callback, ref AABB aabb);
    public List<Fixture> QueryAABB(ref AABB aabb);
Replace with:
    public IEnumerable<Fixture> QueryAABB(ref AABB aabb,  Predicate<Fixture> condition);
The ray-cast API should be simplified in the same way.

wrote May 26, 2014 at 7:42 AM

genbox wrote Aug 30, 2014 at 11:56 PM

While I agree that the API could be simplified using IEnumerable, it does pose another issue. You propose a condition, that when it is true, it will include the fixture in the results. However, the current API have a condition that when it is true, it continues the query, false and it completely stops.

It is made that way so that you can gather all you need and then stop the query. Then it is up to you to filter the results. I will implement an enum such that you can signal the callback to include the fixture, or stop the query and see if that works out.

genbox wrote Aug 31, 2014 at 12:39 AM

After some tests, the performance of those methods would be degraded too much due to internal state copying of the CLR.

jamesford42 wrote Aug 31, 2014 at 6:11 AM

It is not necessary to return true/false to stop the query early because the caller can make that judgement and break out of their foreach statement.

jamesford42 wrote Aug 31, 2014 at 6:23 AM

Also the problem with lambda-garbage isn't the byte size of it, its the number of allocations. After a certain number of object allocations (50-100 i think) a garbage collection pass is triggered, even if they are small allocations, this will happen anyway. Frequent and un-intended gc passes is a big deal for your FPS. Many games do GC passes manually (such as when a level ends), then ensure that their game code does ALL allocations during level loading, so that there are no automatic gc passes occurring at all during gameplay.

genbox wrote Aug 31, 2014 at 12:05 PM

The query method is the third layer of abstraction on a tree structure that is decoupled from the rest of the engine by design. The other layers don't benefit from lazy evaluation at all and they are built for high performance, not convenience. Therefore it is necessary to control the lower layers by returning a flag.

As for the garbage collector. It works different on different platforms, and on mobile platforms, the GC will collect after a fixed amount of allocation (1 MB). I'm not worried about GC on non-mobile platforms at all, as they have async generational-based GC.

As for yield, I did a quick analysis on the performance impact, and the call (call, not workload) to the query method got 200x slower because yield saved the state of the local variables in an object, and restored it upon return. You also can't use the ref keyword for the arguments as the value then can't be stored in the state object, and that meant the CLR had to copy the AABB, again reducing the performance of the call.

wrote Oct 21, 2014 at 10:10 PM

wrote Nov 2, 2014 at 9:45 PM