TPS metric is inaccurate

[gp-Airee]

I understand this plugin isn't officially supported on 1.8, but the metric always reads 20. It'd be nice if it worked on 1.8 too!

[finlaysawyer]

I also have this issue on the latest version of Paper 1.14.4, it only seems to send the correct TPS every now and again.

[BirkhoffLee]

Same here, always reads 20.

[sladkoff]

What are you guys comparing the TPS to?

As far as I remember I re-used code from LagMeter while it was still open source and on GitHub. I was testing the TPS metric against the LagMeter /tps command.

I will keep this open and look into it when I get the time.

[BirkhoffLee]

After testing it does drop to like 14 or something, but it’s mostly inaccurate comparing to what I see in the output of in-game /tps. I also use React to monitor server stats in-game, and the TPS stat from that just differs a lot from this plugin.

[phemmer]

Was also seeing unexpected values out of this measurement, and took a look at the code. The issue is how TPS is being calculated. On TpsCollector.java#32 we have:

long timeSpent = (now - this.lastPoll) / 1000;

What this is doing is taking the amount of wall clock time between now and 40 ticks ago, measured in milliseconds, and then truncating it down to seconds.
So lets say we've got a TPS of 15. Then 40 ticks would take 2,667ms. The code takes that and divides by 1000, so it becomes 2.667. Then it converts that to a long, and it becomes 2. Just 2. So now the code thinks that the last 40 ticks took 2 seconds, instead of 2.667 seconds. It then divides the number of ticks (40) by that value (2) and gets a TPS of 20, instead of 15.

For this to show something other than 20, the TPS has to drop to such a low rate that the truncation produces a different value. In practice, this means the TPS has to drop down to about 13.

This should probably be reclassified as a bug, not enhancement.

Edit: another minor nitpick, is that in getAverageTPS(), instead of the code lying, and saying 20TPS when it doesn't have a full buffer, just use what it does have. If it only has 2 samples instead of 10, just use those 2 for an average. The value will be more accurate than a constant 20.

[gp-Airee]

Just shade in https://github.com/lucko/spark/tree/master/spark-common for a TPS metric or borrow some code from it:

    public static final class TpsRollingAverage {
        private final int size;
        private long time;
        private BigDecimal total;
        private int index = 0;
        private final BigDecimal[] samples;
        private final long[] times;

        TpsRollingAverage(int size) {
            this.size = size;
            this.time = size * SEC_IN_NANO;
            this.total = new BigDecimal(TPS).multiply(new BigDecimal(SEC_IN_NANO)).multiply(new BigDecimal(size));
            this.samples = new BigDecimal[size];
            this.times = new long[size];
            for (int i = 0; i < size; i++) {
                this.samples[i] = new BigDecimal(TPS);
                this.times[i] = SEC_IN_NANO;
            }
        }

        public void add(BigDecimal x, long t, BigDecimal total) {
            this.time -= this.times[this.index];
            this.total = this.total.subtract(this.samples[this.index].multiply(new BigDecimal(this.times[this.index])));
            this.samples[this.index] = x;
            this.times[this.index] = t;
            this.time += t;
            this.total = this.total.add(total);
            if (++this.index == this.size) {
                this.index = 0;
            }
        }

        public double getAverage() {
            return this.total.divide(new BigDecimal(this.time), 30, RoundingMode.HALF_UP).doubleValue();
        }
    }

[sladkoff]

Due to popular demand this is now a bug.

@phemmer @gp-Airee PRs are always welcome, you guys already did half the job 😄
The current implementation isn't set in stone at all and you can feel free to replace it completely.

[phemmer]

I know barely anything about java, so I'm probably not the best person to tackle this. However my first thoughts are to either change the type from long to float, or keep the unit as milliseconds (don't divide by 1000), and multiply by 1000 on line 35.

IMHO, using a decimal data type is overkill (assuming that BigDecimal is the same as decimal data types in other languages). Decimal is heavy, and we don't need that level of precision.