Funktion schneller machen



  • Abend!

    Ich habe in meiner 3D-Anwendung eine Funktion, die sehr oft aufgerufen wird (in jedem Frame einmal) und die leider zu langsam ist. Wenn ich die Funktion rausnehme, habe ich ca 800fps, wenn ich sie reinnehme bricht die Performance ein auf ca 30 fps. Die Funktion sieht so aus:

    void ModifyHeightAction::addVertices(const std::vector<VertexIndexAndHeightPair>& verts) {
       VertexMapIterator iter;
    
       VertexIndexAndHeightPair vert;
       Index i;
       Heights h;
    
       std::vector<VertexIndexAndHeightPair>::const_iterator it	= verts.begin();
       std::vector<VertexIndexAndHeightPair>::const_iterator itEnd	= verts.end();
       for(; it != itEnd; ++it) {
          i.row = it->vertexRow;
          i.col = it->vertexCol;
          iter = mVertexMap.find(i);   // mVertexMap ist vom Typ std::map<Index, Heights>
    
          if(iter == mVertexMap.end()) {
             h.initial = it->initialHeight;
             h.modified = it->modifiedHeight;
             mVertexMap[i] = h;
          }
          else {
             iter->second.modified = it->modifiedHeight;
          }
       }
    }
    // Das hier sind alle beteiligten Strukturen:
    // Index/Höhen Pärchen
    struct VertexIndexAndHeightPair {
       int  vertexRow, vertexCol;	
       float initialHeight, modifiedHeight;		
    };
    
    struct Index { int row, col; };
    
    bool operator<(const Index& lhs, const Index& rhs) {
       if(lhs.row < rhs.row)
          return true;
       else if(lhs.row > rhs.row)
          return false;
       return lhs.col < rhs.col;
    }
    
    struct Heights { float initial, modified; };
    

    Die Funktion kriegt einen vector mit Strukturen, die ein Index/Höhen Pärchen darstellen. Was die Funktion jetzt machen soll ist sehr simpel: Die Klasse ModifyHeightAction hat eine std::map namens mVertexMap als Member (der Typ ist in Zeile 13 dargestellt). Die Funktion soll jetzt über alle Elemente des Eingabevektors gehen und wenn die Map noch kein Element mit diesem Index besitzt, soll ein Eintrag in die Map geschrieben werden (Key = Index, Value = die 2 Höhen). Befindet sich bereits ein Element mit diesem Index in der Map, soll nur der 2. Höhenwert (namens modified) geupdated werden. Es ist völlig egal ob die Keys sortiert sind. Es geht mir nur darum, dass jeder Index maximal 1mal in der Map ist (und wenn der selbe Index erneut im Vektor erscheint eben modified angepasst wird).

    Wie ich es implementiert habe seht ihr ja im Code. Ich gehe mit einem Iterator über den vector, suche in der Map nach dem Index (des Elements, auf den der Iterator gerade zeigt) und wenn er noch nicht in der Map ist, füge ich das Höhenpärchen in die Map ein.

    Lustigerweise scheint das find() garnicht der Flaschenhals zu sein, sondern das Einfügen in die Map in Zeile 18, also: mVertexMap[i] = h;
    Allerdings weiß ich nicht, wie ich das beschleunigen sollte?

    Hat irgendwer eine Idee, wie ich die Funktion schneller implementieren könnte? Vllt ein anderer Algo oder eine andere Datenstruktur oder..?

    Danke!



  • Die Geschichte schaut vom Design her schon sehr ineffizient aus. Dass du überhaupt die Struktur so zerlegen musst.

    Aber als einfache Lösung könntest du std::(tr1::)unordered_map anstelle von std::map probieren. Und insert und nicht map[n] = h; zum einfügen benutzen.



  • Zum Design: Naja, ich hatte ursprünglich ein "simpleres" Design. Als Zieldatenstruktur hatte ich statt einer std::map einfach einen std::vector<VertexIndexAndHeightPair>. Das hinzufügen (mit push_back) war natürlich sehr schnell, nur war die Suche mittels find(), ob der Index bereits in dem vector ist unglaublich langsam. Nur deshalb habe ich auf eine map umgestellt und es ist auch bereits deutlich schneller, aber noch immer nicht schnell genug :|

    Zu der unordered_map: Muss ich mir da eine eigene Hash Funktion für Index schreiben?



  • marcD22 schrieb:

    Abend!

    Ich habe in meiner 3D-Anwendung eine Funktion, die sehr oft aufgerufen wird (in jedem Frame einmal)

    Warum? Du must doch deine mVertexMap nur updaten, wenn sich was ändert. Und dann auch nur das was sich ändert. Übrigens Release kompilierst du schon, oder?



  • preoptimal schrieb:

    marcD22 schrieb:

    Abend!

    Ich habe in meiner 3D-Anwendung eine Funktion, die sehr oft aufgerufen wird (in jedem Frame einmal)

    Warum? Du must doch deine mVertexMap nur updaten, wenn sich was ändert. Und dann auch nur das was sich ändert. Übrigens Release kompilierst du schon, oder?

    Keine Sorge, ich weiß schon wann ich sie aufrufen muss. Und ja, ich kompiliere im Release Modus. 🙄



  • marcD22 schrieb:

    preoptimal schrieb:

    marcD22 schrieb:

    Abend!

    Ich habe in meiner 3D-Anwendung eine Funktion, die sehr oft aufgerufen wird (in jedem Frame einmal)

    Warum? Du must doch deine mVertexMap nur updaten, wenn sich was ändert. Und dann auch nur das was sich ändert. Übrigens Release kompilierst du schon, oder?

    Keine Sorge, ich weiß schon wann ich sie aufrufen muss.

    na dann viel spass



  • marcD22 schrieb:

    Als Zieldatenstruktur hatte ich statt einer std::map einfach einen std::vector<VertexIndexAndHeightPair>. Das hinzufügen (mit push_back) war natürlich sehr schnell, nur war die Suche mittels find(), ob der Index bereits in dem vector ist unglaublich langsam.

    Du könntest vielleicht die Index-Struktur wegschmeissen und stattdessen für den Zugriff einen ganzzahligen Index nehmen: index = zeile * breite + spalte; . Voraussetzung ist, dass du einen Integer-Typen hast, in den Breite * Höhe sicher reinpasst.

    Je nach Größe von Breite * Höhe könntest du als weiteren Schritt die Map sparen und mit dem Index einen vector direkt addressieren. Falls du einen vector dieser Größe anlegen kannst.



  • btw:

    bool operator< (const Index &lhs, const Index &rhs)
    {
      if(lhs.row == rhs.row)
        return lhs.col < rhs.col;
    
      return lhs.row < rhs.row;
    }
    

    bb



  • Registrierter Troll schrieb:

    marcD22 schrieb:

    Als Zieldatenstruktur hatte ich statt einer std::map einfach einen std::vector<VertexIndexAndHeightPair>. Das hinzufügen (mit push_back) war natürlich sehr schnell, nur war die Suche mittels find(), ob der Index bereits in dem vector ist unglaublich langsam.

    Du könntest vielleicht die Index-Struktur wegschmeissen und stattdessen für den Zugriff einen ganzzahligen Index nehmen: index = zeile * breite + spalte; . Voraussetzung ist, dass du einen Integer-Typen hast, in den Breite * Höhe sicher reinpasst.

    Je nach Größe von Breite * Höhe könntest du als weiteren Schritt die Map sparen und mit dem Index einen vector direkt addressieren. Falls du einen vector dieser Größe anlegen kannst.

    Ja, den Gedanken hatte ich auch schon. Wie du richtig erkannt hast, ist mein Index ein Index in einem N x N 2D Gitter, dessen Ausmaße ich kenne. Ich könnte also wie du schon gesagt hast, meinen 2D index umrechnen in einen 1D Index: index = row * breite + column und index als index in einem vector benutzen. Das Problem ist nur: Mein Gitter kann eine Größe bis zu 4000 x 4000 haben und dann hätte ich vektoren mit 16.000.000 Einträgen, obwohl sich die Indices, die ich an addVertices() schicke, sich immer in einem relativ kleinen Block befinden (sprich: es kommt nicht vor, dass ich Indices an addVertices() schicke, die total zerstreut über das ganze Gitter sind. Die Indices liegen alle dicht zusammen in einem bestimmten Bereich (meistens so 30x30 Bereiche) des großen Gitters).
    Dieser Ansatz wird also wegen des Speicheraufwandes nicht gehen, oder?

    Zu der hash_map: Irgendwie finde ich da recht wenig Anleitungen dazu. Sehe ich das richtig, dass unordered_map und hash_map das selbe ist? Da mein Key ja der struct Index sein wird, muss ich mir ja eine Hashfunktion schreiben, oder?
    Was ich bei Hashtabellen nie verstanden habe: Sagen wir meine Hashfunktion sieht so aus:

    int hashFunction(const Index& index) {
       return index.vertexRow * width + index.vertexCol;
    }
    

    Bei einem 4k x 4k Gitter, liefert diese Funktion also Werte im Intervall [0,15999999]. Wieviel Speicher allokiert dann die hashmap? Ich mein, wenn ich jetzt den Index (3999,3999) speichern will, dann liefert die Hashfunktion ja 15999999, dann ist also der Index für diesen Key gleich 15999999, dann muss das Array ja mindestens 16000000 Elemente haben, oder? 😕



  • marcD22 schrieb:

    Dieser Ansatz wird also wegen des Speicheraufwandes nicht gehen, oder?

    Das kannst nur du wissen. Rechne den Speicherbedarf aus und dann entscheide selbst, ob das geht oder nicht. Aktuelle Rechner haben in der Regel mind. 2 GB Speicher.

    Bei einem 4k x 4k Gitter, liefert diese Funktion also Werte im Intervall [0,15999999]. Wieviel Speicher allokiert dann die hashmap? Ich mein, wenn ich jetzt den Index (3999,3999) speichern will, dann liefert die Hashfunktion ja 15999999, dann ist also der Index für diesen Key gleich 15999999, dann muss das Array ja mindestens 16000000 Elemente haben, oder? 😕

    Eine Map ist kein Array. Die Map wird näherungsweise so groß wie die Summe der Elemente die drinstecken.

    Vielleicht wäre hier ein QuadTree o. ä. als Datenstruktur besser geeignet.



  • Registrierter Troll schrieb:

    Bei einem 4k x 4k Gitter, liefert diese Funktion also Werte im Intervall [0,15999999]. Wieviel Speicher allokiert dann die hashmap? Ich mein, wenn ich jetzt den Index (3999,3999) speichern will, dann liefert die Hashfunktion ja 15999999, dann ist also der Index für diesen Key gleich 15999999, dann muss das Array ja mindestens 16000000 Elemente haben, oder? 😕

    Eine Map ist kein Array. Die Map wird näherungsweise so groß wie die Summe der Elemente die drinstecken.

    Hm, wie gesagt, ich versteh nicht wirklich WIE eine Hashmap intern nun speichert. Ich glaube mal gelesen zu haben, dass sie ein Array benutzt.
    Das läuft doch so: Wenn ich einer hashmap m ein Key/Value Pärchen hinzufüge:
    m.insert( value_type(key, value) )
    dann berechnet die hashmap den hash per: int hash = hashFunction(key). Ich dachte, dass eine hashmap jetzt ein Array hat, wobei jedes Element ein Zeiger auf irgend eine Datenstrukur ist (sagen wir mal vector), weil es ja zu Kollisionen kommen kann. Ich dachte also, die speicherung sieht in etwa so aus:
    std::vector<Value_Type>* data[???];
    data[hash]->push_back(value);
    Aber offenbar wird das ja nicht so gemacht, oder? Weil sonst bräuchte ja meine hashmap ein array mit 16000000 Einträgen, da ja hashFunction( (3999,3999) ) den hash 15999999 liefert und die map jetzt data[15999999]->push_back(...) machen müsste.



  • http://de.wikipedia.org/wiki/Hashtabelle
    sollte eigtl deine - mir nicht ganz ersichtliche - Frage beantworten...

    Vll meinst du das
    http://de.wikipedia.org/wiki/Hashtabelle#Dynamisches_Hashing

    ansonsten könntest du auch noch meinen, wie das ganze intern gespeichert wird...
    da eine hashmap im best-case eine zugriffszeit von O(1) hat, wird ein Array der (maximal-)Länge hashfkt_max wohl unabdingbar sein...
    Allerdings kann das mittels Pointern ja relativ klein gehalten werden...

    template <
      typename key_type,
      typename mapped_type
    //...
    >
    class map
    {
      std::vector< mapped_type* > hash_map;
    
      size_type hash_fkt(const key_type &key)
      {
        return /*wie auch immer so ne fkt aussehen könnte*/ size_type(key);
      }
    
      map()
      {}
    
      iterator real_insert(const std::pair<key_type, mapped_type> &to_add)
      {
        std::size_t index = hash_fkt(to_add.first());
    
        hash_map.reserve(index+1);
        while(index >= hash_map.size())
          hash_map.push_back(nullptr);
    
        if(! hash_map[index])
          hash_map[index] = new mapped_type(to_add.second());
        else
          //kollisionserkennung - siehe wikipedia
    
        return iterator(&hash_map[index]);
      }
    };
    

    so würde man auf 32bit Systemen mit einer Hash-Fkt die keine Kollisionen erzeugt max. 16'000'000*4 / 2^20 MB Speicher am Stück brauchen (ein wenig mehr als 60MB) - unabhängig von mapped_type.
    Außerdem bräuchte man nich mal ein swap für mapped_type.

    bb



  • @unskilled: Das mir an einer Hashtabelle nie so ganz klar, sieht man auch in deinem Code. Nämlich die Zeile 22: hash_map.reserve(index+1); Das Problem ist ja, dass die Klasse Map nicht wissen, was der Maximalwert meiner Hash Funktion ist (in meinem Fall 15999999).
    Dh im schlimmsten Fall ist jeder eingefügte Key größer als der davor und du würdest in deinem Code ständig reserve() machen, mit all den Problemen die dazu gehören (Werte umkopieren etc).

    Was ich nicht verstehe: Ich habe jetzt eine Hashmap genommen und habe folgende Hashfunktion:

    int hashFunction(const Index& index) {
       return index.vertexRow * width + index.vertexCol;
    }
    

    Width ist bei mir 1025 (ich habe ein 1025 x 1025 Gitter). Das bedeutet also, dass meine Funktion injektiv ist. Index (2,3) wird auf 2053 abgebildet und KEIN anderer Index kann auf 2053 abgebildet werden. Da müsste dann doch sowohl find() also auch insert() in die Map O(1) sein? Ist es aber offenbar nicht, denn der Code ist nach wie vor langsam. Wenn ich den insert:
    mHashMap.insert( HashMap::value_type(i, h) ); auskommentiere, dann ist der Code wieder schnell.
    Wie kann das insert() so lahm sein, wenn ich doch eine Hashfunktion habe, die niemals Kollisionen erzeugt? 😕



  • marcD22 schrieb:

    Das Problem ist ja, dass die Klasse Map nicht wissen, was der Maximalwert meiner Hash Funktion ist (in meinem Fall 15999999).

    Wozu muss sie sowas denn bitte wissen? Das braucht man doch nirgendwo...

    marcD22 schrieb:

    Nämlich die Zeile 22: hash_map.reserve(index+1);
    Dh im schlimmsten Fall ist jeder eingefügte Key größer als der davor und du würdest in deinem Code ständig reserve() machen, mit all den Problemen die dazu gehören (Werte umkopieren etc).

    Das ist richtig - schlimmstenfalls wird also aller <vernachlässigbar> aufrufe <vernachlässigbar> viel kopiert...
    gehen wir mal von 1'000'000 Zeigern aus - bei einer Verdoppelung des Vector-Speicherplatzes (wovon immer gern ausgegangen wird) haben wir hier die nächste Umkopier-aktion, nachdem wir 1'000'000 Zeiger hinzugefügt(aber keine entfernt) haben. Wir würden dann 1*10^6*4 / 1024 KB kopieren:
    ca. 3900KB = <4MB
    Jz kannst du mal gucken, wie schnell der Durchschnitts-RAM lesen und schreiben kann... Hab leider auf die Schnelle keine Werte gefunden - sollte aber im Gbps-Bereich liegen...
    also:
    aller n zusätzlicher werte wird ein von n abhängiger kleiner speicherblock kopiert...
    das macht map auch zum nicht-optimalen container bei verhältnismäßig vielen einfüge-operationen und wenig find`s...

    marcD22 schrieb:

    Was ich nicht verstehe: Ich habe jetzt eine Hashmap genommen und habe folgende Hashfunktion:

    int hashFunction(const Index& index) {
       return index.vertexRow * width + index.vertexCol;
    }
    

    Width ist bei mir 1025 (ich habe ein 1025 x 1025 Gitter). Das bedeutet also, dass meine Funktion injektiv ist. Index (2,3) wird auf 2053 abgebildet und KEIN anderer Index kann auf 2053 abgebildet werden. Da müsste dann doch sowohl find() also auch insert() in die Map O(1) sein?

    find() auf jeden fall
    insert() hat wie oben beschrieben im worst-case eine laufzeit von O(n), aber du kannst im Durchschnitt mit O(1) rechnen...

    marcD22 schrieb:

    Ist es aber offenbar nicht, denn der Code ist nach wie vor langsam. Wenn ich den insert:
    mHashMap.insert( HashMap::value_type(i, h) ); auskommentiere, dann ist der Code wieder schnell.

    Ich weiß, du hattest schon mal gesagt, dass du den release-Modus nutzt - immernoch?

    marcD22 schrieb:

    Wie kann das insert() so lahm sein, wenn ich doch eine Hashfunktion habe, die niemals Kollisionen erzeugt? 😕

    1. woher soll der statische code das wissen?
    2. siehe oben... http://images.google.de/images?hl=de&source=hp&q=container choice&um=1&ie=UTF-8&sa=N&tab=wi

    Evtl ist hier eine eigene Datenstruktur keine dumme Idee...
    So, wie es sich anhört reicht ja:

    struct 2d_arr
    {
    private:
      size_type x;
      std::vector<T> data;
    
    public:
      2d_arr(size_type x, size_type y)
      : x(x),
        data(x*y)
      {}
    
      const T& operator() (size_type i, size_type j) const
      {
        const_iterator it = data.begin();
        std::advance(it, i+j*x);
        return *it;
      }
    
      T& operator() (size_type i, size_type j)
      {
        iterator it = data.begin();
        std::advance(it, i+j*x);
        return *it;
      } 
    };
    

    hat ne garantiert konstante Laufzeit - bei einfüge und lese-operationen...

    bb


Log in to reply