News:

The Forum Rules and Guidelines
Our forum has Rules and Guidelines. Please, be kind and read them ;).

Citycars and performance?

Started by jamespetts, April 10, 2012, 12:59:55 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

jamespetts

#35
That's a good question. It really doesn't look like there are that many cars: the roads look fairly normal in the game, so that figure is a little odd. I'm wondering whether the issue is stacked cars (has this been fixed on Standard yet)?

Edit: This is a particularly crowded area - most places are less crowded than this. Car stacking is evident on the diagonal road in the centre:

Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

jamespetts

I think that I have worked out what is going on here. It was a combination of a bug and some inefficient coding. As a result of a bug, the origin data of city cars were not saved. Whenever the game was loaded, all city cars would be put on the "unassigned cars" list and not into individual city lists. Consequently, this list became very large.

Then the inefficient coding took effect. In karte_t::neuer_monat(), there is a method to check that the number of cars on the unassigned cars list does not exceed the number that should be in the game, and remove any excess ones. It worked by running a while loop (the condition being while the count of the unassigned cars list was greater than the number of outstanding cars that should be in the game), removing the first car from the list and invoking a method which set its time_to_life to zero, meaning that it would be deleted in the next sync_step. It did not delete it directly, nor set the list pointer in the car objects themselves to NULL.

When the sync_step was invoked, the cars would be sent for deletion, but, because the list was not marked as NULL, the remove method on the unassigned cars list would be invoked; but the cars were not actually in the unassigned cars list, because they had been removed in the method described above, so the entire (very long) list would have to be searched through, with no success, in every destructor call, which is what, I imagine, was taking all of the time.

I have now fixed this on the -devel branch, first by fixing the saving of the origin data, so that cars' origins (and therefore correct lists) are stored, meaning fewer cars in the unassigned list. Secondly, instead of setting time_to_life=0 when excess cars are detected, their list pointer is set to NULL, they are removed from the sync list by karte_t::sync_remove(), and then deleted. When they are deleted, because their list pointer is NULL, there is no attempt to iterate through any lists, and so deletion is much faster. (The remove methods in the destructor are still called when cars are deleted manually or by natural time expiry).

Testing shows that this radically improves performance: running the no cars version of the server game from 1979 and increasing the traffic level to 11 again, good performance is maintained for many game months with perfectly acceptable responsiveness despite a large number of citycars reappearing in the game.

Thank you everyone for assistance with this topic, especially Prissi for suggesting profiling, which eventually lead to the solution to the issue.
Download Simutrans-Extended.

Want to help with development? See here for things to do for coding, and here for information on how to make graphics/objects.

Follow Simutrans-Extended on Facebook.

Carl

Very glad to hear that you found a fix. I think I speak for everyone when I say we're very grateful for your hard work on this!