Mergesort sehr langsam

[Godemit]

Moin,

ich arbeite grade an einem kleinen Programm, in dem eine verkettete Liste per Mergesort sortiert werden soll. Die Liste ist folgendermaßen aufgebaut

struct edge
{
	int cost;
	edge *next;
};

und hat im aktuellen Stadium etwa 100.000 Einträge. Das dürften später allerdings deutlich mehr werden (ich denke mal bis 10 Mio).

Um diese nach "cost" zu sortieren habe ich mich für mergesort entschieden, welches für sehr große verkettete Listen anscheinend das schnellste sein soll.

Hier ist mein Algorithmus (geschrieben nach http://de.wikipedia.org/wiki/Mergesort#Implementierung)

edge* merge(edge *l1, edge *l2)
{
	edge *tmp = NULL;
	edge *start = NULL;
	edge *end = NULL;
	while (l1 && l2)
	{
		if (l1->cost <= l2->cost){
			tmp = new edge;
			tmp->cost = l1->cost;
			tmp->next = NULL;
			if (start == NULL) start = tmp;
			if (end != NULL) end->next = tmp;
			end = tmp;
			l1 = l1->next;
		} else {
			tmp = new edge;
			tmp->cost = l2->cost;
			tmp->next = NULL;
			if (start == NULL) start = tmp;
			if (end != NULL) end->next = tmp;
			end = tmp;
			l2 = l2->next;
		}
	}
	while (l1){
		tmp = new edge;
		tmp->cost = l1->cost;
		tmp->next = NULL;
		if (start == NULL) start = tmp;
		if (end != NULL) end->next = tmp;
		end = tmp;
		l1 = l1->next;
	}
	while (l2){
		tmp = new edge;
		tmp->cost = l2->cost;
		tmp->next = NULL;
		if (start == NULL) start = tmp;
		if (end != NULL) end->next = tmp;
		end = tmp;
		l2 = l2->next;
	}

	return start;
}

edge* mergesort(edge *liste)
{
	if (liste->next){
		edge *tmp=NULL;
		edge *l1=NULL;
		edge *l2=NULL;
		int i=0;
		while (liste){
			tmp = new edge;
			tmp->cost = liste->cost;

			if (i%2==0){
				tmp->next = l1;
				l1 = tmp;
			} else {
				tmp->next = l2;
				l2 = tmp;
			}

			liste = liste->next;
			i++;
		}
		return merge(mergesort(l1),mergesort(l2));
	}
	else return liste;
}

Der Code in Zeile 59-65 dient dazu, die Liste in zwei Teile aufzuteilen. Anstatt sie in der Mitte aufzutrennen, schiebe ich die Einträge abwechselnd in Liste 1 und 2. Das ist, denke ich, schneller als die Mitte zu suchen.

Auf jeden Fall benötigt der Aufruf der mergesort() Funktion für eine Liste mit 100.000 Einträgen über 5 Sekunden. Bei 400.000 stürzt das Programm sogar mit einer leider nicht lesbaren Fehlermeldung der MS Visual C++ Debug Library ab.

Habe ich hier irgendeinen Denkfehler? Diese Laufzeit ist doch jenseits von Gut und Böse, vom Programmabsturz mal ganz abgesehen.

Ich bin für jede Hilfe dankbar

[edit] achso ja, der Code funktioniert, also die Liste wird durch die Funktion ordentlich sortiert

[knivil]

Wieso verwendest du nicht std::list und das entsprechende sort?

Diese Laufzeit ist doch jenseits von Gut und Böse

Debug/Release?

[drakon]

Kompilier mal auf Release. Auch wenn das hier denke ich nicht soo wahnsinnig viel ausmacht (weil ich keine STL sehe).

So, wie ich das auf die schnelle sehe gehst du mit dem Algorithmus bis ganz nach unten. Das solltest du nicht machen, weil du sonst sehr viele kleine Mergeschritte hast, was nicht sehr gut ist. Sortier irgendwo bei einer grösse von 5-10 Elementen mal mit einem O(n^2) Algorithmus, wie z.B Introsort. Das sollte einiges ausmachen.

[unskilled]

introsort ist aber n*log n.. ^^
ich würd auch lieber nen einfacheren nehmen - bei 5-10 elementen sollte es so gar ein bubblesort noch tun^^

[drakon]

unskilled schrieb:

introsort ist aber n*log n.. ^^
ich würd auch lieber nen einfacheren nehmen - bei 5-10 elementen sollte es so gar ein bubblesort noch tun^^

Ach Mist. Ich meinte natürlich Insertionsort..
Warum müssen die auch so ähnlich klingen.

Hmm. Imo kann man Introsort nicht so einfach mit nlog n bewerten.. Wenn es dumm läuft kann man wahrscheinlich (je nach Kritierum für das wechseln) schon ein Beispiel bauen, welches n^2 Worst Case hat.
Ist natürlich für praktsiche Anwendungen sicher eines der besten Verfahren, aber vom theoretischen Standpunkt aus ist diese Laufzeit bestimmt nicht so einfach zu beweisen.

[krümelkacker]

Godemit schrieb:

Anstatt sie in der Mitte aufzutrennen, schiebe ich die Einträge abwechselnd in Liste 1 und 2. Das ist, denke ich, schneller als die Mitte zu suchen.

Jo, das ist das naheliegenste. Das macht std::list<>::sort wahrscheinlich genauso.

Godemit schrieb:

Auf jeden Fall benötigt der Aufruf der mergesort() Funktion für eine Liste mit 100.000 Einträgen über 5 Sekunden. Bei 400.000 stürzt das Programm sogar mit einer leider nicht lesbaren Fehlermeldung der MS Visual C++ Debug Library ab.

Wie sortierst Du denn? Da sind ja 'ne ganze Menge new s drin. Das geht auch ohne, als nur mit Zeiger umbiegen...

Es ist noch gar nicht so lange her, wo ich nen MergeSort (zur Übung, zum Spass) für eine Liste gebaut habe. Ich hatte es in etwa so gemacht:

/// moves every 2nd element from list1 to list2
void split(node*& list1, node*& list2);

/// merges both lists and stores the result in list1, list2 will be empty
void merge(node*& list1, node*& list2);

void mergesort(node*& list) {
  node* temp = 0;
  split(list,temp);
  if (temp==0) return; // only 0 or 1 node(s) in list
  mergesort(list);
  mergesort(temp);
  merge(list,temp);
  assert(temp==0);
}

split und merge waren auch nicht sooo kompliziert. Aber das krieg ich jetzt wahrscheinlich nicht fehlerfrei runtergetippt. Jedenfalls braucht man hier kein new .

kk

[Gast]

Hallo Godemit,

Ich verstehe leider nicht auf die schnelle was der code machen soll,

was mir auffällt, ist daß viele temporäre objekte mit new angelegt werden
aber nicht wieder frei gegeben werden.
Ich kann mir vorstellen, daß genau das zu dieser schlechten Performance führt.

Kannst du nicht sogar ganz auf new verzichten und den Stack verwenden ?

2. (i%2==0) kann man schneller als (i%2) oder noch schneller als (i&0x01) ausdrücken.
Macht aber nicht dein Problem aus.

Gruß Frank

edit hat den Adressat korrigiert

[drakon]

Frank Erdorf schrieb:

Hallo drakon,

Ich verstehe leider nicht auf die schnelle was der code machen soll,

was mir auffällt, ist daß viele temporäre objekte mit new angelegt werden
aber nicht wieder frei gegeben werden.
Ich kann mir vorstellen, daß genau das zu dieser schlechten Performance führt.

Kannst du nicht sogar ganz auf new verzichten und den Stack verwenden ?

2. (i%2==0) kann man schneller als (i%2) oder noch schneller als (i&0x01) ausdrücken.
Macht aber nicht dein Problem aus.

Gruß Frank

Gruß Frank

Warum ich?!

1. Nein hat keinen Einfluss auf Performance. Ausser, dass ev. der RAM gefüllt wird.

2. Solche Sachen kommen halt auch sehr auf den Compiler drauf an und können daher nur sehr spezifsch entschieden werden.

[unskilled]

1. naja, doch schon - du könntest, falls du die news wirklich brauchst/willst höchstens placement new nutzen, aber im vorraus wirds ein wenig schwer, zu wissen, wie viele knoten du brauchst - und spätestens wenn du dann iwas mit allokatoren bauen möchtest, wirst du nicht drum rum kommen, alles wieder neu zu machen...

2. ich kenn keinen compiler, der im release-mode nicht i%2, i%2==0 und i&1 in äquivalenten maschinencode umsetzt

bb

[krümelkacker]

Frank Erdorf schrieb:

Kannst du nicht sogar ganz auf new verzichten und den Stack verwenden?

Man muss überhaupt gar keine node-Objekte irgendwo neu anlegen. Einfach nur Zeiger "verbiegen". Mehr nicht.

Frank Erdorf schrieb:

2. (i%2==0) kann man schneller als (i%2) oder noch schneller als (i&0x01) ausdrücken.

Ich denke nicht, dass das bei einem modernen Compiler einen Unterschied macht. Das ==0 sollte sofort wegoptimiert werden und %2 sofort durch &1 ersetzt, falls äquivalent. Diese Äquivalenz gilt aber nur im Zweierkomplement. :p

kk

[krümelkacker]

krümelkacker schrieb:

Frank Erdorf schrieb:

2. (i%2==0) kann man schneller als (i%2) oder noch schneller als (i&0x01) ausdrücken.

Ich denke nicht, dass das bei einem modernen Compiler einen Unterschied macht. Das ==0 sollte sofort wegoptimiert werden und %2 sofort durch &1 ersetzt, falls äquivalent. Diese Äquivalenz gilt aber nur im Zweierkomplement. :p

Upps... Jetzt hast Du mich reingelegt. i%2==0 und i%2 sind natürlich nicht äquivalent. Ich schreibe auch lieber i%2==0 als !(i%2) .

[mazal]

Hier hat sich mal jemand die Mühe gemacht das in-place (d.h. nur mit konstanten zusätzlichem Speicherplatz) zu implementieren.

http://www.c-plusplus.net/forum/viewtopic-var-t-is-255964.html

[Godemit]

ui, danke für die vielen Antworten.

Zwischenstatus:
ich hatte tatsächlich auf Debug kompiliert, bei Release ist das Sortieren gut 3x so schnell (1,7 statt 5 sek)
ich hab die new-Anweisung nun komplett raus und biege nur noch die Pointer um (530ms)

Hier der Code:

struct edge
{
	int cost;
	edge *prev;
	edge *next;
};

edge* merge(edge *l1, edge *l2)
{
	edge *tmp = NULL;
	edge *start = NULL;
	while (l1 && l2)
	{
		if (l1->cost <= l2->cost){
			if (start==NULL) start = l1;
			else tmp->next = l1;
			l1->prev = tmp;
			tmp = l1;
			l1 = l1->next;
		} else {
			if (start==NULL) start = l2;
			else tmp->next = l2;
			l2->prev = tmp;
			tmp = l2;
			l2 = l2->next;
		}
	}
	while (l1){
		if (start==NULL) start = l1;
		else tmp->next = l1;
		l1->prev = tmp;
		tmp = l1;
		l1 = l1->next;
	}
	while (l2){
		if (start==NULL) start = l2;
		else tmp->next = l2;
		l2->prev = tmp;
		tmp = l2;
		l2 = l2->next;
	}
	return start;
}

edge* mergesort(edge *liste)
{
	if (liste->next){
		edge *tmp1 = NULL;
		edge *tmp2 = NULL;
		edge *l1 = NULL;
		edge *l2 = NULL;
		l1 = liste;
		l2 = liste->next;
		tmp2 = liste;
		liste = liste->next;

		while (liste){
			tmp1 = tmp2;

			tmp1->next = liste->next;
			if (liste->next) liste->next->prev = tmp1;

			tmp2 = liste;
			liste = liste->next;
		}
		l2->prev = NULL;
		return merge(mergesort(l1),mergesort(l2));
	}
	else return liste;
}

530ms ist schonmal eine sehr gute Steigerung gegenüber den 5000ms vorher, allerdings immernoch weit von dem entfernt, was ich erwarte (es sollte eigentlich etwas um die 10-20ms dauern).

die Idee, bei 10 verbleibenden Listeneinträgen auf ein O(n²)-Verfahren zu wechseln halte ich nur für bedingt sinnvoll. Werde ich aber vielleicht noch ausprobieren, wenn keinem mehr was einfällt.

Ansonsten schnupper ich mal in std::list rein. Das kannte ich garnicht

Gruß
Godemit

[drakon]

ich hatte tatsächlich auf Debug kompiliert, bei Release ist das Sortieren gut 3x so schnell (1,7 statt 5 sek)

Ja das ist Ok. Aber nichts im Vergleich, wie wenn du STL Code in Debug/Release vergleichst. Da kann es (ist z.B bei mir) von nicht lauffähig zu praktisch kein Aufwand springen.

die Idee, bei 10 verbleibenden Listeneinträgen auf ein O(n²)-Verfahren zu wechseln halte ich nur für bedingt sinnvoll. Werde ich aber vielleicht noch ausprobieren, wenn keinem mehr was einfällt.

Ich habe es jetzt nicht selbst ausprobiert, aber imo wird dir das den gewünschten boost geben. Versuchs auf jeden Fall mal. Ist ja keine Sache. Stell dir einfach mal vor, was ein Funktionsaufruf so alles an Kosten bringt im vergleich zu einer Schleife. Das sind Welten.

btw:
In tatsächlichem Code bitte zuerst std::list und std::sort benutzen. Als Übung mach das ruhig! Bin auch gespannt, wie viel das jetzt besser wird, aber in echtem Code solltest du solche Sachen eigentlich lieber die Standardbibliothek machen lassen. Effizienter, sicherer, schneller (ausser du weisst es wirklich besser, dann ist das natürlich etwas anderes).

[Godemit]

habs jetzt mit std::list und dem hauseigenen sort implementiert und liege bei 190ms. Damit werde ich jetzt wohl weiterarbeiten auch wenns ruhig noch 10x schneller sein könnte. Vielleicht brauch ich einfach nen 20 GHz Rechner

Irgendwie passiert mir das zu oft. Ich beschäftige mich ne Woche mit einem Problem und am Schluss merke ich, dass es was vorgefertigtes gibt.

Naja, wenigstens was gelernt

[drakon]

Godemit schrieb:

Irgendwie passiert mir das zu oft. Ich beschäftige mich ne Woche mit einem Problem und am Schluss merke ich, dass es was vorgefertigtes gibt.

Ich mache das oft mit Absicht.

[SeppJ]

Wenn du eine list hast, dann nimm unbedingt list.sort(), nicht std::sort(). Die Sortiermethode in list nutzt die Besonderheiten der datenstruktur aus und ist daher wesentlich effizienter.

[asdfasd]

SeppJ schrieb:

Wenn du eine list hast, dann nimm unbedingt list.sort(), nicht std::sort().

Ich denke, das merkt er spätestens am Compile Error.

[brotbernd]

Wenn die Zeien von std::list::sort wirklich kritisch sein sollten, kann man
auch auf boost::intrusive ausweichen.
http://www.boost.org/doc/libs/1_42_0/doc/html/intrusive/performance.html#intrusive.performance.performance_results_sorting

[Godemit]

hallo, ich bins mal wieder

nachdem ich mit std::list auf Dauer nicht gut zurechtkam, bin ich nochmal auf meine eigenen doppelt verketteten Liste zurück gewechselt.

Mergesort braucht nun wieder seine gewohnten 530ms. Ich hab jetzt eine Abfrage eingebaut, die bei 10 oder weniger Einträgen in der Liste auf Bubblesort wechselt. Dies ändert an der Laufzeit genau garnichts. Ich hab mit der Grenze auch schon auf etwas rumgespielt (von 2 bis 20) und es ändert sich auch nichts. Erst ab ca 20 wird die Laufzeit langsam größer (560ms) bis sie irgendwann bei ein paar Stunden ankommt (alles wird mit Bubblesort sortiert)

Allerdings könnte diese fehlende Steigerung auch wieder am Code liegen

hier der bubblesort algo. Ich fands recht kompliziert, den auf doppelt verkettete Listen anzuwenden:

edge* bubblesort(edge* list, int elemente)
{
	int i;
	edge* tmp = NULL;
	edge* start = list;
	while(elemente--){
		list = start;
		for(i = 1; i < elemente; i++){
			if(list->cost > list->next->cost) {
				// vertauschen
				tmp = list->next;
				if (list==start) start = tmp;
				else list->prev->next = tmp;
				list->next = tmp->next;
				tmp->prev = list->prev;
				list->prev = tmp;
				if (tmp->next) tmp->next->prev = list;
				tmp->next = list;
			} else list = list->next;
		}
	}
	return start;
}

und mein größeres Sorgenkind:
ich muss ja zuerst einmal herausfinden, wieviele Elemente die Liste überhaupt hat (um entscheiden zu können, ob merge- oder bubblesort). Das ist nicht ganz trivial, da ich die Liste erstmal durchlaufen muss, was ja wieder Zeit kostet.

Derzeit hab ich das so gelöst:

edge* mergesort(edge *liste)
{
    if (liste->next){
        edge *tmp2 = liste;
		int i;
		int grenze = 10;
		for (i=1;i<grenze;i++) if (tmp2->next) tmp2 = tmp2->next; else break;
		if (tmp2->next==NULL){ // listenlänge <= grenze
			return bubblesort(liste,i);
		} else {
    } else return liste;
}

ich will ja nicht die Länge der gesamten Liste wissen, sondern nur, ob sie kleiner als 10 ist, außerdem benötige ich die genaue Länge für den bubblesort algo.

Habt ihr da vielleicht nen Vorschlag, wie ich das geschickter lösen kann?

Zusätzlich ist Bubblesort nun auch nicht der idealste Algo dafür. Welcher eignet sich denn am besten für derart kurze Listen? Ich find Bogosort ganz gut, hab heute ne Glückssträhne

Wieder einmal danke für jeden Hilfe

[unskilled]

hallo mal wieder ;o)

Also ich komme mit

#include <algorithm>
#include <list>
#include <vector>
#include <iostream>

#include "my/timer.h"

int main()
{
	int count = 100*1000;
	std::vector<int> vec;
	vec.reserve(count);
	for(int i=0; i != count; ++i)
		vec.push_back(i);
	std::random_shuffle(vec.begin(), vec.end());

	std::list<int> list(vec.begin(), vec.end());
	my::timer_t timer;
	list.sort();
	std::cout << my::timer_t::get_seconds_from_ticks(timer()) << std::endl;

	std::cout << '\t' << *list.begin() << std::endl;
}

auf 0.032 Sekunden(x32) bzw 0.027 Sekunden(x64). (Core2Duo 2x3,0GHz; MSVC9; Release ohne Debugger)
int count = 10*1000*1000; : 0.5Sekunden
int count = 100*1000*1000; : 9Sekunden
Klingt schneller als deine reichliche halbe Sekunde ;P

Falls du weiterhin ein eigenes sort brauchst: std::list::splice ist dein Freund.
Ich fürchte aber, du wirst nicht schneller kommen.
Maximal noch, wenn du etwas über die Verteilung der Zahlen in deiner Liste weist.

bb