14
\$\begingroup\$

I'm working on a simple micro-benchmarking library in Java.

This library makes it easy to benchmark multiple reference implementations. You provide the inputs and trigger the calls, the library takes care of executing N times, measuring the runs and printing the averaged results.

Essentially, it works like this:

  1. Create a dedicated class for the benchmark

  2. Prepare input data in the constructor

  3. Create one method per reference implementation, annotate with @MeasureTime

  4. Add a main method to trigger the benchmark runner, with an instance of this class as parameter

  5. Run the class, find the results on stdout

To set the number of warm-up iterations and iterations, use the @Benchmark annotation on the class, for example: @Benchmark(iterations = 10, warmUpIterations = 5)

The annotations:

@Retention(RetentionPolicy.RUNTIME)
public @interface MeasureTime {

    int[] iterations() default {};

    int[] warmUpIterations() default {};
}

@Retention(RetentionPolicy.RUNTIME)
public @interface Benchmark {

    int iterations() default BenchmarkRunner.DEFAULT_ITERATIONS;

    int warmUpIterations() default BenchmarkRunner.DEFAULT_WARM_UP_ITERATIONS;
}

The class that runs the benchmarks on a target object passed in, parameterized by annotations:

import microbench.api.annotation.Benchmark;
import microbench.api.annotation.MeasureTime;
import microbench.api.annotation.Prepare;
import microbench.api.annotation.Validate;

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.*;

public class BenchmarkRunner {

    public static final int DEFAULT_ITERATIONS = 1;
    public static final int DEFAULT_WARM_UP_ITERATIONS = 0;

    private final Object target;

    private final int defaultIterations;
    private final int defaultWarmUpIterations;

    private final List<Method> measureTimeMethods = new ArrayList<>();
    private final List<Method> prepareMethods = new ArrayList<>();
    private final List<Method> validateMethods = new ArrayList<>();

    public BenchmarkRunner(Object target) {
        this.target = target;
        Class<?> clazz = target.getClass();

        Benchmark annotation = clazz.getAnnotation(Benchmark.class);
        if (annotation != null) {
            defaultIterations = annotation.iterations();
            defaultWarmUpIterations = annotation.warmUpIterations();
        } else {
            defaultIterations = DEFAULT_ITERATIONS;
            defaultWarmUpIterations = DEFAULT_WARM_UP_ITERATIONS;
        }

        for (Method method : clazz.getDeclaredMethods()) {
            if (method.getAnnotation(MeasureTime.class) != null) {
                measureTimeMethods.add(method);
            } else if (method.getAnnotation(Prepare.class) != null) {
                prepareMethods.add(method);
            } else if (method.getAnnotation(Validate.class) != null) {
                validateMethods.add(method);
            }
        }
        Collections.sort(measureTimeMethods, (o1, o2) -> o1.getName().compareTo(o2.getName()));
    }

    public static void run(Object target) {
        new BenchmarkRunner(target).run();
    }

    public void run() {
        runQuietly();
    }

    private void runQuietly() {
        try {
            runNormally();
        } catch (InvocationTargetException | IllegalAccessException e) {
            e.printStackTrace();
        }
    }

    private void runNormally() throws InvocationTargetException, IllegalAccessException {
        Map<Method, Throwable> validationFailures = new LinkedHashMap<>();

        for (Method method : measureTimeMethods) {
            MeasureTime measureTime = method.getAnnotation(MeasureTime.class);
            if (measureTime != null) {
                try {
                    runMeasureTime(target, method, measureTime);
                } catch (InvocationTargetException e) {
                    Throwable cause = e.getCause();
                    if (cause instanceof AssertionError) {
                        validationFailures.put(method, cause);
                        printExecutionFailure(method);
                    } else {
                        throw e;
                    }
                }
            }
        }

        if (!validationFailures.isEmpty()) {
            System.out.println();
            for (Map.Entry<Method, Throwable> entry : validationFailures.entrySet()) {
                System.out.print("Validation failed while executing " + entry.getKey().getName() + ": ");
                System.out.println(entry.getValue());
            }
        }
    }

    private void invokeMethods(Object instance, List<Method> methods) throws InvocationTargetException, IllegalAccessException {
        for (Method method : methods) {
            method.invoke(instance);
        }
    }

    private void runMeasureTime(Object instance, Method method, MeasureTime measureTime)
            throws InvocationTargetException, IllegalAccessException {
        for (int i = 0; i < getWarmUpIterations(measureTime); ++i) {
            invokeMethods(instance, prepareMethods);
            method.invoke(instance);
            invokeMethods(instance, validateMethods);
        }

        int iterations = getIterations(measureTime);
        long sumDiffs = 0;

        for (int i = 0; i < iterations; ++i) {
            invokeMethods(instance, prepareMethods);
            long start = System.nanoTime();
            method.invoke(instance);
            sumDiffs += System.nanoTime() - start;
            invokeMethods(instance, validateMethods);
        }
        printExecutionResult(method, sumDiffs / iterations);
    }

    private void printExecutionInfo(String message, String ms) {
        System.out.println(String.format("%-60s: %10s ms", message, ms));
    }

    private void printExecutionFailure(Method method) {
        printExecutionInfo("Validation failed while executing " + method.getName(), "-");
    }

    private void printExecutionResult(Method method, long nanoSeconds) {
        printExecutionInfo("Average execution time of " + method.getName(), "" + nanoSeconds / 1_000_000);
    }

    private int getParamValue(int[] values, int defaultValue) {
        if (values.length > 0) {
            return values[0];
        }
        return defaultValue;
    }

    private int getWarmUpIterations(MeasureTime measureTime) {
        return getParamValue(measureTime.warmUpIterations(), defaultWarmUpIterations);
    }

    private int getIterations(MeasureTime measureTime) {
        return getParamValue(measureTime.iterations(), defaultIterations);
    }
}

An example benchmark class:

public class SimpleSortingDemo {
    private List<Integer> shuffledList;

    public SimpleSortingDemo() {
        shuffledList = new ArrayList<>();
        for (int i = 0; i < 10000; ++i) {
            shuffledList.add(i);
        }
        Collections.shuffle(shuffledList);
    }

    public static void main(String[] args) {
        new BenchmarkRunner(new SimpleSortingDemo()).run();
    }

    @MeasureTime
    public void bubbleSort() {
        BubbleSort.sort(new ArrayList<Integer>(shuffledList));
    }

    @MeasureTime
    public void insertionSort() {
        InsertionSort.sort(new ArrayList<Integer>(shuffledList));
    }
}

If you want to test drive it in your own projects, the GitHub project page explains nicely the steps to get started.

I'd like a review in terms of everything, but here are some points you might want to pick on:

  • What would you do differently?
    • Is there a way to make the library easier to use?
  • Is the implementation of BenchmarkRunner clear and natural?
    • Is it adequate the way it measures the execution time?
  • Are the annotation names intuitive and natural? (If not, can you suggest better names?)
  • The @MeasureTime annotation returns int[] as iterations, which is sort of a dirty hack I use to treat it null by default, inheriting from @Benchmark.iterations or the global default. Is there a cleaner way to do this?
\$\endgroup\$
  • 1
    \$\begingroup\$ Doesn't JMH do the job for you? openjdk.java.net/projects/code-tools/jmh \$\endgroup\$ – luboskrnac Nov 2 '14 at 21:28
  • 3
    \$\begingroup\$ @lkrnac JMH or code.google.com/p/caliper are rather sophisticated frameworks and difficult to handle in cases where you just want to do a quick, basic tests with reasonable effort. (I recently started something similar, maybe I'll have a closer look at this later). One of the first general things that I noticed here is that the results of the methods are not used, and might be optimized away. JMH uses the hg.openjdk.java.net/code-tools/jmh/file/de0365d7e4d0/jmh-core/… class to avoid this (again: really sophisticated...) \$\endgroup\$ – Marco13 Nov 3 '14 at 10:32
  • \$\begingroup\$ I'd strongly suggest, if you have some webspace available somewhere, that you host your maven repo online. Would be a bit more attractive to use it in actual projects in that case. Currently one would have to put additional documentation for the fact that your repo needs to be mvn install'd to be used. \$\endgroup\$ – Simon Forsberg Dec 12 '14 at 23:25
  • \$\begingroup\$ @SimonAndréForsberg I think it's either available on Maven Central or it's not. I looked into making available on Maven Central but it's a bit nightmarish, so I put that on hold. Someday, hopefully, I'll get it done. Till then, if you want to use this without forcing your users the extra step of installing this dependency, another option is to put the benchmark code in a submodule that's not listed in the parent pom. That way you can still do mvn test in your main project without this dependency. Ok ok I'll try to get this in Maven Central soon :) \$\endgroup\$ – janos Dec 13 '14 at 8:49
  • 1
    \$\begingroup\$ Thanks @SimonAndréForsberg, I setup a Maven repo, and updated the README accordingly. This is great! \$\endgroup\$ – janos Dec 13 '14 at 19:49
5
\$\begingroup\$

I like the Idea of this very small framework and it looks like a nice way to measure runtime quickly. I just have some minor comments.

If-Statement

 for (Method method : measureTimeMethods) {
            MeasureTime measureTime = method.getAnnotation(MeasureTime.class);
            if (measureTime != null) {
                try {

I think the if (measureTime != null) { is not needed, as this is the pre-condition for adding a method to measureTimeMethods.

Code Documentation

Your code is lacking any form of comments in the code. In my humble opinion, at least every public method should be commented. For developing it might be useful on private methods as well. For example to me it is not obvious (by reading the method names) what the difference between runQuietly() and runNormally() is. Comment ratio depends on your personal taste a lot though and is very dependant on the methods' names.

Other Comments

runQuietly() could probably be inlined. It is only used once in run().

runNormally(): Maybe rename to to: runMeasurements() or similar?

runMeasureTime(): Maybe rename to measureMethodRuntime() or similar?

getParamValue(): Maybe rename to getFirstOrDefault() or similar?

Someone with more expierence implementing anotations than me should probably answer your questions about the annotation implementation, so I'll not comment on that.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you very much, very nice review, and warm welcome to Code Review! \$\endgroup\$ – janos Dec 13 '14 at 12:59
5
\$\begingroup\$

First of all, I have to say that the purpose of this library is excellent. The usefulness is great!, especially for me.


Your annotations lack a @Target annotation, which makes the following legal:

@MeasureTime
private String test;

Wait, what? That doesn't make sense...

The annotation has no effect, but one could put it there without the compiler complaining.

It is very easy to mixup the Benchmark and MeasureTime annotations because they're not forced to a specific target.


In the @MeasureTime annotation, I think I would use -1 (or any other negative number) as a "inherit / use default" value. I think that makes more sense than the possibility to annotate it with @MeasureTime(iterations = { 4, 2, 3 }).

I am also considering if you really need both @MeasureTime and @Benchmark. As noted above, they can create confusion right now because of a lack of target limitation. I think it would be reasonable to use one annotation for both a class and a method.


If I put an annotation on a method with any argumants, I get:

java.lang.IllegalArgumentException: wrong number of arguments

But this doesn't tell me, as a user of your framework, what the correct usage is.

A better message would be "@MeasureTime can only be used on methods with no arguments"


What is really the point of throwing the Exceptions from runNormally ? That method is only called from runQuietly, where the Exceptions are swallowed!

This causes the following JUnit test class to not fail:

public class NonFailingTest {

    @MeasureTime(warmUpIterations = 100, iterations = 100)
    public void fail() {
        Assert.assertFalse(true);
    }

    @Validate
    public void fail2() {
        Assert.assertFalse(true);
    }

    @Test
    public void testName() {
        new BenchmarkRunner(this).run();
    }

}

That's certainly not good for automated building!


It would be great if you would integrate your benchmarking framework with JUnit, so that one could run a JUnit class with a runner from your framework. Consider a method like this:

@MeasureTime(warmUpIterations = 100, iterations = 100)
@Test(timeout = 1000)
public void longRun() {
    // some long-running method here
}

It would then fail the test if the average time exceeded the timeout or, if any one iteration of running it did (configurable?)


The framework is really useful, but there is some room for improvement.

\$\endgroup\$
4
\$\begingroup\$

In addition to the existing reviews, I have some remarks about the used style in combination with Java 8 that would lead to less lines of code and hopefully more maintainable code.

Using Map.forEach((k, v) -> { ... });

if (!validationFailures.isEmpty()) {
    System.out.println();
    for (Map.Entry<Method, Throwable> entry : validationFailures.entrySet()) {
        System.out.print("Validation failed while executing " + entry.getKey().getName() + ": ");
        System.out.println(entry.getValue());
    }
}

This could be changed into the following, providing clearer variable names:

if (!validationFailures.isEmpty()) {
    System.out.println();
    validationFailures.forEach((method, throwable) -> {
        System.out.println("Validation failed while executing " + method.getName() + ": " + throwable);
    });
}

Using List.forEach(v -> { ... });

private void invokeMethods(Object instance, List<Method> methods) throws InvocationTargetException, IllegalAccessException {
    for (Method method : methods) {
        method.invoke(instance);
    }
}

Changing this would lead to less lines of code:

private void invokeMethods(Object instance, List<Method> methods) throws InvocationTargetException, IllegalAccessException {
    methods.forEach(method -> method.invoke(instance));
}

I don't have many other things to comment on, overall this code looks really great!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.