Why not using vector_tpl for all sync lists? There is still this list:
ptrhashtable_tpl<sync_steppable *,sync_steppable *> sync_eyecandy_list;
Given the recent problems with ptrhashtable , I wonder why these errors did not pop up earlier. What speaks against changing this list into a vector_tpl? This sync list contains animated buildings (and buildings with construction site graphics). Building sites will after some time delete themselves from the synclist. This deletion is costly, hence the hashtable. However, changing the return type of syncstep to an enum containing three values: 0 - delete me, 1 - take me off lis, 2 - keep me on list - would help to speed this up. (Currently, syncstep returns false - delete me, true - keep me on list).
Any thoughts on this?
I would also like to cleanup the dead code that could be activated by compiler swithc SYNC_WAY_HASHTABLE, SYNC_WAY_LIST, and SYNC_VECTOR.
Problems with the comp function doesn't cause problems until there is a hash collision, which should ideally be rare to the point of near non-existing.
I've already done everything you mention. It's all tangled up in that 'expermental' meandering try everything multithreaded sync_step patch I was working on a while ago - been 54 weeks months since I even updated it to trunk, probably 2 years since I tried working on it - whenever those blasted translation patches were going on...
All my benchmarking showed vector_tpl is by far the best for all types of sync objects. I'd also split the sync lists to have one per object type, much better cache utilization stepping all of one type of object before the next.
I could probably extract the simple sync_step stuff out from the rest of the experimental patch junk in a couple weeks...
I assume the reason the sync list is a hash table is to avoid having the same object appear multiple times in the list.
No, I believe it was thought to be faster due to the number of deletions being performed on the eyecandy objects. vectors, benchmark faster. especially with a couple mods to the sync routines to prevent list searches when deleting.
Here is a patch that cleans up the sync lists. All lists are now derived from a simple synclist class, which is a vector_tpl inside and handles all the sync-step tasks. In addition, sync_step of objects returns three different values: (ok, remove from list, delete and remove). This makes the add_lists and remove_lists obsolete, as adding and removing of arbitrary sync-objects does not happen during sync steps.
I do not get the distinction into three lists, which are now: vehicles + transformers + traffic lights, animated buildings, and smoke. The latter two can be merged, and even run mutlithreaded, but these lists are rather small compared to the main list.
How does Simutrans with the patch compare to Simutrans without the patch when profiled? It would be instructive to compare the speed, I think.
Did some testing, but the improvement was rather insignificant: 10micro-seconds per syncstep in fast-word of yoshi game while one sync-step needed ~1.6 milli seconds.
There are often many short lived pedestrians in the eyecandy list. The can make up a good part of all sync_stepobject. I tried to balance the size of all three lists to speed up adding to lists and removing.
So I am for inclusion.
If order isn't important and the element type is something as simple as a pointer, it might be faster to do removal by overwriting the position of the element to remove with the last element, and then doing pop_back (or equivalent).
this is now committed with r7762/7763