'new' keyword in getter > performance hit?

Question

I have the following code:

public class Character
{
    public Vector2 WorldPixelPosition
    {
        get { return Movement.Position; }
    }
    public Vector2 WorldPosition
    {
        get { return new Vector2(Movement.Position.X / Tile.Width, Movement.Position.Y / Tile.Height); }
    }
    public Vector2 LevelPosition
    {
        get { return new Vector2(WorldPosition.X % Level.Width, WorldPosition.Y % Level.Height); }
    }
}

Now somewhere else in my code, I make about 2500 calls in a loop to Character.LevelPosition. This means that per update-cycle, 5000 'new' Vector2s are being made, and on my laptop, it really drops the framerate.

I have temporarily fixed it by creating

var levelPosition = Character.LevelPosition;

before I initiate the loop, but I kinda feel its ugly code to do this everytime I come across a similar situation. Maybe it -is- the way to go, but I want to make sure.

Is there a better or commonly accepted way to do this?

I'm using the XNA-Framework, which uses Vector2's.

Answer 1

From what I understand, you should avoid allocating lots of objects from the heap in XNA, because that causes bad performance. But since Vector2 is a struct, we're not allocating anything on the heap here, so that shouldn't be the problem here.

Now, if you have tight loop, like you do, in a performance-critical application, like a game, you will always have to think about performance, there is no going around that.

If we look at the code for LevelPosition, you call the getter for WorldPosition twice and probably some more getters. The getter for WorldPosition probably calls few other getters. (It's hard to say what exactly is going on without having the source, because getter call and field access look exactly the same.)

Call to a getter, which is actually just a call to a special method, is usually pretty fast and can be even faster if the compiler decides to use inlining. But all the calls add up together, especially if you call them in a loop.

The solution for this is some sort of caching. One option would be to make LevelPosition a field and devise a system to update it when necessary. This could work, but it could also actually hurt performance if you need to update it more often than you read it.

Another solution is, as you discovered, to cache the result in a local variable. If you know that this is correct, i.e. that the value of the property won't change during the execution of the loop, then that's awesome! You solved your performance problem and you did it with only a single line of code that's easily understandable to any programmer. What more do you want?

Let me restate that. You found a solution to your performance problem that:

1. works
2. is simple to implement
3. is easy to understand

I think such solution wold be very hard to beat.

Answer 2

Creating many objects in a loop may be an expensive operation (*). Maybe if would help to create the Vector2 in advance (for example when the coordinates change) and in the future just change the coordinates.

Example:

public class Character
{
    private Vector2 m_worldPosition = new Vector2(0, 0);
    private Vector2 m_levelPosition = new Vector2(0, 0);

    ....

    public Vector2 WorldPosition
    {
        get
        {
            m_worldPosition.X = ...;
            m_worldPosition.Y = ...;
            return m_worldPosition;
        }
    }

    public Vector2 LevelPosition
    {
        get
        {
            m_levelPosition.X = ...;
            m_levelPosition.Y = ...;
            return m_levelPosition;
        }
    }
}

EDIT
The same should be done for the LevelPosition property as well. See modified source code.

(*)
Tim Schmelter pointed me to this question with a detailed discussion about the impact of instantiating objects. I have rephrased my initial sentence that object creation is always expensive. While creating objects is not always an expensive operation, it may still slow down performance in certain cases.

Answer 3

You can make a private field to store the value and not compute it each time. You can make a method to update the private fields and subscribe for the Movement.Position changes in some way. This way the value will be computed only once when position changes.