Check for null or check for implementation?

Performance Tuning
#1

So I am curious, say I have this situation:

I have a base Entity class, it is not specialized in anything and only has a few members, once of them being a sprite.

So say I have a Player class that extends from this but the Player also implements an interface called Drawable.

I put a bunch of entities into an array, only the player implements this.

Is it faster to just do this:



if(sprite != null)
   entity.draw(batch)

Or is it better to check for implementation like so:


	if(entity.getClass().isAssignableFrom(Drawable.class)){
				Drawable drawable = (Drawable) entity;
				drawable.draw(batch);
			}

Although this is an example, and has quite a few flaws (why would a base class have a sprite if it is not specialized etc) but it is mainly a quick scenario I could think of.

What would be the logical one to do? Which would be faster?

#2

Null checks are very fast, reflection is generally slow.
Source, under “Low level safety checks”: https://wikis.oracle.com/display/HotSpotInternals/PerformanceTechniques

#3

Thanks for the link!

It is something I have never though of before, good to know at least. Not that I have any practical use for it at the moment but never know down the line.

#4

Not sure that counts as reflection, but anyway not sure why the OP’s second example isn’t


if (entity instanceof Drawable)

And even then it’s the wrong way around


if ( Drawable.class.isAssignableFrom(entity.getClass()) )

And be wary of either - have an overridden method in your entity if possible, even if that’s a no-op.

#5

[quote]Class.isInstance and Class.isAssignableFrom are as cheap as instanceof bytecodes when the operands are constants, and otherwise no more expensive than aastore type checks.
[/quote]
doesn’t tell if its client or server or both. but usually server performs better.

#6

[quote=“Gibbo3771,post:1,topic:50961”]
First review your program design - the need to cast or use instanceof is often a warning the design could be cleaner. I’ve many times cast when coding in a hurry and then proceeded to run into stickier and sticker waters only to finally realise the problems were coming from the issue I ducked with the cast to start with :slight_smile: Obviously it’s sometimes OK, especially if you’re modifying an existing program where you don’t really have the option to redesign everything, but still have a think anyway.

But to answer the question I usually put a function in the base class like boolean isDrawableInMainEntityLoop() or whatever. It’s clear but kind of deliberately clunky as a reminder there’s a potential hack there.

#7

[/quote]
Note the emphasis though! :wink:

So you replace a wart with a tumour? ;D IMO people sometimes try too hard to remove instanceof and replace it with something many times more complicated or worse performing. I think it has its place, particularly considering Java lacks double dispatch. I think using it to search for objects with a certain capability (ie. implements Drawable) is OK, as long as you’re only looking for a single capability at a time - if you have a chain of ‘if instanceof else if instanceof’ then you do have a problem.

#8

instanceof has a rather large stink factor. If you need one then you should think if there’s a design flaw. That’s quite different from saying you should never use one…and if you do then jumping through hoops to avoid it probably isn’t the solution.

If the original question was important…then the answer is in that statement.

[quote][icode]if(sprite != null) entity.draw(batch);[/icode]
[/quote]
This is a very odd construction. Checking if sprite is null and then not directly doing anything with sprite. From the previous it looks like everything is burdened with ‘sprite’ and only the player will have non-null value. If so then ‘sprite’ is useless.

#9

I actually like instanceof, especially when designing object hierarchies. I never got why so many people try to work around it. It’s quite straight forward and easy to just ask an object what it is to decide what to do with it. Why would you not want to use such an important attribute of an object like it’s type?

Having said that, the OPs problem is easy to solve by just calling [icode]entity.draw(batch)[/icode] on any type of entity and implement the [icode]draw(batch)[/icode] empty for entities not having a sprite… I doubt that using the null-check to prevent calling an empty method will boost performance significantly for any kind of game we are doing here.

I would do it different though (using instanceof). Something along the lines of:


class World
{
    List<Entity> entities=new ArrayList<Entity>();
    List<Drawable> drawables=new ArrayList<Drawable>();

    add(Entity e) {
        entities.add(e);
        if(e instanceof Drawable)
            drawables.add((Drawable)e);
    }

    draw(Batch batch){
        int l=drawables.size();
        for(int i; i < l; i++)
            drawables.get(i).draw(batch);
    }
}

You can do this kind of specialized “caches” with all kind of “filters” that create more or less static lists of objects you need to iterate over and handle type (or otherwise) specific. The only downsides are slightly increased memory foodprint (multiple references to the same object) and some performance overhead for adding and removing objects to/from multiple lists.

I really can’t see, why this kind of desing is bad practice, inperformant or “stinks”.

#10

Let’s ignore performance tuning. Cylab’s example is reasonable enough, although I’d think about only placing each in one list instead in both…but without further knowledge I can’t really say. But it’s somewhat moot for OP’s question: “… only the player implements …”

Generally using instanceof is an explicit type query which in general requires walking lists and requires selecting the exactly right type. And the situation is worse if the type is an interface type. Choose the exactly right type can be problematic if you rework the type hierarchy and all relevant instanceof checks will require change. So instead of an explicit type query, replace it with an implicit via an instance method/variable. Remember that this is all general speak.

#11

It’s in both lists because Entity would be the most generic type in this example and Drawable is a subset. So if you want to do something with all entities you would iterate only the entities list. This makes more sense, if you have more subsets, especially if they might overlap partly, so you can’t just iterate over all lists to so something with all entities once.

#12

I do null checks quite often now, I know it’s a coding faux pas, but damn it. It makes sense in a lot of cases. :wink:

For example, when a villager dies in RPC, part of the death() method does this:

if (mate != null){
	mate.decreaseHappiness(75);
	mate.removeMate();
	removeMate();
}

Basically, it checks if it has a mate, if it does, removes the “bond” between the two, so the survivor can find a new mate in the future.

I believe the technically correct answer is “you shouldn’t do it” though. But hey, I like it.

#13

Make OOP purists recoil in horror with the suggestion that you stick loads of methods into a base class! That’s what I do.

In general I look back from the requirements (“I need to be able to this, this, and this, with the things in a list”) and then work out what thing needs to be done to satisfy those requirements (“so I need to have this, this, and this declared for all potential objects in the list”). Quite often that leads to the perfectly reasonable design of having a phat base class full of abstract methods (or, more usefully, an interface).

Then I move on to more actually difficult problems :wink:

Cas :slight_smile:

#14

@cylab: Like I said…it depends what I’d do. If partitioned into different sets then you just iterate over the set of sets that each transform needs to work with, then nothing needs to check if the transform applies to that specific instatance.

@Rayvolution: that’s a statistically insignificant example…doing whatever works best for your brain is the right approach.

This.

#15

This should be the answer to all programming questions. :wink:

#16

Huh? Why? IMO, using with interfaces makes the most sense as it’s effectively using instanceof as has-capability-to.

That depends. Having no-op methods isn’t too bad, but take something like the List interface - having an API where half the methods are defined to throw exceptions when they’re not supported is IMO even more daft than having a sub-interface and using instanceof to check if it supports modification.

+1 ;D

#17

Short answer. Classes only have a single parent and interfaces are a potentially arbitrary list per class, so in general more searching is required in the interface case. And yes both cases include possibilities of optimization.

#18

None answer?? :stuck_out_tongue: I’m not sure where the list is relevant, unless we’re talking about the actual bytecode required (which I didn’t think you were?)

This is where an interface would seem to make more sense, not less. If you treat the interface as a capability you’re filtering for, you can change your type hierarchy and implementation as much as you want - a good reason to choose an interface over a concrete type, no?

I’m not advocating it as a general approach, but it does make sense occasionally, particularly when (as per previous post) the alternative involves methods throwing exceptions.

Another alternative I like, though not going to perform quite as well, is something like the Lookup.Provider API in the NetBeans platform where you make the entity responsible for providing its own capabilities - something like


Drawable d = entity.getLookup().lookup(Drawable.class);
if (d != null) {
  d.draw();
}
#19

There a nice libs out there even for java.
Because I see Interfaces used as components right here in this thread :smiley:

-ClaasJG

#20

Actually it’s the oposite way around. Entity systems use components as replacement for interfaces :stuck_out_tongue: