"new" keyword in getter> hit performance?

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 in my code, I make about 2500 calls in a loop for Character.LevelPosition. This means that for the update cycle, there are 5000 “new” Vector2s, and on my laptop it really reduces the frame rate.

I temporarily fixed it by creating

 var levelPosition = Character.LevelPosition; 

before I start the loop, but I seem to feel like its ugly code does this every time I come across a similar situation. Maybe this is the way, but I want to make sure.

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

I am using XNA-Framework which uses Vector2 .

Answer 1

From what I understand, you should avoid allocating a large number of objects from the heap in XNA, because this leads to poor performance. But since Vector2 is a struct , we are not Vector2 anything on the heap, so there should be no problems.

Now, if you have a limited cycle, like yours, in a performance-critical application, for example, in a game, you will always have to think about performance, you can’t get around this.

If we look at the code for LevelPosition , you call the getter for WorldPosition twice and probably a few more getters. The getter for WorldPosition probably triggers several other getters. (It's hard to say what exactly happens without a source, because getter call and field access look exactly the same.)

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

The solution for this is a kind of caching. One option would be to make the LevelPosition field a field and design a system to update it when necessary. This may work, but it can also hurt performance if you need to update it more often than you read it.

Another solution is, as you discovered, caching the result in a local variable. If you know that this is correct, i.e. That the value of the property does not change during the execution of the loop is awesome! You solved the performance problem, and you did it with only one line of code, which is easy for any programmer to understand. What more do you want?

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

• working
• just implement
• easy to understand

I think that such a decision will be very difficult to defeat.

Answer 2

Creating multiple objects in a loop can be an expensive operation (*). Perhaps if it helped to create Vector2 in advance (for example, when changing coordinates) 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. See Modified Source Code.

(*)
Tim Schmelter pointed me to this question with a detailed discussion of the effect of instance objects. I rephrased my initial suggestion that building an object is always expensive. Although creating objects is not always an expensive operation, in some cases it can slow down performance.

Answer 3

You can create a private field to store the value and not calculate it every time. You can create a way to update private fields and somehow subscribe to Movement.Position changes. Thus, the value will be calculated only once when the position is changed.