Pooling for Transient Objects in Maths Classes

[ianp]

Here's the commit message (if nobody objects):

* Added object pooling for quaternions and arrays and started to update the
  maths classes to use pooled objects for temporary calculations.

I'd like to propose that I am going to mark as deprecated those methods which only exist to allow the caller to pass in 'working objects' such as #multLocal(TransformMatrix,Vector3f) in TransformMatrix. If people agree on this I'll mark them as deprecated and then remove them in, let's say, 2-3 months time.

If everybody is OK with this then I'll go through the rest of the maths classes and do the same.

One thing to note: I've created an ObjectPool class in com.jme.util, there are a couple of reasons for this:

• it seems wrong to have com.jme classes depending upon com.jmex classes; and
• the existing object pooling mechanism uses synchronized, which will be problematic with many threads.

also, r.e. synchronized vs. thread local, I think that it would be useful to be able to use the same classes on the client and the server (for multi-player games) and the server environment is probably going to be heavily multi-threaded, so the TL based approach is a huge win there (it should be faster on a modern multi-core CPU based client as well).

The patch is available at http://ianp.org/2009/01/object-pool-patch.txt.gz

[ianp]

I almost forgot: happy new year! 

[hevee]

What are the gains? Can you elaborate?
Also your character encoding for the patch is somewhat messed up, jME's encoding is (unfortunately) Windows-1252, but at some point in generating your patch something appears to have misinterpreted the source code as UTF-8.

[Momoko_Fan]

I like the direction of this. Reduces garbage and increases speed as thread locals are supposedly very fast.
I suggest that you take a more centralized approach however, Perhaps all temporary variables should be grouped in one thread local? E.g ObjectPool.get().tempVec, ObjectPool.get().floatArray16, etc. Since each thread has it's own temp var set, there's no need to manage who has access to what.
Using lists to manage all of this, it might actually be slower as lists create garbage as well.. It's much better to take a light-weight approach on this I think.

[nymon]

I like the idea. To make sure I understand, this would alleviate the use of temp objects repeatedly defined in individual classes?

[basixs]

I am rather concerned about the performance loss in a couple of methods actually...

Code:

     public void lookAt(Vector3f direction, Vector3f up ) {
-        tmpZaxis.set( direction ).normalizeLocal();
-        tmpXaxis.set( up ).crossLocal( direction ).normalizeLocal();
-        tmpYaxis.set( direction ).crossLocal( tmpXaxis ).normalizeLocal();
-        fromAxes( tmpXaxis, tmpYaxis, tmpZaxis );
+        Vector3f x = Vector3f.POOL.get();
+        Vector3f y = Vector3f.POOL.get();
+        Vector3f z = Vector3f.POOL.get();
+        z.set(direction).normalizeLocal();
+        x.set(up).crossLocal(direction).normalizeLocal();
+        y.set(direction).crossLocal(x).normalizeLocal();
+        fromAxes(x, y, z);
+        Vector3f.POOL.recycle(x);
+        Vector3f.POOL.recycle(y);
+        Vector3f.POOL.recycle(z);
     }

This method alone has 3 list lookups, and 3 'returns to the pool'; where before it used 3 global temp variables w/ no overhead to access at all.  This method is used ALOT throughout the jME system and could create a noticeable performance drop (IMO)...

I think in some cases using your pool is very appropriate, however in others global variables might be the way to go...

[renanse]

If I can still offer my two cents here... Part of the rewrite for Ardor3D was to get rid of static variables such as the mentioned helper variables.  The reason was multithreaded access and the same static variable being easily munged by two competing threads.  So if you think you might want MT access, you may have to make some choices about performance (usually a small hit) vs. function.

[basixs]

Ah, thanx Renanse that does change my opinion a little bit

[Momoko_Fan]

If I can still offer my two cents here... Part of the rewrite for Ardor3D was to get rid of static variables such as the mentioned helper variables.  The reason was multithreaded access and the same static variable being easily munged by two competing threads.  So if you think you might want MT access, you may have to make some choices about performance (usually a small hit) vs. function.

Yes, using static variables the way they are now would cause threading issues. That's why it is preferable to call certain non-GL methods in the GL thread. However the performance hit is not necessary, with the use of thread locals, each thread has it's own private temp variables which means you receive both improved memory usage, performance, and thread safety.

[renanse]

Well, maybe.  As long as you don't end up chaining together two methods that make use of the same thread local temp variable.  That was always an interesting issue...  which temp variable is already being used?  Is it tempB that's currently open?  etc. etc.

[ianp]

@Momonko_Fan: agreed that having a single thread local would be less overhead then 1 per class, I'll post an updated patch over the weekend.

@nymon: yes, that's right. Although my main concern is not really to improve performance as much as it is to reduce the number of GC pauses. IMHO it's worth sacrificing ~1FPS or so if by doing so it's possible to reduce the number of GC pauses. If the performance actually improves then that's sweet as well

I'll put together some performance tests to try to measure the impact of this change and post the results.

[ianp]

@hevee: oops! I'll make sure the encoding gets set to CP-1252 before it hits the repository.

Hmmm, as I'm using a Mac it seems like I'll have to use ISO-8859-1, as I understand it this is pretty compatible with CP-1252 so I hope there won't be any issues.

[ianp]

Using lists to manage all of this, it might actually be slower as lists create garbage as well. It's much better to take a light-weight approach on this I think.

I don't think that the lists will be a problem really, if you look at the ObjectPool class it has a (user configurable via a system property) limit on the number of pooled instances that will be held, this is low by default ( 8 ), I've also edited the patch to set the initial size of the list to this value, so that should ensure that it never needs to grow and reallocate storage space.

[vear]

Just to add my opinion on object pools.

You should avoid having "Vector3f.POOL.recycle(x)". Serving objects from a pool is ok, but when you have to manage the deletion too, it gets unmanageable. I suggest to have a single "Vector3f.POOL.clear()" which just resets the pool to serve the first object again. The point is: its not the memory you want preserve by reusing objects, you just want to avoid new object creation.

http://code.google.com/p/vlengine/source/browse/trunk/vle_cleanup/src/com/vlengine/bounding/ReuseManager.java

You can reset the pool each frame. So the program does not lose too much time micro-managing the pool, but wipes all object at once.

[ianp]

@vear: doesn't that make it difficult if multiple methods want to grab objects from the pool? having a single clear method would mean that you'd need to manually keep track of which methods are on the stack and when you need to clear the pool. Being able to return individual objects actually seems like it would be easier.

I think that for most temporary objects they're acquired and recycled in the same method, so it's fairly easy to manage (as long as the methods don't get too big, but that's a code smell anyway).

I'll have a look at the VL Engine code though.

[Tags:]

jMonkey Forum > jME2 Development > Contribution Depot (jME2) (Moderator: sbook) > Topic: Pooling for Transient Objects in Maths Classes