getType() vs instanceof

Why have you introduced this getType()? I liked instanceof alot more - it's Java!



If it is because of the myth that instanceof would be slow, one can easily proove the opposite:

Run this simple test with JRE1.5:

package jmetest.util;

import java.util.logging.Level;

import com.jme.app.SimpleGame;
import com.jme.scene.Geometry;
import com.jme.scene.Node;
import com.jme.scene.Spatial;
import com.jme.scene.shape.Box;
import com.jme.util.LoggingSystem;

/**
 *
 */
public class TestInstanceOf extends SimpleGame {
    private static final int COUNT = 100000;

    /**
     * Called near end of initGame(). Must be defined by derived classes.
     */
    protected void simpleInitGame() {

        Spatial[] spatial = new Spatial[COUNT];
        for ( int i = 0; i < COUNT; i++ ) {
            if ( ( i & 1 ) != 0 ) {
                spatial[i] = new Node( null );
            }
            else {
                spatial[i] = new Box( null );
            }
        }
        {
            long start = System.currentTimeMillis();
            for ( int j = 0; j < 100; j++ ) {
                for ( int i = 0; i < COUNT; i++ ) {
                    Spatial s = spatial[i];
                    if ( s instanceof Node ) {
                        ( (Node) s ).getQuantity();
                    }
                    else if ( s instanceof Geometry ) {
                        ( (Geometry) s ).getModelBound();
                    }
                }
            }
            long time = System.currentTimeMillis() - start;
            System.out.println( "With instanceof: " + time + " ms" );
        }
        {
            long start = System.currentTimeMillis();
            for ( int j = 0; j < 100; j++ ) {
                for ( int i = 0; i < COUNT; i++ ) {
                    Spatial s = spatial[i];
                    if ( ( s.getType() & Spatial.NODE ) != 0 ) {
                        ( (Node) s ).getQuantity();
                    }
                    else if ( ( s.getType() & Spatial.GEOMETRY ) != 0 ) {
                        ( (Geometry) s ).getModelBound();
                    }
                }
            }
            long time = System.currentTimeMillis() - start;
            System.out.println( "Without instanceof: " + time + " ms" );
        }
        System.exit( 0 );
    }

    public static void main( String[] args ) {
        LoggingSystem.getLogger().setLevel( Level.WARNING );
        new TestInstanceOf().start();
    }
}



Output:

With instanceof: 1632 ms
Without instanceof: 2063 ms



So why getType  :?

Hmmmm, I based that decision on a couple optimization articles. However, I didn't remove instanceof from the language  :wink: so, if this is the case, we can change the getType stuff in the jME core back to instanceof and no one will know. We can leave the getType for other people to use if they wish. I guess we need something more authoritative than a couple articles on the web and a micro-benchmark.

mojomonk said:

I guess we need something more authoritative than a couple articles on the web and a micro-benchmark.

And what would that be? More authoritative than testing it yourself?  ;)
And what would that be? More authoritative than testing it yourself?


What I mean is your microbenchmark is not always effective for real performance (in fact, studies shown they can be misleading)... so try not to get holy on me. ;)

But that being said, I really don't care, they were very quick and easy changes and can be switched back. The getType will still be there for those who want to use it.

Well, let's make sure these microbench results hold up in the real world.  I recall you saying that FPS rose when you made the switch.  I don't want to throw away real world performance due to some microbench result.  We need a real test.

Since renanse made a comment on the CVS mailing list on this (reverting back to instanceof), I thought I might as well bring this old thread back…



It seems in the straight forward case irrisor decribes instanceof is pretty damned good now.



The line that more or less kicked this off again is line 361 in RenderQueue:


 if ((o1.getType() & SceneElement.GEOMBATCH) != 0 && (o2.getType() & SceneElement.GEOMBATCH) != 0) {



Now that I look at it:


if ((o1.getType()  & o2.getType() & SceneElement.GEOMBATCH) != 0) {
       return compareByStates((GeomBatch) o1, (GeomBatch) o2);
}



seems to make sense to me (after all, it's likely to be true in most cases). This might even give it a little edge over instanceof

Also, irrisors test takes a Node and then looks if it's Node. It does not, for example take a BillboardNode and looks if it's a Spatial. There were a lot of calls to see if a SharedBatch was a GeomBatch (with this line) that prompted me to make the CVS commit that renanse commented on. I think that's the kind of scenario that used to make instanceof slow and promted all those optimizations articles (not asking if Node is a Node), but ofcourse Sun could have optimized for these cases just as well.

So, food for thought and more testing perhaps..?

You'll notice I was not against throwing out getType in my above posts either.  I guess my main gripe is maintaining the methods when Java syntax gives us this already and also the fact that we don't uniformly use getType or instanceof.

Yes, I don’t have any clear preference myself… kind of hard when you don’t know which is better. Maybe I’ll do some tests too when I’m back in the country this friday.



I know we’re dealing with minute differences here probably, but in many cases it is the most invoked piece of code in the whole engine. I checked in the change I mentioned belowabove since it seems logical we should optimize for GeomBatch. In fact, is there any type of Geometry or SceneElement that can be put in the queue that’s not GeomBatch derived left anyway?

I'm not saying anything on this topic…



*** -server flag ***



…What was that?



darkfrog

oblique… :?