0

I have written a merge sort algorithm, but it fails to sort in N log N. The code for sorting an array list is:

void merge(int start, int mid, int end) {
    int i,j,q;

    for (i=start; i<=end; i++) 
        l[i]=list[i]; 

    i=start; j=mid+1; q=start; 

    while (i<=mid and j<=end) { 
        if (l[i]<l[j]) 
            list[q++]=l[i++];
        else
            list[q++]=l[j++];
    }

    while (i<=mid) 
            list[q++]=l[i++];
}


void mergesort(int start, int end) {
    int mid;
    if (start<end) {
        mid=(start+end)/2;
        mergesort(start, mid);   
        mergesort(mid+1, end);  
        merge(start, mid, end); 
    }
}

However, if I for example sort 7800 numbers, the runtime is approximately 1.243 ms. The same sample is sorted by std::sort in 0.668 ms, and I know that sort has a complexity of N logN. What is wrong with my code? I cannot seem to find the waste of time.

Measurement of time:

#include <time.h>

clock_t start = clock();

//SORTING ALGORITHM HERE//

clock_t stop = clock();
double elapsed =(stop - start) * 1000.0 / CLOCKS_PER_SEC;
giusti
  • 3,156
  • 3
  • 29
  • 44
L. Bardos
  • 17
  • 3
  • why do you think it is not O(N log N)? – W.F. Jan 05 '17 at 13:30
  • It looks like you are making a copy of the data in every call to `merge`. Pretty sure `std::sort` does not do that. That could be your difference there. – NathanOliver Jan 05 '17 at 13:31
  • 3
    It's not because the algorithm takes double time, that it runs in a different time complexity. – Willem Van Onsem Jan 05 '17 at 13:31
  • 1
    but you know that O(k N log N) = O(N log N) – W.F. Jan 05 '17 at 13:33
  • @W.F. The runtime difference between the mergesort and std::sort is worrying me since its very significant and std::sort is O(N log N) – L. Bardos Jan 05 '17 at 13:34
  • So, did you expect your sort to be faster than the standard library? Also, did you test a debug version? Those can make a difference. – crashmstr Jan 05 '17 at 13:35
  • So it is acceptable that it has double the time? It was worrying me very much. Thank you all for your help. – L. Bardos Jan 05 '17 at 13:35
  • @crashmstr I expected it to be in the same time window because of the same complexity. – L. Bardos Jan 05 '17 at 13:36
  • 2
    What you would need to do is compare your code *against itself* for increasing large values of N. If the results match N log N, then your algorithm is O(N log N). – crashmstr Jan 05 '17 at 13:37
  • 1
    Big O notation says nothing about how long it takes in real time, just a relative amount with regards to the size of N. – crashmstr Jan 05 '17 at 13:38
  • Your code looks good. Try search a bug in measurement code or share it here. – Толя Jan 05 '17 at 13:42
  • @Толя I have included it in the post. – L. Bardos Jan 05 '17 at 13:46
  • You are most likely not programing in a realtime operating system. `clock()` will not reflect the runtime of your algorithm, as your program will certainly be interrupted randomly because other processes want CPU time, the CPU needs bring data from the memory to the cache, etc. To reduce the influence of those factors, call your algorithm many times so that the complexity of your algorithm outweights the architecture costs. – giusti Jan 05 '17 at 13:51
  • Other than you problem, you might want to avoid lines such as list[q++]=l[i++]; . That is, assignments coupled with incrementing variables. It works here, but those commands are different in nature. Go with list[q] = l[i]; q++; i++;. – Aziuth Jan 05 '17 at 14:48
  • 1
    @aziuth: arguable. `p[i++]=q[j++]` is a very common C idiom, alrhough perhaps not as common as `*p++ = *q++`. You are entitled to not like it, but many others may disagree. – rici Jan 05 '17 at 15:36
  • @rici Well, as you say it, it's a **C** idiom, in distinction to C++. But of course I agree that this is about personal taste, like a lot in coding styles. – Aziuth Jan 05 '17 at 16:09
  • What is types of l and list? If it a vector try change to array and meassure again. – Толя Jan 25 '17 at 13:30

2 Answers2

4

Assuming your implementation is correct, two O(N logN) won't necessarily run at the same amount of time. Asymptoptic complexity is a measure of how much the resources required to run a program grow as the input becomes very large. Just to give you an example, the following loops are both O(1), since each of them always run a constant number of steps:

for (i = 0; i < 10; i++) {
    printf("%d\n", i);
}
for (i = 0; i < 1000000000; i++) {
    printf("%d\n", i);
}

But there's no question that the second will take much longer to run. As a matter of fact, the runtime gap between these two loops will be significantly larger than the gap you are observing for your sort algorithm versus std::sort. That's because asymptoptic analysis neglects constants.

Moreover, asymptoptic complexity is usually for the average or worst case scenario. The same algorithm can run in more or less time for inputs of equal size depending on the data.

Not to mention that std::sort is very likely not a single sorting algorithm. It probably uses different strategies depending on the size of the array. In fact, somple implementations of std::sort use a mixture of algorithms.

The proper way to analyze a program's complexity is by reading the code. For a numerical approach, the closest you can do is to run your program, without comparing it to other programs, for several inputs of different sizes. Plot a graph an observe the curve.

Community
  • 1
  • 1
giusti
  • 3,156
  • 3
  • 29
  • 44
  • 1
    I'd say those loops look more like one algorithm with O(N) complexity, but I see your point of those being blocks of code with constant time (as there is no variance of the loop bounds). – crashmstr Jan 05 '17 at 13:40
  • You mean they are strictly pseudoconstant, because of word size? – giusti Jan 05 '17 at 13:47
  • No, looking at them as blocks of code, I would expect each one to be relatively constant time with itself. But looking at them together, I see one algorithm with two different values for N. – crashmstr Jan 05 '17 at 13:50
  • 2
    IMHO a for loop is always an `O(N)` operation where `N` is the number of iterations the loop does. – NathanOliver Jan 05 '17 at 13:50
  • 2
    @crashmstr it all depends on what you choose as N i.e. what you are analyzing. If you analyze the complexity of iterating N integers, then the complexity is of course O(N). But if you analyze the complexity of an algorithm that regardless of input (N) iterates a million integers - such as the example in the answer - the complexity is O(1) in relation to N. – eerorika Jan 05 '17 at 13:51
  • 1
    Complexity analysis is always about the size of the input. If there is not input, then it must be O(1). Or, if you want to be very strict, you say it's O(k), where k is the size of the data in bits or bytes -- this is one of the situations we get to say an algorithm is pseudoconstant, pseudolinear, etc. – giusti Jan 05 '17 at 13:54
  • 1
    @crashmstr I intended to make them two separate blocks. I edited to reflect this better. But if two algorithms separated are both O(something), an algorithm that runs each of them sequentially is also O(something). – giusti Jan 05 '17 at 13:58
0

In the case of Visual Studio, std::sort() is a mix of quick sort, heap sort (only to prevent worst case O(n^2) time complexity), and insertion sort, while std::stable_sort(), is a mix of merge sort and insertion sort. Both are reasonably fast, but it's possible to write faster code. The example code in the question is copying data before every merge, which consumes time. This can be avoided by doing a one time allocation of a working buffer, and switching merge direction based on level of recursion, using a pair of mutually recursive functions (shown below), or a boolean parameter to control merge direction (not used in example below).

Example C++ code for top down merge sort that is reasonably optimized (bottom up merge sort would be slightly faster, as it skips the recursion used to generate indices, using iteration instead).

// prototypes
void TopDownSplitMergeAtoA(int a[], int b[], size_t ll, size_t ee);
void TopDownSplitMergeAtoB(int a[], int b[], size_t ll, size_t ee);
void TopDownMerge(int a[], int b[], size_t ll, size_t rr, size_t ee);

void MergeSort(int a[], size_t n)       // entry function
{
    if(n < 2)                           // if size < 2 return
        return;
    int *b = new int[n];
    TopDownSplitMergeAtoA(a, b, 0, n);
    delete[] b;
}

void TopDownSplitMergeAtoA(int a[], int b[], size_t ll, size_t ee)
{
    if((ee - ll) == 1)                  // if size == 1 return
        return;
    size_t rr = (ll + ee)>>1;           // midpoint, start of right half
    TopDownSplitMergeAtoB(a, b, ll, rr);
    TopDownSplitMergeAtoB(a, b, rr, ee);
    TopDownMerge(b, a, ll, rr, ee);     // merge b to a
}

void TopDownSplitMergeAtoB(int a[], int b[], size_t ll, size_t ee)
{
    if((ee - ll) == 1){                 // if size == 1 copy a to b
        b[ll] = a[ll];
        return;
    }
    size_t rr = (ll + ee)>>1;           // midpoint, start of right half
    TopDownSplitMergeAtoA(a, b, ll, rr);
    TopDownSplitMergeAtoA(a, b, rr, ee);
    TopDownMerge(a, b, ll, rr, ee);     // merge a to b
}

void TopDownMerge(int a[], int b[], size_t ll, size_t rr, size_t ee)
{
    size_t o = ll;                      // b[]       index
    size_t l = ll;                      // a[] left  index
    size_t r = rr;                      // a[] right index
    while(1){                           // merge data
        if(a[l] <= a[r]){               // if a[l] <= a[r]
            b[o++] = a[l++];            //   copy a[l]
            if(l < rr)                  //   if not end of left run
                continue;               //     continue (back to while)
            while(r < ee)               //   else copy rest of right run
                b[o++] = a[r++];
            break;                      //     and return
        } else {                        // else a[l] > a[r]
            b[o++] = a[r++];            //   copy a[r]
            if(r < ee)                  //   if not end of right run
                continue;               //     continue (back to while)
            while(l < rr)               //   else copy rest of left run
                b[o++] = a[l++];
            break;                      //     and return
        }
    }
}
rcgldr
  • 27,407
  • 3
  • 36
  • 61