Game is stuttering #4963
Comments
|
The speed is the same as it used to be (debug builds on large maps tend to be very slow), but it is more noticeable now than before due to multithreading. In 1.0, Widelands simply hangs for a brief (more or less) interval every time the game state is progressing by a certain number of milliseconds; whereas now the game is redrawn all the time, which has the side-effect that the continuous alterations in the maximum game speed your CPU can actually handle (and which may be lower than the desired speed setting), as well as the delays between a click and its effect, are more noticeable. The largest portion of those slow-speed periods are filled by the AI – which can not be reasonably multithreaded – and the economy routing code – which I intend to make multithreading-compatible in the near future which will improve performance. |
|
I simply do not understand enough to know what has to be done to make the Ai thread safe. Discussing such things would be one big benefit of the proposal of @klaus-halfmann to have a devcon. So if somebody could teach me some principles I might help with my understanding of the AI code |
|
The AI could be run pretty much independent from everything else already except for one thing: The state of the game might change while the AI is analysing it. For example look at any piece of AI code: Lines 4038 to 4062 in f305643 With the current implementation, the game logic freezes while the AI's code is running, guaranteeing that the game state is the same at all lines. But if we put the AI into its own thread, then it may change at any point while the AI is running. For example, in this code snippet flag is initially valid, but if this flag happens to be deleted e.g. because another player conquers some land then it may become a dangling reference at any moment so it may e.g. be still valid in line 4046 and suddenly segfault in line 4053. The gametime reference will also keep changing its value.
The solution is to add an appropriate widelands/src/wui/inputqueuedisplay.cc Lines 385 to 406 in f305643 This lock ensures that no changes related to map objects will happen while the function is running, so we can guarantee that the building pointer does not suddenly turn invalid at some point. The problem with the current AI code is that is uses object pointers that stay in existence for a very long time. It would be necessary to create large-scoped mutex locks, which would make the thread so inefficient that having a separate thread would offer virtually no benefits in the first place. So the code would need to be restructured to use If you like I can create a branch to give the AI its separate thread and you can then look at the crashes and fix each one symptomatically until there are none left… which is pretty much what I did to make the rest of the codebase threadsafe :D |
Noordfrees
added
memory & performance
|
I would appreciate If you could create such a branch for me. Although I don't know how far I will get with it. At least I might learn something. Furthermore I wanted to try to restructure the AI code anyway to make it more readable as it is currently to unstructured imho. |
|
Branch is up: https://github.com/widelands/widelands/tree/multithread-ai Edit: The AI also uses some local variables declared |
|
ok thanks I will have a look. |
|
Ok I am in Progress in understanding the concept. |
|
The only mutex you should need for the AI is the A normal pointer just points to an address in memory at which an object is located. Such a pointer does not "know" if this memory is freed by another thread, so a pointer that is valid at some time may point to invalid memory later, resulting in a heap-use-after-free. void do_something(Building* b) {
b->some_function();
do_something_else_for_a_long_time();
b->some_function();
}This code above is problematic, because building You can use a mutex lock, if you need to be absolutely certain that void do_something(Building* b) {
MutexLock m(MutexLock::ID::kObjects);
b->some_function();
do_something_else_for_a_long_time();
b->some_function();
// `m` goes out of scope here
}This has the disadvantage that the lock stays in existance for a long time. The better solution is to use void do_something(OPtr<Building> b) {
{
MutexLock m(MutexLock::ID::kObjects);
if (Building* bld = b.get(game())) {
bld->some_function();
}
// `m` goes out of scope here
}
// There is no active lock any more now
do_something_else_for_a_long_time();
{
MutexLock m(MutexLock::ID::kObjects);
if (Building* bld = b.get(game())) {
bld->some_function();
}
// `m` goes out of scope here
}
} |
|
thanks for the explanation. |
|
ok i found out how to use gdb and get the following crash although it should be safeguarded by a Mutex. I don't understand this |
|
At a quick glance you seem to create the mutex lock after upcasting |
|
Well Moving the mutex did work. I even realized this was more performant then having it further down. So sometimes a bigger focus of a mutex seems to be better. |
|
Well I got a full match on crater running until one AI won, without using any mutex and quite nice performance |
|
I'm curious about the progress on this. FWIW, I got a huge gamespeed performance boost (in a situation where max_fps was set to 99 and actual rendering was happening at around ~40fps; previously the game would barely keep up at just about 1-2x speed, now 10x was okay) by setting widelands/src/base/multithreading.cc Line 159 in 41a4d8a Quick look at profiling seems to indicate that the logic thread often takes 2 or 3 30ms attempts to obtain the contentious kObjects mutex, now it typically gets it in at most ~20ms. |
well, I gave up on the Ai multithreading for the moment being, I could not prevent it to crash, and mutexing the relevant code was slower then the current implementation. |
The locking interval is defined per mutex, not per thread. i.e. widelands/src/base/multithreading.cc Line 160 in 41a4d8a widelands/src/base/multithreading.h Lines 90 to 91 in 41a4d8a widelands/src/logic/map_objects/bob.cc Line 186 in 41a4d8a |
|
Hi, widelands.campaign.ship.launch.bug.mp4 |
|
Hi, this is indeed the wrong place, the best place is to open a new issue with the bug report template at https://github.com/widelands/widelands/issues/new/choose |
After long time not playing i started a new game today. But i am a bit disappointed about the speed... The game noticeable stutters some time. Animals and carriers are walking not with the same speed during a single walk. Placing a building can last a while (click a building to place, but after some noticeable milliseconds the construction appears on the map) and it seems to me that even the mouse does not react smooth.
This is most noticeable when setting Max-FPS to 99. Setting this to a value of 30 the game reacts a little bit smoother, but playing at 2x speed it feels like the game speed switches between 1x and 2x speed in irregular intervals.
To Reproduce
What i have done:
Right after start one can see the Animals stumbling around. But i remember this was also in versions before releasing 1.0 (the versions i played before this time where all before version 1.0, but always current master).
Expected behavior
A smooth game play.
Version:
Maybe because i use a debug build? This wasn't a problem so far, except on really big maps. Will also test a release build soon.
The text was updated successfully, but these errors were encountered: