[FIXED] Accurate Display.sync()

[Riven]

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.

[kappa]

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.

[Simon Felix]

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:

Code:

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                 }

[Riven]

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.

[Simon Felix]

Got very positive feedback from our playerbase regarding the newest code. Thanks a lot!

[Riven]

Thanks!