Mergesort sehr langsam



  • 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



  • 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 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) .



  • 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



  • 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



  • 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).



  • 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 🙂



  • 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. 😉


  • Mod

    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.



  • 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.



  • 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



  • 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 🙂



  • 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



  • Es geht doch noch schneller:

    template<typename T>
    void list_sort(T& to_sort)
    {
    	if(to_sort.size() < 10000)
    	{
    		to_sort.sort();
    		return;
    	}
    
    	try
    	{
    		std::vector<typename T::value_type> tmp(to_sort.begin(), to_sort.end());
    		std::sort(tmp.begin(), tmp.end());
    		to_sort.assign(tmp.begin(), tmp.end());
    		return;
    	}
    	catch(std::bad_alloc&)
    	{}
    
    	std::deque<typename T::value_type> tmp(to_sort.begin(), to_sort.end());
    	std::sort(tmp.begin(), tmp.end());
    	to_sort.assign(tmp.begin(), tmp.end());
    }
    

    if(to_sort.size() < 10000)
    Die Zahl ist einfach nur geraten. Evtl ausmessen und in ne Konstante packen.
    Wenn man zum Programmstart Zeit hat, könnte man die auch für jeden PC "ausrechnen".
    Also als static-Variable in die Fkt packen und beim ersten Aufruf nen paar Sortier-Vorgänge simulieren:

    namespace detail
    {
    	template<typename T>
    	void sort_list(T& to_sort)
    	{
    		to_sort.sort();
    	}
    
    	template<typename T>
    	void sort_and_assign(T& to_sort)
    	{
    		try
    		{
    			std::vector<typename T::value_type> tmp(to_sort.begin(), to_sort.end());
    			std::sort(tmp.begin(), tmp.end());
    			to_sort.assign(tmp.begin(), tmp.end());
    			return;
    		}
    		catch(std::bad_alloc&)
    		{}
    
    		try
    		{
    			std::deque<typename T::value_type> tmp(to_sort.begin(), to_sort.end());
    			std::sort(tmp.begin(), tmp.end());
    			to_sort.assign(tmp.begin(), tmp.end());
    			return;
    		}
    		catch(std::bad_alloc&)
    		{}
    
    		sort_list(to_sort);
    	}
    
    	template<typename S, typename T>
    	S get_performance_bound_for_list_search(const T&)
    	{
    		S ret_val = 32;
    
    		my::timer_t::tick_type ticks_list_sort;
    		my::timer_t::tick_type ticks_assign_and_sort;
    
    		do
    		{
    			if(ret_val >= std::numeric_limits<S>::max()/2)
    				return std::numeric_limits<S>::max();
    
    			ret_val *= 2;
    			std::list<S> list1, list2;
    			{
    				std::vector<S> vec;
    				vec.reserve(ret_val);
    				for(S i=0; i != ret_val; ++i)
    					vec.push_back(i);
    				std::random_shuffle(vec.begin(), vec.end());
    				list1.assign(vec.begin(), vec.end());
    				list2.assign(vec.begin(), vec.end());
    			}
    
    			my::timer_t timer;
    			detail::sort_list(list1);
    			ticks_list_sort = timer();
    
    			timer.reset();
    			detail::sort_and_assign(list2);
    			ticks_assign_and_sort = timer();
    
    		} while(ticks_list_sort < ticks_assign_and_sort);
    
    		return ret_val;
    	}
    }
    
    template<typename T>
    void list_sort(T& to_sort, bool strict_use_list_sort = false)
    {
    	static typename T::size_type performance_bound_for_list_search = detail::get_performance_bound_for_list_search<typename T::size_type>(to_sort);
    
    	if(strict_use_list_sort || to_sort.size() < performance_bound_for_list_search)
    	{
    		detail::sort_list(to_sort);
    	}
    	else
    	{
    		detail::sort_and_assign(to_sort);
    	}
    }
    
    int main()
    {
    	int count = 10*1000*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(list);
    	std::cout << my::timer_t::get_seconds_from_ticks(timer()) << std::endl;
    
    	std::cout << '\t' << *list.begin() << std::endl;
    }
    

    Zeigt, dass das list-sort immer langsamer ist als das umkopieren und kopieren (zumindest bei mir).
    Das hab ich erst nicht geglaubt und noch nen Sleep in die Umkopier-Variante gemacht, aber dann hab ichs auch geglaubt... ^^
    Aber evtl gibts ja nen System, wo ein new (erheblich) teurer ist... ^^

    Das einzige, was man jz noch machen könnte, wäre den entsprechnenden Allokator zu nutzen - aber das wird wohl kaum jmd brauchen (es sei denn, du hast irgend nen GC, den du nutzt)... ^^

    bb

    PS: Anwendung bleibt wie gehabt:

    list_sort(list);
    

    evtl sollte man die fkt auch in inplace_sort oder so umbenennen - kA...

    edit: was ich vergessen habe zu erwähnen: Bei den Tests wird list<size_type> anstatt list<value_type> genutzt - das liegt daran, dass value_type nicht zwangsläufig einen CTor besitzen muss, der einen Integer-Wert entgegennimmt - und da ist mir nix besseres eingefallen, als den size_type zu nehmen. Einfach nur value_type() zu nehmen, war mir zu gefährlich beim Testen - evtl. gibt es ja list.sort()-Implementierungen, die zu erst gucken, ob schon geordnet ist - weiterhin gibt es ja auch nicht immer einen Standard-CTor. Kurz: wenn die Objekte kompliziert(zeitaufwendig) zu kopieren sind, dann solltest du den test entsprechend anpassen...

    edit #2:
    Zeile 46-48 im zweiten Quellcode eingefügt.



  • ja, bei dem Test ist das auch erstmal kein Wunder. Immerhin ist das kopieren von ints nicht wirklich teuer - und dann ist die ganze Listenoptimierung im Eimer. Dast du aber Objekte, die nicht trivial kopierbar sind, oder die einen großen State haben, dann zieht dir die Liste sofort davon.

    Was mich viel eher interessieren würd eist, welche Probleme der Threadstarter mit der std::list hatte...



  • otze schrieb:

    ja, bei dem Test ist das auch erstmal kein Wunder. Immerhin ist das kopieren von ints nicht wirklich teuer - und dann ist die ganze Listenoptimierung im Eimer. Dast du aber Objekte, die nicht trivial kopierbar sind, oder die einen großen State haben, dann zieht dir die Liste sofort davon.

    Jopp - habe ich ja auch geschrieben.
    Ist aber eben auch kein Problem, das zu ändern - auch das habe ich aber geschrieben...
    Allerdings hat der TO ja explizit ints verwendet.

    Was mich viel eher interessieren würd eist, welche Probleme der Threadstarter mit der std::list hatte...

    ich nehme an, es lag daran, dass er list::sort nicht gefunden hatte und std::sort nicht ging und dann hat er splice nicht gefunden und konnte somit die Zeiger nicht umbiegen...

    bb



  • das Problem mit std::list hat nichts mit dem sortieren zu tun. Das funktioniert tadellos und schnell.

    Mein Problem war, dass ich im späteren Verlauf des Programms Zeiger auf einzelne Listenelemente benötigt und das überhaupt nicht hinbekommen hab.

    Daraufhin hab ich mich entschlossen, lieber eine komplett eigene Datenstruktur zu benutzen, die ich dafür vollständig verstehe.

    Vielleicht noch was über die Daten, die zu sortieren sind: Es sind integer Werte zwischen 0 und 765 mit einem sehr hohen Anteil von kleinen Werten (12% sind 0, 74% unter 10, 95% unter 100)

    Welches Verfahren sortiert mir eine Liste mit 10 solcher Werte am schnellsten?

    Gruß
    Godemit



  • Godemit schrieb:

    Mein Problem war, dass ich im späteren Verlauf des Programms Zeiger auf einzelne Listenelemente benötigt und das überhaupt nicht hinbekommen hab.

    An deiner Stelle würde ich mich dann mit std::list (und Iteratoren bei diesem speziellen Problem) vertraut machen, anstatt das Rad neuzuerfinden. Dadurch wirst du auch sehr viel Zeit sparen, denn den Umgang mit std::list hast du nach spätestens ein paar Stunden raus. In der Zeit hast du keine eigene gute, schnelle, exceptionsichere, fehlerfreie Klasse geschrieben.



  • Godemit schrieb:

    das Problem mit std::list [...] Mein Problem war, dass ich im späteren Verlauf des Programms Zeiger auf einzelne Listenelemente benötigt und das überhaupt nicht hinbekommen hab.

    list<int> intlist;
    intlist.push_back(11);                             // [11]
    intlist.push_back(22);                             // [11 22]
    intlist.push_back(33);                             // [11 22 33]
    list<int>::iterator it1 = intlist.begin();         //  ^^
    ++it1;                                             //     ^^
    list<int>::iterator it2 = intlist.insert(it1,20);  // [11 20 22 33]
                                                       //     ^^ ^^
                                                       //    it2 it1
    

    Ein Iterator funktioniert so ähnlich wie ein Zeiger. Die Iteratorn einer Liste sind bidirektional. D.h. Du kannst ++ und -- benutzen, um ein Element vor und zurück zu gehen, Du kannst sie vergleichen, Iteratoren bleiben relativ lange gültig bei std::list (im Gegensatz zu std::vector zB).



  • krümelkacker schrieb:

    Iteratoren bleiben relativ lange gültig bei std::list (im Gegensatz zu std::vector zB).

    Nicht nur relativ lange, sondern so lange das Element gelöscht wird.

    Mal was ganz anderes:
    Wieso denkst du denn, dass list die optimale Datenstruktur ist?
    Erzähl mal ein wenig, was du mit der Liste alles so anstellst... Und vll auch, wie oft circa.

    bb


Anmelden zum Antworten