Does this loop unrolling make sense?

Question

I am coding a Hanning filter which is a recursive relation:

y[i] = 0.25*(x[i] + 2*x[i-1] + x[i-2])

And here is my code were I attempt to unroll 3 times:

public void HanningFilter(float[] array) {  
        int len = array.length;
        int i;
        for (i = 2; i < len-3; i+=3) {
            array[i] = 0.25f*(array[i] + 2*array[i-1] + array[i-2]);
            array[i+1] = 0.25f*(array[i+1] + 2*array[i] + array[i-1]);
            array[i+2] = 0.25f*(array[i+2] + 2*array[i+1] + array[i]);
        }
        //Clean-up at the end
        for (int j = i; j < len; j++) {
            array[i] = 0.25f*(array[i] + 2*array[i-1] + array[i-2]);
        }
    }

Would this help at all with an OoO superscalar processor? Is 3 the right amount of times to unroll? Thanks

Answer 1

No, rule of thumbs for this case:

• Never guess optimizations, use a profiler
• You're not smarter then the JIT compiler

Loop unrolling only makes sense if you profiled the code and found the unrolled version to be faster then the normal version.

I guess you're coming from a C background, so please don't try to bring C-habits to Java. Java is different, always go for readability first.

---

public void HanningFilter(float[] array) {

The Java naming conventions dictate that function names are lowerCamelCase.

Also the name array is a bad name, more appropriate would be a name like data or input.

---

int len = array.length;
int i;
for (i = 2; i < len-3; i+=3) {

What you're doing here is absolutely unnecessary. Java allows declarations of variables inside the for and there's no need to define a variable for the condition.

for (int idx = 2; idx < data.length; idx++) {

Naming loop variables idx or counter is not very famous, I prefer it to i.

Answer 2

The JIT compiler does loop unrolling for you (a lot more than 3 iterations per loop) and it will revert the change if it does not help performance - writing your own is probably counterproductive and detrimental to performance.

You could play with the -XX:LoopUnrollLimit=n option on hotspot to see if it makes a difference (probably not).

Answer 3

The standard advice is, premature optimization is evil. Write the code cleanly and let the compiler and JIT do their job. If, after profiling the code for your entire program, this particular function is found to be a hot spot, then try it with loop unrolling and see if it makes a difference. But you shouldn't uglify your code unless you have numerical evidence that it improves performance enough to matter.

By the way, each array element is calculated based on itself and two previous values, but the previous values are taken from the output rather than the input. That's a bug. In other words, you implemented

y[i] = 0.25*(x[i] + 2*y[i-1] + y[i-2])

instead of

y[i] = 0.25*(x[i] + 2*x[i-1] + x[i-2])

To convolve the values without allocating extra space, start at the end of the array and work towards the front.