Generic Timing Class

Question

I have written a class which can time functions, and I'd like to have it reviewed. I'm interested in everything (better naming and commenting, accuracy of measurement, usability, structure, use of lambda, etc.).

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.IntFunction;

/**
 * A timing class.
 */
public class Timing {

    private List<TimingObject> functionsToTime;
    private int amount = 1_000_000;

    public Timing() {
        functionsToTime = new ArrayList<>();
    }

    /**
     * adds a new function which will be timed.
     *
     * @param <R> return type of functionToTime (irrelevant)
     * @param <T> input type of functionToTime (same as return type of
     * inputConverter)
     * @param functionToTime a function expecting input of type T, returning
     * output of any type (R)
     * @param inputConverter converts the loop variable to type T and passes it
     * to functionToTime
     * @param name name of the function (used for output)
     */
    public <R, T> void add(Function<R, T> functionToTime, IntFunction<T> inputConverter, String name) {
        functionsToTime.add(new TimingObject(functionToTime, inputConverter, name));
    }

    /**
     * sets how often the given functions should be run when timed.
     *
     * @param amount amount
     */
    public void setAmount(int amount) {
        this.amount = amount;
    }

    /**
     * performs the actual timing for all given functions.
     */
    public void time() {
        for (TimingObject timingObject : functionsToTime) {
            time(timingObject);
        }
    }

    /**
     * passes the result of the timing to the given consumer.
     *
     * @param consumer consumer
     * @param sort how to sort the result
     */
    public void output(Consumer<String> consumer, Sort sort) {
        switch (sort) {
            case ASC:
                Collections.sort(functionsToTime,
                        (TimingObject t1, TimingObject t2) -> (int) (t1.timeTaken - t2.timeTaken));
                break;
            case DESC:
                Collections.sort(functionsToTime,
                        (TimingObject t1, TimingObject t2) -> (int) (t2.timeTaken - t1.timeTaken));
                break;
            case NO:
                break;
            default:
                break;
        }
        consumer.accept(Formater.output(functionsToTime));
    }

    /**
     * times the function in the given object.
     *
     * @param timingObject timingObject
     */
    private void time(TimingObject timingObject) {
        runXTimes(timingObject, amount / 100); // warm up
        long startTime = System.nanoTime();
        runXTimes(timingObject, amount); // actual timing
        timingObject.timeTaken = System.nanoTime() - startTime;
    }

    /**
     * runs the function in the given object x amount of times.
     *
     * @param timingObject timingObject
     * @param amount amount
     */
    private void runXTimes(TimingObject timingObject, int amount) {
        for (int i = 0; i < amount; i++) {
            timingObject.function.apply(timingObject.inputConverter.apply(i));
        }
    }

    protected class TimingObject {

        private Function function;
        private IntFunction inputConverter;
        protected String name;
        protected long timeTaken;

        public TimingObject(Function function, IntFunction inputConverter, String name) {
            this.function = function;
            this.inputConverter = inputConverter;
            this.name = name;
        }
    }

    public static enum Sort {

        ASC, DESC, NO
    }
}

The formater (not reusable or anything):

import java.util.List;

public class Formater {

    /**
     * returns name and timeTaken of the given TimingObjects.
     *
     * The times will be aligned properly.
     *
     * @param functionsToTime functionsToTime
     * @return string
     */
    public static String output(List<Timing.TimingObject> functionsToTime) {
        int maxNameLength = getMaxNameLength(functionsToTime);
        String spaces = getSpaces(maxNameLength) + 1;
        StringBuilder out = new StringBuilder();
        for (Timing.TimingObject timingObject : functionsToTime) {
            out.append(timingObject.name);
            out.append(spaces.substring(0, spaces.length() - timingObject.name.length())); // align
            out.append(timingObject.timeTaken);
            out.append("\n");
        }
        return out.toString();
    }

    /**
     * returns n spaces.
     *
     * @param n number of spaces
     * @return n spaces
     */
    private static String getSpaces(int n) {
        StringBuilder spaces = new StringBuilder(n);
        for (int i = 0; i < n; i++) {
            spaces.append(" ");
        }
        return spaces.toString();
    }

    /**
     * returns the length of the longest function name.
     *
     * @return max length
     */
    private static int getMaxNameLength(List<Timing.TimingObject> functionsToTime) {
        int max = 0;
        for (Timing.TimingObject timingObject : functionsToTime) {
            int currentLength = timingObject.name.length();
            if (timingObject.name.length() > max) {
                max = currentLength;
            }
        }
        return max;
    }
}

And how it can be used:

import java.util.function.IntFunction;

public class Main {
   public static void main(String[] args) {
        Timing t = new Timing();
        IntFunction<String> intToString = (int i) -> String.valueOf(i) + "test";
        IntFunction<Integer> intToInt = (int i) -> i;

        // time function string->string:
        t.add((String s) -> functionToTimeString(s), intToString, "s + s");
        t.add((String s) -> functionToTimeString2(s), intToString, "s + s + s");
        // [...]

        // we can also time int->int functions at the same time:
        t.add((Integer i) -> functionToTimeInt(i), intToInt, "i + i");

        t.time();

        // output to stdo
        t.output((String s) -> System.out.println(s), Timing.Sort.DESC);
    }

    private static String functionToTimeString(String s) {
        return s + s;
    }

    private static String functionToTimeString2(String s) {
        return s + s + s;
    }

    private static int functionToTimeInt(int i) {
        return i + i;
    }
}

Output:

s + s + s 121048384
s + s     109057922
i + i     55232562

Answer 1

I really like the concept of this system, but it has some.... issues.

The Code Style is pleasant. I am sure there are some nit-picks in there, but, it is certainly clean enough to make reading it easy.

The issues I have are more with what you are timing, and some use cases that I think would break things. First though, what is good is the idea of having the two input functions, the one to time, and the other to generate the timing inputs.

The problem is that your timing function also times the generation time. The other issue is that you reuse inputs for both the warmup, and the actual timing process. Timing the generation is... awkward because you don't know whether to blame the time on the actual function, or the generator. The instance reuse is also awkward because I know instances where rerunning certain values may cause failures, or unreasonably-fast execution.

What I would recommend is that, instead of doing what you do with the warmup, you change your reporting system, and you time each function call individually, then throw out the 10% fastest runs, and the 10% slowest runs, then average the remainder..... (perhaps reporting the statistics like the 95th percentile as well, etc.). I would probably have a timing function like:

private long[] timeRuns(T[] input) {
    long[] times = new long[input.length];
    for (int i = 0; i < input.length; i++) {
        long start = System.nanoTime();
        timingObject.function.apply(input[i]);
        times[i] = System.nanoTime() - start;
    }
    return times;
}

With the above function, I would probably split the input in to chunks of say 1000 and call the timeRuns for each chunk. That way you can populate a steady stream of data in to chunks, time the chunks, and move on. You can collect all the individual times outside the timing loop, and then statistically analyze them separately.

This way, you won't need warmup runs as you will discard the slow ones. Slow runs can also be impacted by Garbage Collection, and other factors, so discarding the slow runs makes sense anyway. You also remove the input-value generation, and the duplicate-processing by doing this.

Even as it stands, I can see that this is a useful tool. I think the value can be improved by timing the right things, and being smarter about the reporting.