[fixed] Fix the linux yield logic

[hendu]

Sleeping for zero [unit]s is considered wrong, and can result in unexpected results.

Patch not posted to the tracker, because it's just changing one number. Please apply to 1.7 as well.

diff --git a/source/Irrlicht/CIrrDeviceLinux.cpp b/source/Irrlicht/CIrrDeviceLinux.cpp
index 8d2d6c1..4f52bd9 100644
--- a/source/Irrlicht/CIrrDeviceLinux.cpp
+++ b/source/Irrlicht/CIrrDeviceLinux.cpp
@@ -1121,7 +1121,7 @@ bool CIrrDeviceLinux::run()
//! Pause the current process for the minimum time allowed only to allow other processes to execute
void CIrrDeviceLinux::yield()
{
- struct timespec ts = {0,0};
+ struct timespec ts = {0, 1};
nanosleep(&ts, NULL);
}

[CuteAlien]

Are you certain? The documentation of nanosleep which I found says the valid range is 0 to 999999999.

[hendu]

It is a valid number, but it might not do what you expect, ie let the cpu idle. It is completely legal for it both to return immediately without freeing the cpu, or to sleep for several seconds.

Changing it to 1 means the first case can't happen anymore, some sleeping is guaranteed.


[ though, in my opinion using this call is bad design anyway, it should still be fixed ]

[CuteAlien]

OK, I'll patch it in the evening when I'm back on Linux.

Not sure why you think it's bad design - seems better to me than not yielding at all. I mean you should give free as much as possible, but unfortunately with applications like games needing to refresh all the time you can't behave perfectly nice like other desktop applications.

[hendu]

Now on to the design discussion

If you're going to sleep, you should do a proper fps cap be it 60, 30, or 125. Hogging all cpu and then pretending to be nice by only using 99.5% instead of 100% has a negligible effect. In fact it's usually unnecessary on any modern system from a scheduling point of view, the kernel can well suspend it without a yield if something else has more privileges.

If there's something that absolutely needs to happen more often than screen refreshes, that's what threads are for.

[CuteAlien]

Hm, makes a rather noticeable difference on my system if a yield is in there or not. I agree that hitting screen-refresh-rates is certainly better (although not so trivial doing that with sleep() exactly).

[hendu]

It's only a couple lines of code, I think many examples have been posted on these forums too.

[CuteAlien]

You mean for sleeping exactly? I don't trust examples I haven't tested myself ;-) The thing is - 30,60 fps only makes sense if you hit the sync - otherwise you suddenly lose half your framerate. And if you just sleep at the end (as most code I've seen was doing because they just replaced the yield() from the examples) and then run your calculations chances are good your calculations will take a little too long (hard to predict) and you exactly miss the sync. Not sure what the right solution would be - probably calculating the time before the endScene and adding the sleep there.

[hendu]

Adaptive sleep is quite ok, works well with the commands at the end of frame. There is no issue with a frame taking longer than 16ms, in that case you simply don't sleep.

Pseudocode for 60fps:

Code: Select all

 
#render
 
newtime = getrealtime()
sleeptime = 16 - (newtime - oldtime)
if (sleeptime > 0) sleep(sleeptime)
oldtime = getrealtime()
 

[CuteAlien]

End of frame... before endScene() I guess? And maybe add a ms to avoid hitting the update exactly (although I guess 16 is probably fine as it's 16.6666 ) . Would have to tests to be certain - but yeah - maybe it would be nicer to use something like that in the examples - or at least in one of them to show it.

edit: oldtime probably after endScene - while newTime should be checked before it.

[hendu]

It's all after endscene in my current project, as the last thing in the frame loop.

That way, if vsync was used (fullscreen), endscene blocks until the vertical trace and there may not be sleep anyway. It's the windowed modes where you need to sleep to save cpu.

[CuteAlien]

Ah yes, right.

[CuteAlien]

I've searched around some more and indeed found no documentation on behaviour with 0 nanoseconds (from what I understand it will behave different depending on kernel-version and some high-resolution blabla thingy). So changed now in 1.7, trunk will follow on next merge.

Edit: Btw. it seems yield is usually used when the application is not active - not to yield for a steady framerate.

[hendu]

Even in that case it would be better to sleep say 16ms than one nanosecond

[hybrid]

Well, if you want to have your app to sleep instead of yield, then call sleep, not yield. I don't see the point here. The yield function is not thought to reduce the CPU usage, it's just to keep the other processes running on the same system responsive. For this reason, a nice app should give the scheduler a go from time to time. And that already works with nanosleep(0).