Laggy rendering.

[Veritas]

The problem is that I don't have any experience with profilers and many of the advised ones are for windows for which I don't really have a copy at the moment. I could spend a lot of time trying to find something that works for my system and I will, but right now I am just working on the project on my free time since I have to study for the university entry exams. Besides, the functions are simply wrappers for an std::find_if call using dynamic_casting comparison as a predicate. I heard that dynamic_casting is slow and std::find_if uses linear search. I don't think that it's far-fetched to believe that this is the bottle neck. I put some std::couts in the functions and putting the update functions aside, hasComponent and getComponent are pretty much the only ones that get called.

[Jesper Juhl]

The problem is that I don't have any experience with profilers and many of the advised ones are for windows ...

All the ones I mentioned run om Linux (and other UNIX platforms) 

[Nexus]

If you don't want to use profilers, then use one of the classical debugging approaches. Simplify the code, reduce various parts until things run fast, and you will know what caused the slow-down.

dynamic_cast is indeed often an indicator of bad design -- runtime polymorphism (virtual functions) exists for the whole reason of abstracting from dynamic types. However, I doubt it's the reason of bad performance if you call it only dozens of times per second. Maybe you're performing unnecessary and expensive copies somewhere.

A map with std::type_index sounds better -- you're still employing RTTI, but in an implicit way. The idea is to go away from explicit case differentiations, and let language or library mechanisms (VTable or map data structure) do it for you.

[Veritas]

I understand that dynamic_cast is an indicator of bad design when there are actual hierarchical relations between the base and the derived classes. In my case they are simply bags of data, no functions whatsoever. They are "components" but for the components to have meaning you need to get the data they contain. Such things are easier to implement in dynamic languages than strong typed ones and if anyone has any better alternatives I am all ears.

As for copying, the two functions simply return pointers. I was very troubled on why there is such a performance issue and it's why I made the topic in the first place.


PS: Is it note worthy that I am using assert? It shouldn't create any problems since I have defined NDEBUG but just asking to be safe.

[Nexus]

I understand that dynamic_cast is an indicator of bad design when there are actual hierarchical relations between the base and the derived classes. In my case they are simply bags of data, no functions whatsoever. They are "components" but for the components to have meaning you need to get the data they contain.

The point is that your abstraction is not the best one if you have to query explicitly whether an object is of certain type. It doesn't matter if you use if/else, switch, dynamic_cast or typeid for this case differentiation. Since you seem to use it only internally, you can directly use a map and trade the O(n) search for a O(log n) or O(1) lookup. Not that performance would make any difference for such small numbers, but it's generally a better and more scalable design that would help in bigger scenarios.

The user should just ask your system: give me the component X, and component X not being there is typically a logic error.

Such things are easier to implement in dynamic languages than strong typed ones

Not really, unless your language has dynamic dispatch. Otherwise you have the same problems. Sometimes, it's even worse since you don't even have overloading and are forced to dispatch manually.

[Veritas]

Yes dynamic_cast on the comparisons was a bad design, like I said I will have to replace them by using a map. The map however needs to store values of the same type. This is where downcasting is needed.

Anyway at this point, there isn't much that can be done about my problem. I will change the way GameObjects store the components and I will come back to you.

[Nexus]

This is where downcasting is needed.

Yes, but static downcasting -- you know the type in advance.

[Veritas]

I think there is a misunderstanding here. I used dynamic_cast as a predicate for type-checking in the hasComponent function. The getComponent function uses static casts but also calls the hasComponent function to validate the request. I realized that this just doesn't work, dynamic_casts are slow and linear search (especially using dynamic_cast as a comparison) is not effective. So I am looking for ways to improve the design. So far I think that a map using std::type_index as the key should be a good way to handle things. There is no need to cast everything for comparisons and checking for existance will be covered by the map. Plus we get logarithmic lookup which is always nice.

[Nexus]

Yes, I think I understood that

What I meant is that -- since your map can only store base class pointers -- you will need a static cast to the derived class when requesting the component.

template <typename T>
T& GetComponent()
{
    std::unique_ptr<BaseComponent>& basePtr = map[typeid(T)];
    // better: map.find() or map.at() to not insert new ones
    return static_cast<T&>(*basePtr);
}

[Veritas]

Oh yes I understand that. I said I will have to downcast the pointer, I didn't plan to use dynamic_cast. Even in my current code I only used it as a predicate for the find_if function. I used static_casts for the other cases. Maybe I didn't come across as I wanted, my English isn't perfect.

[Veritas]

Hello everyone again. I have almost finished rewriting the code. It should work after I manage to deal with one minor detail. The engine has two containers. One std::map that keeps the unique_ptrs to the systems and the pipeline which is a vector and should also refer to the wanted systems. Both containers need to keep pointers to the same systems. Am I wrong in assuming that I have to change the unique_ptrs to shared_ptrs? Also, the systems contain pointers of GameObject. They are not responsible for them though, they simply register the game object that meet the criteria. Shouldn't the container keep normal pointers to them instead of the stl ones?

Basically:

- The GameObjects own their Components so they should keep unique_ptrs.
- The Engine owns the system but it needs to keep them in two containers so it should keep shared_ptrs.
- The Systems don't own the Components so they should keep normal pointers to them.

[Ixrec]

It is an option, and likely the one you want, but there are other options, so it's not a good idea to simply assume shared pointers is what you want without thinking about the actual ownership semantics.

If you use shared pointers in both places, that means if either place destroys it, it will continue existing and the other place can continue to use it.  Both places will "own" the object they point to.  This is certainly the simplest solution.

You can also use shared pointers in one place and weak pointers in another.  That means if the first place destroys any, the second place will automatically lose it as well (in a safe and deterministic way, of course).  Only the first place "owns" the objects.

It's also valid to use unique pointers in one place and raw pointers in another *if* you can guarantee the first place won't start destroying objects until after the second place is done using them.  In the context of an entity system, this is almost certainly not the case, so you probably won't do this.  But it's worth noting that raw pointers are not evil if they are used only to point, and not to own.

I'm sure Nexus and the others will have more to add to this, but that's what I know.  In your case it probably boils down to whether you want the pipeline to be able to keep using objects the systems have already destroyed, and vice versa.  I'm not familiar enough with entity systems to know if there's an obvious answer to that.

[Veritas]

It wouldn't make sense to remove the system from the map if you removed it from the pipeline. You may want to free the pipeline from proccessing a system for some time. In this case a unique_ptr doesn't make sense. Now there is also the detail that registring is handled in the system map. You may want to keep processing the already registered objects but stop all the other registrations so you may not want to remove the system from the pipeline if you remove it from the systems container. I don't think I'm going to use such a feature but since I have to decide I may as well think of the different scenarios.

[Veritas]

UPDATE:

I officially have no idea what's wrong. I got rid of all dynamic_casts , used a map for fast retrieval and changed the raw pointers to the appropriate smart ones but the problem persists.

Here is a part of the fps log:

884.956
29.7389
831.947
32.1719
285.063
58.2479
87.0095
373.692
64.7166
80.8211
502.513
74.5657
26.6007
20.6979
480.538
33.0393
476.417
19.75
1584.79
70.4871

The fps jump from good to garbage. The logic is the same in every iteration, there are no any memory allocations in the loop and the only if statements are the ones in the input system. I am completely lost, I guess I will have to find some time to learn how to work with a profiler.

[Veritas]

UPDATE:

Managed to get gprof to work with codeblocks.  I don't think I am using the profiler correctly though, the file is missing the call graph and the times are 0 for everything. I have the -g and -pg flags enabled.