snippetcppMinor
Merge sort implementation efficiency
Viewed 0 times
sortefficiencyimplementationmerge
Problem
I made a merge sort implementation in C++, which I have tested with the values:
producing the correct output of:
However, when running my program, it seemed pretty slow, and I was wondering if there were any places in my code where there is an inefficiency, or if the entire implementation (recursive in most places, but iterative in parts of the merging) is the wrong way forward.
```
#include
#include
template
void merge(InputIt f1,
InputIt l1,
InputIt f2,
InputIt l2)
{
size_t d1 = std::distance(f1, l1),
d2 = std::distance(f2, l2);
if (d1 == 1 && d2 == 1 && f1 > f2) {
std::iter_swap(f1, f2);
}
else {
for (size_t i = 0; i *(f2 + j)) {
std::iter_swap(f1 + i, f2 + j);
}
}
}
}
}
template
void merge_sort(InputIt first,
InputIt last)
{
size_t dist = std::distance(first, last);
size_t left = (size_t)(dist / 2);
size_t right = dist - left;
//traverse left
if (left > 1) {
merge_sort(first, first + left);
}
//furthest left point found, merge upwards
merge(first, first + left, first + left, last);
//ensure that all the way right (limited to left section) has been reached
if (right > 1) {
merge_sort(first + left, last);
}
//furthest right point in left section found, merge upwards
merge(first, first + left, first + left, last);
//traverse right
if (right > 1) {
merge_sort(first + left, last);
}
//furthest right point found, merge upwards
merge(first, first + left, first + left, last);
//ensure that all the way left (limited to right section) has been reached
if (left > 1) {
merge_sort(first, first + left);
}
//furthest left point in right section found, merge upwards
merge(first, first + left, first + left, last);
}
int main()
{
6, 38, 27, 7, 12, 58, 92, 43, 3, 9, 82, 10producing the correct output of:
3 6 7 9 10 12 27 38 43 58 82 92However, when running my program, it seemed pretty slow, and I was wondering if there were any places in my code where there is an inefficiency, or if the entire implementation (recursive in most places, but iterative in parts of the merging) is the wrong way forward.
```
#include
#include
template
void merge(InputIt f1,
InputIt l1,
InputIt f2,
InputIt l2)
{
size_t d1 = std::distance(f1, l1),
d2 = std::distance(f2, l2);
if (d1 == 1 && d2 == 1 && f1 > f2) {
std::iter_swap(f1, f2);
}
else {
for (size_t i = 0; i *(f2 + j)) {
std::iter_swap(f1 + i, f2 + j);
}
}
}
}
}
template
void merge_sort(InputIt first,
InputIt last)
{
size_t dist = std::distance(first, last);
size_t left = (size_t)(dist / 2);
size_t right = dist - left;
//traverse left
if (left > 1) {
merge_sort(first, first + left);
}
//furthest left point found, merge upwards
merge(first, first + left, first + left, last);
//ensure that all the way right (limited to left section) has been reached
if (right > 1) {
merge_sort(first + left, last);
}
//furthest right point in left section found, merge upwards
merge(first, first + left, first + left, last);
//traverse right
if (right > 1) {
merge_sort(first + left, last);
}
//furthest right point found, merge upwards
merge(first, first + left, first + left, last);
//ensure that all the way left (limited to right section) has been reached
if (left > 1) {
merge_sort(first, first + left);
}
//furthest left point in right section found, merge upwards
merge(first, first + left, first + left, last);
}
int main()
{
Solution
Not a merge sort
This step in the
is an \$O(N^2)\$ operation and works more like an insertion sort or bubble sort rather than a merge. Also, you should only have to merge once instead of four times. The pseudocode for the mergesort function should be:
I believe your merge function is not only slow but also doesn't merge properly in one pass, which is why your program seems to require calling it four times.
This step in the
merge() function:for (size_t i = 0; i *(f2 + j)) {
std::iter_swap(f1 + i, f2 + j);
}
}
}is an \$O(N^2)\$ operation and works more like an insertion sort or bubble sort rather than a merge. Also, you should only have to merge once instead of four times. The pseudocode for the mergesort function should be:
mergesort(leftHalf);
mergesort(rightHalf);
merge(leftHalf, rightHalf);I believe your merge function is not only slow but also doesn't merge properly in one pass, which is why your program seems to require calling it four times.
Code Snippets
for (size_t i = 0; i < d1; ++i) {
for (size_t j = 0; j < d2; ++j) {
if (*(f1 + i) > *(f2 + j)) {
std::iter_swap(f1 + i, f2 + j);
}
}
}mergesort(leftHalf);
mergesort(rightHalf);
merge(leftHalf, rightHalf);Context
StackExchange Code Review Q#123976, answer score: 4
Revisions (0)
No revisions yet.