Finding minimum and maximum in array

Question

This is a very basic algorithm(can't get simpler), but i am stumped. We have an array of elements and we have to determine the minimum and maximum.

Normal approach is to go through the array and find min and max which is 2n compares.

Slightly more efficient way would be to first compare consecutive elements of array in pair to determine max and min of any two elements(n/2 compares). We now have n/2 min and n/2 max elements. Now we can have final max and min in n/2 + n/2 + n/2(previous step) = 3/2* n or 1.5n compares

That's fine. Theoretically then the code should take less time to run in 2nd case as we are doing less compares. But when i run the code the results are otherwise.

My code snippet is below:

public class MinMax {
public static void nonEfficient(int [] array){
    int min=array[0],max=array[0];
    for (int anArray : array) {
        if (anArray < min)
            min = anArray;
        else {
            if (anArray > max)
                max = anArray;
        }
    }
    System.out.println("Max is :" + max);
    System.out.println("Min is :" + min);
}
public static void efficient(int [] arr,int length){
    int max,min;
    max = min = arr[0];
    int i = 0;
    for (; i < length / 2; i++)
    {
        int number1 = arr[i * 2];
        int number2 = arr[i * 2 + 1];
        if (arr[i * 2] >= arr[i * 2 + 1])
        {
            if (number1 > max)
                max = number1;
            if (number2 < min)
                min = number2;
        }
        else
        {
            if (number2 > max)
                max = number2;
            if (number1 < min)
                min = number1;
        }
    }
    if (i * 2 < length)
    {
        int num = arr[i * 2];
        if (num > max)
            max = num;
        if (num < min)
            min = num;
    }
    System.out.println("***********************");
    System.out.println("Max is :" + max);
    System.out.println("Min is :" + min);
}
public static void main(String[] args) {
    int [] array =  new int[10000000];
    Random rand = new Random();
    for(int i=0;i<array.length;i++)
        array[i] = rand.nextInt(100000)-144;
    long startTime = System.currentTimeMillis();
    nonEfficient(array); //theoretically non efficient 2n compares
    long stopTime = System.currentTimeMillis();
    long elapsedTime = stopTime - startTime;
    System.out.println(elapsedTime);// just 11ms

    startTime = System.currentTimeMillis();
    efficient(array, 10000000);///theoretically more efficient 1.5n compares
    stopTime = System.currentTimeMillis();
    elapsedTime = stopTime - startTime;
    System.out.println(elapsedTime);//whooping 37 ms..what happpened ????
}
}

Could someone help me in figuring out what i am doing wrong. Is there something very obvious that i am missing.

Thanks for the help.

My first bet would be that first if check that you have in the "efficient" method. Rather than using number1 and number2, you're searching through the array again. Of course I'm sure the java for each is much better optimized at a much lower level than just the code implies, so you're probably losing a lot of efficiency there too. — Taugenichts, Mar 25 '14 at 06:16
Explicit *Comparsions* like `>` are not *the only* time consuming operations in the routine. Accessing arrays like `arr[i * 2]` can have comparioson as to check array's borders. — Dmitry Bychenko, Mar 25 '14 at 06:18
Nope.. I don't think that would contribute to such a huge margin. — thePoly_glot, Mar 25 '14 at 06:20
Try the straightforward approach using the `max` function. I could imagine the compiler to optimize the heck out of that one (SSE) — Niklas B., Mar 25 '14 at 06:20
@TapasKumarSenapati Well, obviously you are wrong, as your own benchmark shows... — Niklas B., Mar 25 '14 at 06:20
You are correct. But trying to understand something simple won't be harmful either. Might help in a different problem :-) — thePoly_glot, Mar 25 '14 at 06:23
The cache misses and additional index computations of your "efficient" function actually make it a lot less efficient than the straightforward approach which can be vectorized very well. I would not be suprised if your first code sample would actually be compiled down to SIMD instructions on the machine level — Niklas B., Mar 25 '14 at 06:24
By the way, your benchmark is flawed because you don't warmup the JIT, so your code might actually not even be compiled down to machine code. You should run the same code a few hundred times in a loop before timing it — Niklas B., Mar 25 '14 at 06:32
Your `nonEfficient` method accesses every item of the array ONCE and makes approximately ONE AND HALF comparisions in each iteration (one for `min`, possibly another for `max`). On the other hand your `efficient` method accesses every item TWICE (first to fetch the value into `number1` or `number2`, then in `if()`) and performs THREE comparisions every iteration (one to check which item is greater, then for `max` and for `min`). And performs additional `2*i` and `2*i+1` computations, which also consumes some time... — CiaPan, Mar 25 '14 at 07:00
That's a very clever approach, I like it. Did you make sure to benchmark the Release rather than Debug version of it ? You can rewrite the code to avoid the *2 in indexing, but I really doubt this can be the reason. — , Mar 25 '14 at 07:18
@CiaPan: mind that the second version does half of the loops, so the access count is ONE per element and the comparison count is ONE AND HALF. In addition, in the first version, the value of min goes decreasing so that less and less comparisons to min allow to bypass the comparison to max; so the number of comparisons is much closer to TWO than to ONE AND HALF. — , Mar 25 '14 at 07:26
How many `arr[...];` expressions are evaluated in each loop iteration? — CiaPan, Mar 25 '14 at 08:23
Look This : http://stackoverflow.com/questions/5846156/get-min-and-max-value-in-php-array — Dymencharles, Mar 25 '14 at 09:57

Marco13 · Accepted Answer · 2014-03-25T13:23:04.757

First of all: The benchmark is totally flawed. The time span that you are measuring is too short, there is no warmup of the JIT whatsoever, you are including the time for the System.out.println in your measurement. It could be made slightly more meaningful by applying the "usual" microbenchmark-pattern (see end of this answer)

But even with this "benchmark", there are significant differences. And there are many possible reasons for that:

One can assume that a single comparison on a modern CPU takes (amortized) a single CPU cycle. A single CPU cycle is the duration that a beam of light needs in order to travel 10 centimeters. Now, measure that ;-) Basically every other operation in your algorithm will take at least the same time, or much longer: Every i++ will take the same time, every min=x will take the same time or longer, every i*2 will most likely take longer...

Additionally, I think the most important point here is: CPUs are fast, but memory is slow. In the (not) "not efficient" case, you are running through the array sequentially. This is perfect for the cache. Every read of one cache line will be fully exploited. In contrast to that, in the (not) "efficient" case, your memory accesses are basically scattered over the whole array. It will have to read chunks of data from the main memory into the cache, and most of this data will have to be thrown away because it is not used immediately, but will be read again in the next pass.

Concerning the benchmark: It could be made slightly more meaningful by repeating the respective methods several times, and taking the average time, and doing this repeatedly with an increasing array size - roughly like this:

public static void main(String[] args)
{
    Random rand = new Random();
    long beforeNS = 0;
    long afterNS = 0;
    int results[] = { 0, 0 };
    int runs = 100;
    for (int size=10000; size<=10000000; size*=2)
    {
        int[] array = new int[size];
        for (int i = 0; i < array.length; i++)
            array[i] = rand.nextInt(size) - 144;

        beforeNS = System.nanoTime();
        for (int i=0; i<runs; i++)
        {
            nonEfficient(array, results);
        }
        afterNS = System.nanoTime();
        System.out.println(
            "Not efficient, size "+size+
            " duration "+(afterNS-beforeNS)/1e6+
            ", results "+Arrays.toString(results));

        beforeNS = System.nanoTime();
        for (int i=0; i<runs; i++)
        {
            efficient(array, array.length, results);
        }
        afterNS = System.nanoTime();
        System.out.println(
            "Efficient    , size "+size+
            " duration "+(afterNS-beforeNS)/1e6+
            ", results "+Arrays.toString(results));
    }
}

Still, the results may be questioned, als long as the VM options are not known etc. But it at least gives a slightly more reliable indication about whether there are "significant" differences between the two approaches.

+1: Fewer code bytes and sequential memory access is hard to beat when you have to look at every item. Nice info on the timing. — Jim Mischel, Mar 25 '14 at 13:00
Nice reasoning Marco. I am going to accept this answer as the most plausible explanation. — thePoly_glot, Mar 25 '14 at 17:16

Dmitry Bychenko · Answer 2 · 2014-03-25T09:26:11.020

Let's just count the comparisons:

for (int anArray : array) { // <- One: check array's border
    if (anArray < min)      // <- Two
        min = anArray;
    else {
        if (anArray > max)  // <- Three
            max = anArray;
    }
}

Finally N * 3 = 3N (in the worst case)

for (; i < length / 2; i++) {  // <- One
        int number1 = arr[i * 2]; // <- Two
        int number2 = arr[i * 2 + 1]; // <- Three
        if (arr[i * 2] >= arr[i * 2 + 1]) // <- Five, Six: arr[i * 2]; Seven: arr[i * 2 + 1]  
        {
            if (number1 > max) // <- Eight
                max = number1;
            if (number2 < min) // <- Nine
                min = number2;
        }
        else
        {
            if (number2 > max) // <- Eight
                max = number2;
            if (number1 < min) // <- Nine
                min = number1;
        }
    }

Finally: N/2 * 9 = 4.5N

or if optimizer is good enough and it can eliminate 5, 6 we have however

N/2 * 7 = 3.5N

You can slightly improve your code

int min = array[array.length - 1];
int max = min; // <- there's no need to call array[array.length - 1]

// You can start from array.length - 2 not from usual array.length - 1 here    
// Comparison with constant 0 is slightly better
// (one JZ or JNZ assembler instruction)
// than with array.length
for (int i = array.length - 2; i >= 0; --i) {
  int v = array[i];

  if (v > max)
    max = v;
  else if (v < min) 
    min = v;
}

On, say, "arr[i * 2]" compiler should check if "i * 2" is within array borders range [0..length - 1]. — Dmitry Bychenko, Mar 25 '14 at 07:38
That makes sense. The implicit loop construct avoids this bounds checking (luckily, can be done in a single compare), giving it a big advantage. — , Mar 25 '14 at 07:56

score 1 · Answer 3 · answered Mar 25 '14 at 07:20

When using Java 8, I'd have to disagree on the "can't get simpler" remark:

public static void java8(int[] arr, int length){
  IntSummaryStatistics stats = 
        IntStream.of(arr)
        .summaryStatistics();

  System.out.println("***********************");
  System.out.println("Max is :" + stats.getMax());
  System.out.println("Min is :" + stats.getMin());
}

The performance of this piece of code is less than yours, but considering the fact that you also get the count and total of the array, it’s not that bad. This compares your code to the java8 method:

nonEfficient:
Max is :99855
Min is :-144
12
***********************
efficient:
Max is :99855
Min is :-144
43
***********************
java8:
Max is :99855
Min is :-144
69

score 0 · Answer 4 · answered Mar 25 '14 at 06:23

0

I would use the JDK to do the heavy lifting:

 Arrays.sort(array);   // sort from lowest to highest
 int min = array[0];   // the minimum is now the first element
 int max = array[array.length - 1];   // and the maximum is last

And you're done in 3 lines. Unless you were dealing with huge arrays, this would be quite efficient.

answered Mar 25 '14 at 06:23

Bohemian

412,405
93
575
722

2

What? Even for `n=1000000` this should be a *lot* slower due to the unnecessary log-factor – Niklas B. Mar 25 '14 at 06:24
1

@NiklasB. I would call 1 million elements "huge" – Bohemian Mar 25 '14 at 06:25
But that is what OP is testing. Even for `n=1000` the factor would probably be well noticable – Niklas B. Mar 25 '14 at 06:26
@NiklasB. You can't use `Collections.min()`, because an array is not a collection. Further, you can't easily turn a primitive array into a collection of wrapper – Bohemian Mar 25 '14 at 06:26
I see, I don't know Java too well – Niklas B. Mar 25 '14 at 06:26
@NiklasB. Just ran a test. It took about .5 seconds to sort 1 million ints, but took only .005 to iterate over them. But for 10K ints, it took .021 seconds to sort and .003 to iterate - much closer. At 100 ints, there's hardly a difference. I think my contention that performance would be OK for small arrays is acceptable. – Bohemian Mar 25 '14 at 06:35
1

Sure, I was assessing this in the context of OPs benchmark with 1 million elements – Niklas B. Mar 25 '14 at 06:38
2

Using an `O(N.Log(N))` solution for a problem that can be solved in `O(N)` is clearly inefficient. – Mar 25 '14 at 07:34

score 0 · Answer 5 · answered Mar 25 '14 at 06:24

0

Your function nonEfficient is incorrect. If it is called with [5, 4, 3, 2, 1], the else statement will never be executed and max will be set to Integer.MIN_VALUE.

answered Mar 25 '14 at 06:24

jelinson

845
8
15

Nope, it is set to `arr[0]`, indeed the largest value. – Mar 25 '14 at 07:31
@YvesDaoust -- the post was edited after my response. The original implementation did not set `min` and `max` to the first element, but instead `Integer.MAX_VALUE` and `Integer.MIN_VALUE`, respectively. – jelinson Mar 28 '14 at 06:53
1

I understand. This is indeed a subtle bug. I recall it struck me back in '80 when implementing a surface plotting algorithm ;-) – Mar 28 '14 at 08:07

score 0 · Answer 6 · answered Mar 25 '14 at 06:36

0

If you're using java8 you can take the stream approach max value using stream

In my opinion it is more readable in complex structures. Hope that helps

answered Mar 25 '14 at 06:36

Inge

427
7
20

score 0 · Answer 7 · 2014-03-25T14:45:30.127

Try this:

if (length & 1)
{
  // Odd length, initialize with the first element
  min= max= a[0];
  i= 1;
}
else
{
  // Even length, initialize with the first pair
  if (a[0] < a[1])
  {
    min= a[0]; max= a[1];
  }
  else
  {
    min= a[1]; max= a[0];
  }
  i= 2;
}

// Remaining pairs
while (i < length)
{
    int p= a[i++], q= a[i++];
    if (p < q)
    {
      if (p < min) min= p;
      if (q > max) max= q;
    }
    else
    {
      if (q < min) min= q;
      if (p > max) max= p;
    }
}

You can even get rid of the array indexing using this ugly foreach loop, at the expense of a Boolean test and extra overhead.

// Initialize
min= max= a[0];

bool Even= true;
int p;
for (int q : a)
{
  if (Even)
  {
    // Set p on hold
    p= q;
  }
  else
  {
    // Process the pair
    if (p < q)
    {
      if (p < min) min= p;
      if (q > max) max= q;
    }
    else
    {
      if (q < min) min= q;
      if (p > max) max= p;
    }
  }
  Even= !Even;
}

if (!Even)
{
  // Process the last element
  p= a[length - 1];
  if (p < min) min= p;
  else 
    if (p > max) max= p;
}

Finding minimum and maximum in array

7 Answers7