Debugging mergesort

Question

Ich brauche eine doppelt verknüpfte Liste zu sortieren. Nach der allmächtigen wikipedia ist mergesort der Weg für das gehen.

Der rekursive Algorithmus funktioniert recht gut, aber da ich eine Allzweck- Umsetzung Schreiben bin, Leistung ein Problem sein könnte.

Portierungs die iterative Version für Arrays wird die Leistung töten, wie die Liste erneut zu scannen es in Teil-Listen zu unterteilen ist langsam; für alle Interessierten - hier ist der Code:

static void sort(struct linked_list *list,
    int (*cmp)(const void *, const void *))
{
    size_t slice_size = 1;
    for(; slice_size < list->size; slice_size *= 2)
    {
        struct node *tail = list->first;
        while(tail)
        {
            struct node *head = tail;

            size_t count = slice_size;
            while(tail && count--) // performance killer
                tail = tail->next;

            count = slice_size;
            while(head != tail && tail && count)
            {
                if(cmp(head->data, tail->data) <= 0)
                    head = head->next;
                else
                {
                    struct node *node = tail;
                    tail = tail->next;
                    remove_node(node, list);
                    insert_before(node, list, head);
                    --count;
                }
            }

            while(tail && count--) // performance killer
                tail = tail->next;
        }
    }
}

Aber es gibt eine andere iterative Version einen Stack-basierten Ansatz:

struct slice
{
    struct node *head;
    size_t size;
};

static void sort(struct linked_list *list,
    int (*cmp)(const void *, const void *))
{
    if(list->size < 2) return;

    struct slice stack[32];
    size_t top = -1;
    struct node *current = list->first;

    for(; current; current = current->next)
    {
        stack[++top] = (struct slice){ current, 1 };
        for(; top && stack[top-1].size <= stack[top].size; --top)
            merge_down(list, cmp, stack + top);
    }

    for(; top; --top)
        merge_down(list, cmp, stack + top);
}

Dies wird Push Größe 1 Listen auf den Stapel und geht nach unten, solange die Top-Liste der größer oder gleich groß ist als sein Vorgänger.

Leider gibt es irgendwo einen Fehler wie für einige Eingabelisten, merge_down() wird eine Plausibilitätsprüfung fehlschlagen:

static void merge_down(struct linked_list *list,
    int (*cmp)(const void *, const void *), struct slice *top)
{
    struct node *right = top->head;
    size_t count = top->size;

    --top;

    struct node *left = top->head;
    top->size += count;

{
    // sanity check: count nodes in right list
    int i = count;
    struct node *node = right;
    for(; i--; node = node->next) if(!node)
    {
        puts("too few right nodes");
        exit(0);
    }
}

    // determine merged list's head
    if(cmp(left->data, right->data) <= 0)
    {
        top->head = left;
        left = left->next;
    }
    else
    {
        top->head = right;
        struct node *node = right;
        right = right->next;
        remove_node(node, list);
        insert_before(node, list, left);
        --count;
    }

    while(left != right && count)
    {
        if(cmp(left->data, right->data) <= 0)
            left = left->next;
        else
        {
            struct node *node = right;
            right = right->next;
            remove_node(node, list);
            insert_before(node, list, left);
            --count;
        }
    }
}

Die verknüpfte Liste Implementierung relevant sein könnte auch:

struct node
{
    struct node *prev;
    struct node *next;
    long long data[]; // use `long long` for alignment
};

struct linked_list
{
    struct _list _list; // ignore
    size_t size;
    struct node *first;
    struct node *last;
};

static void insert_before(struct node *node, struct linked_list *list,
    struct node *ref_node)
{
    if(ref_node)
    {
        node->next = ref_node;
        node->prev = ref_node->prev;
        if(ref_node->prev) ref_node->prev->next = node;
        else list->first = node;
        ref_node->prev = node;
    }
    else // empty list
    {
        node->next = NULL;
        node->prev = NULL;
        list->first = node;
        list->last = node;
    }
    ++list->size;
}

static void remove_node(struct node *node, struct linked_list *list)
{
    if(node->prev) node->prev->next = node->next;
    else list->first = node->next;
    if(node->next) node->next->prev = node->prev;
    else list->last = node->prev;
    --list->size;
}

Was bin ich hier fehlt?

Answer 1

fand ich den Fehler selbst:

for(; current; current = current->next)
{
    stack[++top] = (struct slice){ current, 1 };
    for(; top && stack[top-1].size <= stack[top].size; --top)
        merge_down(list, cmp, stack + top);
}

Hier wird der nächste Wert von current bestimmt nach der Anruf an merge_down(), die den Knoten verschieben könnten um, dh current->next wird nicht mehr auf den korrekten Knoten.

Rearranging behebt das Problem:

while(current)
{
    stack[++top] = (struct slice){ current, 1 };
    current = current->next;
    for(; top && stack[top-1].size <= stack[top].size; --top)
        merge_down(list, cmp, stack + top);
}

Danke für die Mühe der PMG:. Ich habe einige Stimmen, die hinzugefügt

Answer 2

Sie benötigen immer einen Knoten zum Ende der Liste kopieren?
Was ist der insert_before() Anruf dann?

insert_before(node, list, NULL);

Das würde vermasseln list->first und node->prev.

Answer 3

ich jetzt habe Ihren Code ausführen und bekam es zu arbeiten, nachdem ich kommentierte aus der Zeile unten angegeben.

static void merge_down(struct linked_list *list,
    int (*cmp)(const void *, const void *), struct slice *top)
{
    struct node *right = top->head;
    size_t count = top->size;

    --top;

    struct node *left = top->head;
    top->size += count; /* possible bug? */
 /* ^^^^^^^^^^^^^^^^^^^ */

funktioniert das auch für Sie?

Answer 4

Stack-basierter Ansatz

/* ... */
    struct slice stack[32];
    size_t top = -1;
    struct node *current = list->first;

    for(; current; current = current->next)
    {
        stack[++top] = (struct slice){ current, 1 };
        for(; top && stack[top-1].size <= stack[top].size; --top)
        /*    ^^^    */
            merge_down(list, cmp, stack + top);
    }
/* ... */

top wird immer 0 das erste Mal durch die Schleife sein, nicht wahr?
Die merge_down() Funktion wird nie aufgerufen werden. Ich habe den Code nicht versuchen, aber das sieht nicht richtig aus.

---

Bearbeiten 32 Elemente für die stack ist nicht genug ... Wenn die Liste mehr als 32 Elemente hat, um (vielleicht nach ein paar Pässe) Sie über das Ende des stack schreiben.

Answer 5

Als kriss darum gebeten, hier ist die rekursive Version (ein Standard mergesort die Merge-Funktion von dem anderen exaples verwendet wird):

static struct node *merge(struct linked_list *list,
    int (*cmp)(const void *, const void *),
    struct node *left, struct node *right, size_t right_count)
{
    struct node *head;
    if(cmp(left->data, right->data) <= 0)
    {
        head = left;
        left = left->next;
    }
    else
    {
        head = right;
        struct node *node = right;
        right = right->next;
        remove_node(node, list);
        insert_before(node, list, left);
        --right_count;
    }

    while(left != right && right_count)
    {
        if(cmp(left->data, right->data) <= 0)
            left = left->next;
        else
        {
            struct node *node = right;
            right = right->next;
            remove_node(node, list);
            insert_before(node, list, left);
            --right_count;
        }
    }

    return head;
}

static struct node *mergesort(struct linked_list *list,
    int (*cmp)(const void *, const void *), struct node *head, size_t size)
{
    if(size < 2) return head;
    size_t left_count = size / 2;
    size_t right_count = size - left_count;

    struct node *tail = head;
    size_t count = left_count;
    while(count--) tail = tail->next;

    return merge(list, cmp,
        mergesort(list, cmp, head, left_count),
        mergesort(list, cmp, tail, right_count),
        right_count);
}

static void sort(struct linked_list *list,
    int (*cmp)(const void *, const void *))
{
    mergesort(list, cmp, list->first, list->size);
}