Designfrage: Pool



  • Ad aCTa schrieb:

    > weil man damit alles möglich ein deinem pool machen kann, am mutex vorbei usw..

    Was will man denn machen? Der Iterator ist konstant, der Pool kann durch ihn nicht verändert werden. Er hat nur Zugriff auf dieses eine Element, und das gehört jetzt dem Thread und muss bis zum putback nicht mehr synchronisiert werden.

    Dieses const bringt bei dem iterator garnichts.

    int t=0;
    int f(){return t++;}
    
    template <typename HTy_>
    class pool {
    public:
        typedef typename std::list<HTy_>::iterator iterator;
    
        template <typename Fnc_>
        pool(unsigned poolsize, const Fnc_& fnc)
        {
            while(--poolsize) unused_.push(fnc());
        }
    
        const iterator get() {
            used_.push_front(unused_.front());
            unused_.pop();
            return used_.begin();
        }
    
        void putback(iterator) {
            unused_.push(*iterator);
            used_.erase(iterator);
        }
    
    private:
        std::queue<HTy_> unused_;
        std::list<HTy_> used_;
    }; 
    
    int main()
    {
    	pool<int> p(3,f);
    	pool<int>::iterator ittmp = p.get();
    	pool<int>::iterator it = p.get();
    	std::cout<<*it;
    	it++;
    	std::cout<<*it;
    }
    


  • Ad aCTa schrieb:

    > Nein, du darfst keinen iterator auf den pool nach aussen geben.

    Warum?

    Weil ich dann einfach ein ++it; mache und boese Sachen machen kann. Weiters hast du ein Implementationsdetail im Interface - das ist meistens schlecht.

    Im Normalfall loest man sowas ueber Proxy Objekte. Dein iterator ist ja genauso ein Proxy objekt - nur dass er sich unnatuerlich anfuehlt und so fiese Sachen wie

    *i=some_other_resource;
    

    erlaubt.

    > Davon abgesehen hast du hier ja keinen relevanten Code, also kann man nichts sagen.

    Anwendungsfall wäre:

    Und der Code kommt dir nicht komisch vor mit dem iterator? Aber ja, in der Anwendung ist der pool trivial, du hast eine get und eine putback Methode. Das Spannende sind aber die konkreten Implementierungen.

    Weiters ist dein Ctor immernoch Fehlerhaft.
    Die used Liste kannst du in der Debug Variante ja fuehren. Dann fehlt natuerlich noch ein Dtor und eine schoene delete Funktion fuer die Resourcen.

    Dann gibts noch so kleinigkeiten wie zB der Pool sollte nicht kopierbar sein.

    Sprich du hast die Idee fuer einen Pool gehabt, das passt. Aber du musst alles zu Ende denken. Aber wenn du das Locking Fehlerfrei machst, die Poolklasse nicht kopierbar machst, die used Liste entfernst, den Fehler im Ctor fixt (uU einen dynamisch wachsenden Pool machst?), einen Dtor implementierst und die iteratoren entfernst - dann passt es so in etwa 🙂

    Und ueberdenke wie du Resourcen erstellen willst die nicht kopierbar sind.



  • > 1. CloseHandle(..) wird nicht aufgerufen bei deiner mutex Implemenation.

    Guter Punkt, das habe ich jetzt behoben. (Nutze Mutex der Windows API).

    > Weil ich dann einfach ein ++it; mache und boese Sachen machen kann.

    Das wusste ich noch garnicht! 😮 Dann ist klar, warum das nicht sicher ist...

    > Weiters hast du ein Implementationsdetail im Interface - das ist meistens schlecht.

    Ich weiß, der Code hier ist nur so zur Darstellung. Im echten Code sind sie getrennt. Ich schreibe es jetzt hier aber auch mal getrennt auf.

    Die used-Liste ist so gesehen sinnlos. Meine Idee war, dass ich aus der Queue einfach freie Handles bekomme (ohne große Schleifen mit Sachen wie if(iter->is_used()) ) und sie dann daraus entferne. Sobald der Client des pools fertig ist, soll Stack-Unwinding den Destruktor aufrufen, welcher das Handle in den Pool zurückschreibt. Der Pool speichert seine Handles in std::shared_ptr s, die dann auch von pool::get() zurückgegeben werden. Ich hätte gerne sowas:

    template <typename HTy_>
    std::shared_ptr<HTy_> pool<HTy_>::get() {
    	mutex lck(lock_);
    	std::shared_ptr<HTy> tmp(unused_.front().get(), std::bind(&pool<HTy_>::deleter, this, std::placeholders::_1)); // shared_ptr bekommen und Deleter einstellen
    	unused_.pop(); // aus Queue entfernen
    	return tmp; // Zack! (siehe unten)
    }
    
    // ...
    { // Scope
    	pool<socket> socket_pool(5, create_socket); // socket ist ein Handle-Wrapper
    	typedef std::shared_ptr<socket> socket_ptr;
    	socket_ptr socket = socket_pool.get();
    	// Service...
    } // Scope verlassen, der Deleter von shared_ptr soll das Handle in die Queue zurückschieben
    

    Der Deleter ist einfach sowas:

    template <typename HTy_>
    void pool<HTy_>::deleter(HTy* object) {
    	mutex lck(lock_);
    	unused_.push(std::shared_ptr<HTy>(object)); // neuen shared_ptr aus der Ressource erstellen + pushen
    }
    

    Oben steht "Zack!", und zwar weil ja schon der Destruktor des shared_ptr in der pool::get() samt Deleter aufgerufen wird und die Ressource in die Queue zurückschreibt, obwohl sie ja jetzt als "used" gilt. So kann das demnach nicht klappen. Wie kann ich das lösen? (Automatisches zurückschreiben in Queue)

    > die Poolklasse nicht kopierbar machst,

    Kommt danach, erst mal muss der Pool selbst funktionieren. 😉

    > den Fehler im Ctor fixt

    Fehler?



  • Ad aCTa schrieb:

    > den Fehler im Ctor fixt

    Fehler?

    while(--poolsize) unused_.push(fnc());
    

    Wieviele Objekte legst du an wenn poolsize zB 1 ist?

    ->

    while(poolsize--) unused_.push(fnc());
    

    PS:
    mir persoenlich gefaellt diese shared_ptr Variante nicht so toll weil ich diese Option lieber dem Clientcode ueberlasse, aber im Prinzip passt es so schon.


Anmelden zum Antworten