LWJGL Forum

Archive => Resolved Bugs/RFE => Topic started by: kappa on February 20, 2012, 22:52:04

Title: [FIXED] Accurate Display.sync()
Post by: kappa on February 20, 2012, 22:52:04
LWJGL's Display.sync() method has been broken/inaccurate for a while now. It doesn't do an accurate 60fps, many have abandoned it for custom solutions and you might as well use Thread.sleep() directly for similar results.

So thought I'd have an attempt at a replacement that beats it. Generally the most accurate solutions seem to be the ones that don't sleep and burn CPU using something like Thread.yield() (or nothing at all). However on the other hand sleeping is needed to stop overuse of the CPU/GPU but sleeping on its own is inaccurate.

So the solution I propose is a hybrid of the above, basically sleep as much as possible but then burn a little CPU to maintain the accuracy. Here's what I've come up with

/**
* An accurate sync method
*
* Since Thread.sleep() isn't 100% accurate, we assume that it has
* roughly a margin of error of 1ms. This method will sleep for the
* sync time but burn a few CPU cycles "Thread.yield()" for the last
* 1 millisecond plus any remainder micro + nano's to ensure accurate
* sync time.
*
* @param fps The desired frame rate, in frames per second
*/
public static void sync(int fps) {
if (fps <= 0) return;

long errorMargin = 1000*1000; // 1 millisecond error margin for Thread.sleep()
long sleepTime = 1000000000 / fps; // nanoseconds to sleep this frame

// if smaller than sleepTime burn for errorMargin + remainder micro & nano seconds
long burnTime = Math.min(sleepTime, errorMargin + sleepTime % (1000*1000));

long overSleep = 0; // time the sleep or burn goes over by

try {
while (true) {
long t = getTime() - lastTime;

if (t < sleepTime - burnTime) {
Thread.sleep(1);
}
else if (t < sleepTime) {
// burn the last few CPU cycles to ensure accuracy
Thread.yield();
}
else {
overSleep = Math.min(t - sleepTime, errorMargin);
break; // exit while loop
}
}
} catch (InterruptedException e) {}

lastTime = getTime() - overSleep;
}

/**
* Get System Nano Time
* @return will return the current time in nano's
*/
private static long getTime() {
   return (Sys.getTime() * 1000000000) / Sys.getTimerResolution();
}


Basically the code assumes that Thread.sleep(1) can be about 1ms inaccurate (errorMargin) so sleep for all the time and burn the CPU for the small errorMargin time. Nano time is used as it allows easy swapping with System.nanoTime(), in case we ever want to do that in the future.

I've been testing it for a number of days on different systems and have gotten really nice results with it (solid 60fps), much better than the current Display.sync(). Thought I'd post it here for some peer review and feedback before considering whether its a feasible solution to replace the current LWJGL sync method.

So what do you think?
Title: Re: [RFE] Accurate Display.sync()
Post by: princec on February 21, 2012, 10:19:55
I get a variance of about 5ms with sleep() on my i7. Maybe you could get it to auto-tune itself?

Cas :)
Title: Re: [RFE] Accurate Display.sync()
Post by: kappa on February 21, 2012, 10:55:58
Quote from: princec on February 21, 2012, 10:19:55
I get a variance of about 5ms with sleep() on my i7. Maybe you could get it to auto-tune itself?

Cas :)
ooh, that is actually a pretty good idea and I think very doable. I'll do some experimentation later today with auto tuning code and see what I can come up with. The above code (if you include the oversleep bit) already accounts for a variance of up to 2ms.

Just curious, the 5ms variance sounds pretty bad, what windows edition are you using? I had heard the Thread.sleep() situation had gotten better since Windows Vista+. I've tested above code on Windows XP/Pentium 4 and it does work pretty well, maybe its because its a quad core or something?.
Title: Re: [RFE] Accurate Display.sync()
Post by: princec on February 21, 2012, 12:37:20
Core i7 @ 2.6ghz with Vista64. It's pretty steady most of the time but spikes fairly consistently if irregularly by 5ms either way. I guess if you just make a note of the maximum error that sleep() gives (after say about 1000-2000 frames) and keep the threshold set to that value it'll tune itself out pretty fast.

Cas :)
Title: Re: [RFE] Accurate Display.sync()
Post by: kappa on February 22, 2012, 10:29:20
Quote from: princec on February 21, 2012, 12:37:20
Core i7 @ 2.6ghz with Vista64. It's pretty steady most of the time but spikes fairly consistently if irregularly by 5ms either way. I guess if you just make a note of the maximum error that sleep() gives (after say about 1000-2000 frames) and keep the threshold set to that value it'll tune itself out pretty fast.
Thanks for that, implemented the above and it works rather nicely on my Windows Vista machine.

It kinda breaks on the Windows XP machine, I get 1-2ms sleep and a random spike of about 3ms-5ms after every 60 frames or so. Which works fine using the auto tuning however randomly I get like one or two major spikes during the program run of 20ms+ (especially when dragging the LWJGL Display window, even seen values of up to 50-65ms) which break the auto tuning as they causing the loop to run full time using Thread.yield(). Since these big spikes happen so rarely, i would say it should be safe to ignore those by setting the auto tune code to only consider sleep values between 1ms to 10ms and ignore anything above that. What do you think?

A useful page on Thread.sleep() on XP (http://www.javamex.com/tutorials/threads/sleep.shtml) which shows that it is usually < 10ms even on a busy system.
Title: Re: [RFE] Accurate Display.sync()
Post by: kappa on February 22, 2012, 10:40:45
Just tested above and setting the auto tuning to only consider Thread.sleep() values between 1ms-10ms seems to work pretty nicely on XP at least.
Title: Re: [RFE] Accurate Display.sync()
Post by: princec on February 22, 2012, 11:18:50
Hm that seems a little kludgy. Maybe we could come up with a better heuristic - I think possibly the frequency of spikes should probably be taken into account somehow, over a short period, and picking the highest one over the last few seconds. And then continually doing that; so you need to hold the last few seconds of sleep data in a circular buffer. That way it'll keep autotuning if some other system characteristic caused it to spike a bit, and it'll return to normal after a few seconds, too.

Cas :)
Title: Re: [RFE] Accurate Display.sync()
Post by: kappa on February 22, 2012, 12:12:38
Testing out an auto tuning method that continually adapts using the following:

- Increasing the time that Thread.yield() is used by 1ms every time there is a Thread.sleep() spike greater than it (there by decreasing the time Thread.sleep() is used) this eventually tunes it to the right value after a few spikes.

- Slowly decrease the Thread.yield() time by 10 micro's when Thread.sleep() spikes are smaller than the Thread.yield() time, Any spike will push it back up again.

This seems to work pretty well (at least on XP) and continuously auto tunes the time Thread.yield() is used. So pretty useful especially if the System becomes more or less busy. I'll run a few more tests on some different Systems to see if it holds up.
Title: Re: [RFE] Accurate Display.sync()
Post by: spasi on February 22, 2012, 18:28:58
Quote from: kappa on February 21, 2012, 10:55:58Just curious, the 5ms variance sounds pretty bad, what windows edition are you using?

I don't think it's the OS, but the CPU going into a deep sleep state. If we assume a Display.sync()ed Puppygames game and nothing else heavy running in the background, it's probably quite likely that an i7 will go to sleep often. So the spikes are due to the CPU waking up cores rather than some OS scheduling weirdness. Cas, try running a heavy process in the background and see if the spikes go away.

kappa, two random ideas you could try:

- Rotate between Thread.sleep(1) and yielding in the sync loop, instead of sleeping all the way to the burn time threshold. This could deter the CPU from going to a sleep state.
- Use LockSupport.parkNanos() instead of Thread.sleep(). It's supposed to have lower latency (probably not on Windows though).
Title: Re: [RFE] Accurate Display.sync()
Post by: spasi on February 22, 2012, 18:46:57
Another idea: Spikes could be caused by the thread getting scheduled to a different core after the sleep call. You could try setting the process affinity (through the Windows task manager) or thread affinity (Java Thread Affinity (https://github.com/peter-lawrey/Java-Thread-Affinity)) to a single core and see if it fixes the spikes.
Title: Re: [RFE] Accurate Display.sync()
Post by: kappa on February 24, 2012, 12:44:53
@spasi thanks for that explanation, will test out the alternating sleep and yield() idea. Not tried LockSupport.park*() methods but might be worth investigating.

I've got the adaptive code to work really nice now, seems to work smoothly and accurately on all the machines I've tested on (including XP) while using minimal yielding by continually auto adapting to the system its running on.

/**
* Get System Nano Time
* @return will return the current time in nano's
*/
private static long getTime() {
   return (Sys.getTime() * 1000000000) / Sys.getTimerResolution();
}

static long variableYieldTime;

/**
* An accurate sync method that adapts automatically
* to the system it runs on to provide reliable results.
*
* @param fps The desired frame rate, in frames per second
*/
public static void sync(int fps) {
if (fps <= 0) return;

long sleepTime = 1000000000 / fps; // nanoseconds to sleep this frame
// yieldTime + remainder micro & nano seconds if smaller than sleepTime
long yieldTime = Math.min(sleepTime, variableYieldTime + sleepTime % (1000*1000));
long overSleep = 0; // time the sync goes over by

try {
while (true) {
long t = getTime() - lastTime;

if (t < sleepTime - yieldTime) {
Thread.sleep(1);
}
else if (t < sleepTime) {
// burn the last few CPU cycles to ensure accuracy
Thread.yield();
}
else {
overSleep = t - sleepTime;
break; // exit while loop
}
}
} catch (InterruptedException e) {}

lastTime = getTime() - Math.min(overSleep, sleepTime);

// auto tune the time sync should yield
if (overSleep > variableYieldTime) {
// increase by 200 microseconds (1/5 a ms)
variableYieldTime = Math.min(variableYieldTime + 200*1000, sleepTime);
}
else if (overSleep < variableYieldTime - 200*1000) {
// decrease by 2 microseconds
variableYieldTime = Math.max(variableYieldTime - 2*1000, 0);
}
}

/**
* Get System Nano Time
* @return will return the current time in nano's
*/
private static long getTime() {
   return (Sys.getTime() * 1000000000) / Sys.getTimerResolution();
}
Title: Re: [RFE] Accurate Display.sync()
Post by: kappa on February 29, 2012, 23:24:02
above is now committed and should be part of the next nightly build.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 01, 2012, 22:01:29
This new sync with Thread.yield is sometimes giving me 30-70% cpu usage.
(with a simple empty gameloop and with a more "advanced" loop)
On a 3Ghz C2D.
OS: mac osx
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 01, 2012, 23:10:57
Thanks for reporting, I've tweaked the Display.sync() method further, it had a minimum Thread.yield() of 1ms of the total sync() time, I have reduced this down to 0 now, so if there is a perfect Thread.sleep() there will be no Thread.yield()'ing at all. Do test the next nightly build of LWJGL.

Some testing here shows CPU usage to be 1% or below with a basic game loop. The sync() method will sleep as much as possible and only yield for the amount of time by which Thread.sleep() is inaccurate (0.2ms here on my linux system), so Display.sync() should still use Thread.sleep() for most of its duration.

Just curious, how are you measuring CPU usage?
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 02, 2012, 11:59:28
With atMonitor and the built in activity monitor.
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 02, 2012, 12:19:25
Any difference with the latest nightly build of LWJGL? also can you profile your lwjgl app to see where most of the time is being spent and if indeed in the sync() and Thread.yield() methods. The sync() method should really be sleeping for most of its time (especially on mac), so really shouldn't be using 30%-70% of the CPU.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 02, 2012, 15:35:04
I'm going to test/profile it on sunday!
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 04, 2012, 16:07:10
56% in Display.sync()
15% in swapBuffers
4% in getTime
...

Tested it with the latest build.
I noticed that this high % problem happens when the app window is out of focus and I do something like exposé! (or anything in the background)
And the % will go up to 90 and slowly normalize to 1-2%. (really slowly)
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 04, 2012, 23:22:18
hmm, that is really odd, can't reproduce that on Window or Linux. Could be a OS X specific issue, will see if I can reproduce the problem on an OS X machine.

Can't really see how Thread.sleep(1) mostly in the method could cause so much CPU usage when the window becomes unfocused (the code doesn't have anything to do with window focus).

Does the problem happen with the previous Display.sync() that was used by LWJGL? There was an odd looking "synchronized (GlobalLock.lock)" in that method which might have been a fix for this issue.
Title: Re: [FIXED] Accurate Display.sync()
Post by: spasi on March 05, 2012, 01:11:52
Obsyd: Could you please post your render loop code? It might help us reproduce the issue.

kappa, the new sync code works great here. The LWJGL gears sample with sync(60) runs at a solid 299 frames / 5 seconds (59.8 fps), with 2% CPU usage.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 05, 2012, 17:22:19
Non thread.yield versions(2.8.3) work fine. (1-2%)
A really simple loop. All it does is randomly draw 96 quads.

while(Display.isCloseRequested() == false && Keyboard.isKeyDown(Keyboard.KEY_ESCAPE) == false){

benchmark.benchmarkrender();

Display.update();
Display.sync(60);
}

public static void benchmarkrender() {
        glClear(GL_COLOR_BUFFER_BIT);
        for (int i = 0; i < 96; i++) {

            int x = veletlen.veletlenfv(560) + 4;
            int y = veletlen.veletlenfv(400) + 4;

            init_textures.benchmark.bind();
            glBegin(GL_QUADS);
            glTexCoord2f(0, 0);
            glVertex2f(x, y);
            glTexCoord2f(1, 0);
            glVertex2f(x + init_textures.benchmark.getTextureWidth(), y);
            glTexCoord2f(1, 1);
            glVertex2f(x + init_textures.benchmark.getTextureWidth(), y + init_textures.benchmark.getTextureHeight());
            glTexCoord2f(0, 1);
            glVertex2f(x, y + init_textures.benchmark.getTextureHeight());
            glEnd();
        }
    }
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 18, 2012, 16:33:36
I've had a chance to test this on OS X now and can confirm your issue. It seems that any form of Thread.yield() or even an empty while loop (for even a ms or so a frame) burns excessive amounts of CPU on OS X (much more than on Windows or Linux). However the OS X Thread.sleep() timer seems much more accurate that both Windows and Linux.

I've adjusted the Display.sync() method now to fix this issue and CPU usage should be back to normal on OS X. Please do test and confirm with the next nightly build of LWJGL.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 19, 2012, 03:06:50
The new Display.sync seems perfect :)
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 19, 2012, 04:25:27
Well, it is perfect on OSX, but I booted up a win7 on my iMac and the same problem happened as before.
As before, if the window is out of focus and you are doing things in the background (browsing the net,or grab the window and toss it around a little), the CPU percentage goes up and when the window regains focus it will SLOWLY start to drop to 0-2%.
Goes up to 30-40%.
Again, simple loop and advanced loop.

I think thread.yield is not the best way.
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 19, 2012, 17:23:40
hmm, something sounds really odd, not really sure how having the window focused or not should change the amount of CPU being used by Display.sync(), wasn't able to reproduce that on OS X yesterday or on Windows and Linux.

Quote from: Obsyd on March 19, 2012, 04:25:27
I think thread.yield is not the best way.
Do keep in mind that Thread.yield() should only be running for a small fraction of the Display.sync() time with the majority of time being spent in Thread.sleep().

Also it seems to work fine for you while the Display is focused, so using 30-40% of the system resources while not focused sounds way too excessive, something is seriously wrong somewhere.

One theory that I can think of which might cause the above is that when the Display is not focused then somehow Thread.sleep(1) becomes massively inaccurate and the Display.sync() method increases the usage of Thread.yield() to try to compensate, which would explain the above behaviour. I suppose a potential fix could be to only use Thread.yield() while the Display.isActive() (and thus lower the accuracy when not focused).

However I haven't been able to reproduce the above behaviour after testing on multiple computers and OS's. Can anyone else confirm the above problem?

What type of CPU are you using?
What Java version are you running on windows?
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 19, 2012, 20:13:45
Tested it on a friends laptop.
Win XP
CPU: C2D 2.0Ghz


http://www.youtube.com/watch?v=SjgdnnLRQVU
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 19, 2012, 23:04:32
Ah right, thanks for that. I know what the issue is now and can reproduce it. Basically what happens is when you minimise and maximise the windows randomly, like in the video, or even hold and drag the window (basically any windows event that freezes the lwjgl app), the Thread.sleep(1) massively oversleeps and the auto tuning code increases the rate of Thread.yield() use in an attempt to adapt to the varying timer (until it maximises to a point where its running for the full Display.sync()).

If you do leave the window stable for a while, you'll notice the CPU usage slowly decrease back to normal (can take like 30+ seconds with the current setting). However it decreases too slowly but it does decrease, I've got a fix for it now, however need to test on a few different systems before committing.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Simon Felix on March 21, 2012, 19:33:10
Compiled our game with yesterdays nightly build. Today the players said the following:
4x "super smooth" (XP, Win7)
1x "smooth for a few secs, then not smooth again" (Win7)

The one who said "smooth for a few secs, then not smooth again" said it's currently unplayable. The game tries to sync to 120fps, previous version ran at 125fps, the current version runs at 110-120fps on his computer. To me it appears that the new code is over-adjusting, then backing off, then over-adjusting again, etc.. Could that happen?

Maybe you could adjust the code so that it always uses yield for the last few ms, for extra safety. And btw: Limiting the yield-time to 15ms (or so) might be useful too, just to avoid extreme cases.
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 21, 2012, 19:41:50
@Obsyd do test next nightly build, added a fix to avoid the excessive CPU using on windows now, so should adapt much faster now and be a lot more robust.

Currently experimenting with Riven on a new better adapting algorithm, will see what comes from that, early results from new algorithm are really promising.

@dr_evil yeh yield time is capped to handle extreme cases, but do test the latest changes.
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 21, 2012, 19:48:05
Quote from: dr_evil on March 21, 2012, 19:33:10
The one who said "smooth for a few secs, then not smooth again" said it's currently unplayable. The game tries to sync to 120fps, previous version ran at 125fps, the current version runs at 110-120fps on his computer. To me it appears that the new code is over-adjusting, then backing off, then over-adjusting again, etc.. Could that happen?
Yeh its theoretically possible, if that is indeed the case then it needs to be tweaked a little more but i'll run a few tests with higher fps like 120fps, so far mostly just been testing with 60fps.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Simon Felix on March 21, 2012, 20:56:34
We run at 120fps to
a) make our players happy and
b) improve responsiveness

Competitive Tetris is serious business... ;)
Title: Re: [FIXED] Accurate Display.sync()
Post by: princec on March 22, 2012, 08:33:43
But almost every single display out there available except the odd ancient CRT can only do 60Hz...?

Cas :)
Title: Re: [FIXED] Accurate Display.sync()
Post by: CodeBunny on March 22, 2012, 12:32:09
Or 75 fps. I've seen that before. However, people usually have to change monitor settings to enable that, and that usually never happens.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Simon Felix on March 22, 2012, 13:15:21
I first thought too that switching to 60fps should make no difference, since we run the logic at (fixed) 400fps anyway and poll the keyboard more often anyway (Display.processMessages()), not just once per frame. But the game certainly feels more responsive and accurate at 120fps. Did some experiments with our pro-gamers to confirm.

In all cases the lag from "Input -> Rendering -> Screen" should be shorter when you let rendering run more often, in our case half a frame (1/120th).
Title: Re: [FIXED] Accurate Display.sync()
Post by: spasi on March 22, 2012, 13:48:36
Just a heads up, the Nvidia R300 drivers released today should support EXT_swap_control_tear (http://www.opengl.org/registry/specs/EXT/wgl_swap_control_tear.txt). You can use it by passing -1 to Display.setSwapInterval(). Unfortunately there's no way to detect the presence of a WGL/GLX extension in LWJGL right now, this is something that we plan to fix in 3.0.

EXT_swap_control_tear will synchronize to the monitor refresh rate if rendering is faster and it will swap without synchronization if the frame rate drops below it. Hence you get the best of both worlds and there's no need for Display.sync(). From the spec: "This reduces the visual stutter on late frames and reduces the stall on subsequent frames". I remember John Carmack pushing for this functionality a few years ago, lets hope we'll see support for it from AMD as well. Edit: "Shipping with NVIDIA Forceware 285.62 and AMD Catalyst 11.10 drivers.", it's available on my Radeon and it works great.
Title: Re: [FIXED] Accurate Display.sync()
Post by: princec on March 22, 2012, 14:03:27
Interesting, so that should basically solve the problem of 60fps... 60fps... 60fps... 30fps, where performance suddenly halves? (Or, heh, 45fps, should you perhaps have triple buffering)

Cas :)
Title: Re: [FIXED] Accurate Display.sync()
Post by: spasi on March 22, 2012, 14:17:23
Yes. Nice explanation here (http://www.geforce.com/whats-new/articles/introducing-the-geforce-gtx-680-gpu?sf3580904=1/), under "Genuinely Smoother Gameplay".
Title: Re: [FIXED] Accurate Display.sync()
Post by: Mickelukas on March 22, 2012, 14:44:12
Quote from: princec on March 22, 2012, 08:33:43
But almost every single display out there available except the odd ancient CRT can only do 60Hz...?

Cas :)

Several new screens do 120Hz to be able to do active shuttering 3D. If you have such a screen I'm guessing that you have a new enough gfx card/driver to support vSyncing though :)

Mike
Title: Re: [FIXED] Accurate Display.sync()
Post by: Obsyd on March 22, 2012, 17:52:30
Tested the new code. It definitely does not eat the cpu as earlier builds do, but it still can do 14% (slowly goes down to 2-4%) on a C2D 3.06Ghz on win7 (2-4% idle). (simple loop,drawing a few triangles) The 2.8.3 display.sync is not as accurate, but it uses only 0-1%.
So this is a step in the right direction :)
Title: Re: [FIXED] Accurate Display.sync()
Post by: CodeBunny on March 23, 2012, 12:47:05
How well does it perform if the game is very CPU-intensive (i.e., already close to 95% or so)? I assume it won't be a problem, just wanting to make sure.
Title: Re: [FIXED] Accurate Display.sync()
Post by: matheus23 on March 23, 2012, 13:09:58
Okey, this is completly cracy :D
60.0000 FPS all the time :)
this self-optimizing thing is perfect :)

EDIT: 5% CPU Usage on both cores with an amd athlon 2.55GHz CPU, with only 1 Thread woking at once (but I think thats only because I'm on Archlinux, and linux switches between the cores time to time...)

EDIT2: Idle: ~2% CPU usage.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Simon Felix on March 23, 2012, 22:53:14
Oh, by the way: I run only the game at 120fps and switch to/from 60fps for the menus. Just in case somebody's looking for test cases... :)
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 24, 2012, 00:51:17
Display.sync algorithm completely rewritten again (thanks Riven), now works by calculating the average sleep and yield times from the last few frames and adjusts accordingly. Overall much better, uses less yield, more accurate, adapts faster and generally just badass now :)

Do test with the next nightly builds.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Simon Felix on March 26, 2012, 19:18:08
Have thought about using the maximum (instead of average) sleep time? Makes more sense to me.

Pro tip: Store the sum of all elements in addition to the individual values in RunningAvg. Allows to calculate the average in constant time.
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 26, 2012, 19:35:55
Quote from: dr_evil on March 26, 2012, 19:18:08
Have thought about using the maximum (instead of average) sleep time? Makes more sense to me.
Yup, did try that (see earlier in the thread), however it doesn't work too well as it mostly ends up using yield most of the time, since you can get random spikes with sleep that can be pretty large which mess up the calculation (silly Windows XP can have spikes as large as 60ms). In any event the current method works pretty well and is the most accurate so far.

Quote from: dr_evil on March 26, 2012, 19:18:08
Pro tip: Store the sum of all elements in addition to the individual values in RunningAvg. Allows to calculate the average in constant time.
Yeh, had that in there before (http://pastebin.com/Y5b2Ztys) but was changed due to the dampening code not working too well, though when benchmarked its pretty insignificant when it comes to overall time spent in sync() (takes maybe a dozen nano's) with most time being spent in the getTime() method. However its a valid optimisation and might be worth tweaking in. Thanks for taking a look.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Riven on March 26, 2012, 21:27:50
Quote from: kappa on March 26, 2012, 19:35:55
However its a valid optimisation and might be worth tweaking in.

I'd like to stress that dampening is impossible if the 'sum' is an aggregrated value (used for calculating 'avg'). It will leak time, and can't recover. Look very carefully at the code, the dampening is applied to the values in the slots, which means that the dampening will (slowly) disappear after 10 frames. This handles both erratic delays and poor resolution timers. Please don't optimize for optimizations-sake. The current code is correct and extremely fast. The 'correct' part is most important.

It can be a few nanoseconds faster, indeed, but at the expense of reliability and correctness! There are enough screwed up sync( ) implementations, I'd prefer to keep this implementation correct.

Don't get me wrong, dr_evil: if you think your implementation is better, taking into account a vast number of corner cases, I'd be very interested.
Title: Re: [FIXED] Accurate Display.sync()
Post by: kappa on March 26, 2012, 21:41:11
Agreed, happy with the way the code is now but yeh improvements are always welcome.

However as it stands unless there is some serious issue found I'd say this RFE is pretty much complete.
Title: Re: [FIXED] Accurate Display.sync()
Post by: Simon Felix on March 26, 2012, 23:17:44
Agreed, it won't make much difference. I proposed to have the sum in addition to the slots array. Just for completeness, here's the proposed implementation:

135        private static class RunningAvg {
136                 private final long[] slots;
                        private long sum;
137                 private int offset;
138                
139                 private static final long DAMPEN_THRESHOLD = 10 * 1000L * 1000L; // 10ms
140                 private static final float DAMPEN_FACTOR = 0.9f; // don't change: 0.9f is exactly right!
141
142                 public RunningAvg(int slotCount) {
143                         this.slots = new long[slotCount];
144                         this.offset = 0;
                                this.sum = 0;
145                 }
146
147                 public void init(long value) {
                                this.sum = this.slots.length * value;
148                         for (int i = 0; i < this.slots.length; i++) {
149                                 this.slots[i] = value;
150                         }
151                 }
152
153                 public void add(long value) {
                                this.sum += value - this.slots[this.offset];
154                         this.slots[this.offset++] = value;
155                         this.offset %= this.slots.length;
156                 }
157
158                 public long avg() {
163                         return this.sum / this.slots.length;
164                 }
165                
166                 public void dampenForLowResTicker() {
167                         if (this.avg() > DAMPEN_THRESHOLD) {
                                        this.sum = 0;
168                                 for (int i = 0; i < this.slots.length; i++) {
169                                         this.sum += (this.slots[i] *= DAMPEN_FACTOR);
170                                 }
171                         }
172                 }
Title: Re: [FIXED] Accurate Display.sync()
Post by: Riven on March 26, 2012, 23:32:39
You're basically tracking time using two seperate mechanisms.
Yes, it works, and it's marginably faster, but you trade maintainability.
I'd prefer KISS, until problems arise.

If the Running Average was wildly over 10 samples, I'd say great, as then it would matter, but in this case it's a very specialized piece of code, as proven by the appearance of the naughty .dampenForLowResTicker() method. :)
Title: Re: [FIXED] Accurate Display.sync()
Post by: Simon Felix on March 27, 2012, 17:57:54
Got very positive feedback from our playerbase regarding the newest code. Thanks a lot!
Title: Re: [FIXED] Accurate Display.sync()
Post by: Riven on March 27, 2012, 21:14:49
Thanks!