Bitte um Kritik an Implementierung einer Klasse zur Vektorrechnung

SeppJ

Delther schrieb:

Sehe ich es richtig das Ausdruck wie dieser:
double length(const CVektor3 &A);
schnelleren Code erzeugt als
double length(CVektor3 A);
da das Objekt nicht kopiert werden muss?

Das ist eine sehr allgemeine Aussage, da kann man immer Gegenbeispiele finden. Tendenziell: Ja.

Man ersetzt eine Komplettkopie einer Datenstruktur mit einer Zeigerkopie. Ist die Datenstruktur größer als ein Zeiger, so ist dies einen Tacken schneller, ist sie viel größer als ein Zeiger ist es sogar viel schneller. Dafür handelt man sich jedoch eine Indirektion beim Zugriff ein, dadurch wird jeder Zugriff einen ganz kleinen Tacken langsamer. Dadurch ist es möglich, Beispiele zu konstruieren, in denen der Kopiervorteil wieder aufgefressen wird. Falls der Kopiervorteil von vornherein klein oder nicht vorhanden war, sind diese Beispiele sogar realistisch.

Das heißt diese Art der Parameterübergabe ist bei nicht primitiven Typen(bei denen es egal ist?) zu bevorzugen?

Nicht immer. Faustregel mit der ich bisher ganz gut fahre ist alles über 2-3 Pointergrößen als Referenz zu übergeben, sowie alle Datenstrukturen unbekannter Größe.

Dein Vektor fällt da genau auf die Grenze, ich würde aber tendenziell die Referenz nehmen. Da die Funktionen sehr einfach sind und daher sowieso inline sein sollten (das solltest du übrigens noch nachholen! Definier deine Einzeilerfunktionen im Header.) darf man da auf komplette Optimierung durch den Compiler hoffen, egal was von beidem man macht.

P.S.: Statt getter/setter und public-Membern würde ich stattdessen den operator[] überladen.
P.P.S.: Letzteres macht jedoch erst so richtig Sinn, wenn die Dimension auch flexibel ist. Wenn du dich auf 3er-Vektoren festlegst, dann bleib ruhig bei dem wie es ist.

dot

SeppJ schrieb:

Definier deine Einzeilerfunktionen im Header.) darf man da auf komplette Optimierung durch den Compiler hoffen, egal was von beidem man macht.

Stimmt, das ist in dem Fall sicherlich eine gute Idee. Ich wollt auch schon darauf hinweisen, aber meiner Erfahrung nach machts bei MSVC dank LTCG keinen Unterschied, der Compiler wird so oder so inlinen. Hast du da vielleicht Erfahrungswerte, wie das mit gcc aussieht? Aber spätestens wenn er das Ding zu einem Template macht, bleibt ihm eh nix andres übrig...

SeppJ

dot schrieb:

SeppJ schrieb:

Definier deine Einzeilerfunktionen im Header.) darf man da auf komplette Optimierung durch den Compiler hoffen, egal was von beidem man macht.

Stimmt, das ist in dem Fall sicherlich eine gute Idee. Ich wollt auch schon darauf hinweisen, aber meiner Erfahrung nach machts bei MSVC dank LTCG keinen Unterschied, der Compiler wird so oder so inlinen. Hast du da vielleicht Erfahrungswerte, wie das mit gcc aussieht? Aber spätestens wenn er das Ding zu einem Template macht, bleibt ihm eh nix andres übrig...

Der GCC kann ja auch Linkzeitoptimierung. Ich habe jedoch keine Studien darüber durchgeführt, wie sich Linkzeitoptimierung gegenüber explizitem inline verhält. Bei mir sind solche Funktionen sowieso immer inline, so dass man auch ohne LTO auf der sicheren Seite ist. Wenn ich inlining jedoch explizit ausschalte (selbst wenn andere Optimierungen an bleiben) kann man sofort merken, dass mein üblicher Code gleich um mehrere Größenordnungen langsamer wird. In C++ hat man so oft triviale Funktionen, die einfach nur der Abstraktion dienen. Wenn die auf einmal Laufzeitkosten verursachen, dann haut das richtig rein.

P.S.: Ein Kollegen, dem ich LTO mal erklärt habe (damals, als das noch neu war), hat sein Programm dadurch angeblich um 670% beschleunigt. Ich kenne seinen Code nicht, aber ich vermute mal, dass dort ein ähnlicher Fall vorlag mit vielen kleinen Funktionen, die nicht inline waren.

Werner Salomon

Außer den Themen, die schon genannt worden sind, habe ich noch ein paar Bemerkungen.

- Die Methoden

/*Vektor + Skalar*/
CVektor3 CVektor3::operator+(const double &N)
{return(  CVektor3(m_x + N,m_y + N,m_z + N)  );}

/*Vektor - Skalar*/
CVektor3 CVektor3::operator-(const double &N)
{return(  CVektor3(m_x - N,m_y - N,m_z - N)  );}

machen mathematisch gar keinen Sinn. Ich habe noch nie eine Anwendung gesehen, wo man so etwas benötigt. Ich würde sie einfach weglassen. Es sei denn, Du erzählst uns wozu sie dienen.

- Die Methode

/*Eingeschlossener Winkel*/
double CVektor3::winkel(CVektor3 B)
{return(  acos(dot(B)/(length()*B.length()))  );}

sieht harmlos aus, ist aber furchtbar inperformant. In length() wird zwangsläufig die Wurzel (sqrt) aufgerufen. Und das auch noch zweimal. Besser wäre hier

/*Eingeschlossener Winkel*/
double CVektor3::winkel(CVektor3 B)
{ return acos(dot(B)/dot(*this)); }

bzw.

/*Eingeschlossener Winkel*/
double CVektor3::winkel(CVektor3 B)
{ return acos(dot(*this, B))/dot(*this, *this); }

wenn dot eine Funktion ist und 2 Parameter hätte.
Damit sparst Du 4 Multiplikationen, zweimal die Wurzel und 2 Additionen, wenn ich mich nicht verrechnet habe.

- Bei allen Methoden, die das Vektor-Objekt nicht verändern, solltest Du const hinzufügen (const correctness), Das gilt in jeden Fall für eventuelle getter und auch für die Methoden

/**Vektorfunktionen*/
        double length() const;                 /** Betrag */
        CVektor3 unit() const;                 /** Einheitsvektor*/
        double dot(CVektor3 B) const;          /**Skalarprodukt*/
        CVektor3 kreuz(CVektor3 B) const;      /**Kreuzprodukt*/
        double winkel(CVektor3 B) const;       /**eingeschlossener winkel*/

Das steht auch in dem Buch von Scott Meyers, welches Dir dot empfohlen hat und hier ist auch ein Artikel zum Thema const.

- Wenn Du bei einer Klasse arithmetische Operator (z.B. + und -) implementierst, die als Ergebnis wieder ein Objekt dieser Klasse haben, so solltest Du auch immer die passenden =-Operatoren hinzufügen. Also hier

CVektor3& operator+=( const CVektor& b );

Da hierbei natürlich das selbe herauskommen sollte, wie beim operator+ mit zwei Parametern, kannst Du sie auch so implementieren, dass sie sich gegenseitig aufrufen. Wenn der Zuweisungsoperator implementiert wird, so kann die frei Verknüpfung für + z.B. so aussehen

CVektor3 operator+( CVektor a, const CVektor& b ) {
    return a += b;
}

Diese Konstruktion hätte aber Nachteile bei der RVO des Compilers. Aber das lass Dir vielleicht von anderen erklären.
Zum Thema hat pumuckl bereits drei Artikel im C++-Magazin geschrieben und die boost.operators-Lib ist auch noch interessant.

- Im Gegensatz zu manch' anderen bin ich der Meinung, dass die setter-Methoden beim Vektor völlig überflüssig sind - bei private members versteht sich. Ein CVektor3::set(x,y,z) kann leicht durch

CVektor3 a = ...;
    a = CVektor(x,y,z);

realisiert werden. Ich bezweifele auch dass das in irgendeiner Weise Performance-Verluste aufweist.

- hier findest Du noch einen ähnlichen Thread zum Vektor und auch meinen Beitrag dazu.

Gruß
Werner

Werner Salomon

.. noch was vergessen:

#include <math.h>

ist veraltet. Nach dem C++-Standard sollte es #include <cmath> heißen, dann kommen die mathematischen Funktionen in den namespace std. Beim Aufruf von acos oder sqrt muss man dann std::acos bzw std::sqrt schreiben.

- die Berechnung der Länge des Vektors würde ich als freie (friend)Funktion implementieren und sie abs nennen. Dann ist das äquivalent zu dem std::abs aus <cmath>.

Gruß
Werner

Werner Salomon schrieb:

Die Methoden machen mathematisch gar keinen Sinn. Ich habe noch nie eine Anwendung gesehen, wo man so etwas benötigt. Ich würde sie einfach weglassen. Es sei denn, Du erzählst uns wozu sie dienen.

Da hast du schon recht, ich vermute an der Stelle habe ich MATLAB imitiert.

Dieses hier verstehe ich nicht und bekomme da auch nicht das Ergebnis heraus:

Werner Salomon schrieb:

Besser wäre hier
/*Eingeschlossener Winkel*/
double CVektor3::winkel(CVektor3 B)
{ return acos(dot(B)/dot(*this)); }
bzw.
/*Eingeschlossener Winkel*/
double CVektor3::winkel(CVektor3 B)
{ return acos(dot(*this, B))/dot(*this, *this); }
wenn dot eine Funktion ist und 2 Parameter hätte.
Damit sparst Du 4 Multiplikationen, zweimal die Wurzel und 2 Additionen, wenn ich mich nicht verrechnet habe.

Ich konnte nur eine Wurzel sparen:

double winkel(const CVektor3 &A,const CVektor3 &B)
{return(  acos((A*B)/sqrt(A*A*B*B)  ));}

Die Funktion dot() habe ich abgeschafft und dafür den * operator überladen.

Werner Salomon

Delther schrieb:

Ich konnte nur eine Wurzel sparen:

double winkel(const CVektor3 &A,const CVektor3 &B)
{return(  acos((A*B)/sqrt(A*A*B*B)  ));}

stimmt - ich hatte mich da völlig vertan. Deine Lösung ist fast korrekt - klammere noch die Skalarpodukte; also:

double winkel(const CVektor3 &A,const CVektor3 &B)
{return(  acos((A*B)/sqrt((A*A)*(B*B))  ));}

sonst ist die Abarbeitung nicht eindeutig. Das Ergebnis ist aber das gleiche.

Delther schrieb:

Die Funktion dot() habe ich abgeschafft und dafür den * operator überladen.

so habe ich es auch gemacht.

Gruß
Werner

Werner Salomon schrieb:

wenn dot eine Funktion ist und 2 Parameter hätte.

Ja fragen wir ihn doch!

SCNR

meine meinung zum stil: CVektor ist ein furchtbarer bezeichner. das C-präfix nervt, und Vektor ist deutsch. deutsche bezeichner sind die pest. class Vector3 und gut ist. ist natürlich eine geschmacksfrage.

das ganze zum template zu machen wirft die frage auf, welchen typ funktionen bzw. methoden entgegennehmen sollen, die mit übergebenen vektoren arbeiten? das geht u.u. bis zur gpu runter. müssen das dann auch alles template-funktionen werden?

ich habe das vor vielen jahren auch mal versucht so zu implementieren, und das vector-template hat sich als eine seuche herausgestellt, die kaum einzudämmen war. alles mögliche wurde zum template. am ende wurde ein typedef vector3<float> vector3f; draus und damit haben alle komponenten gearbeitet. den aufwand, integer- und fließkommazahlen unter einen hut zu bringen, hätte ich mir da sparen können.

Eisflamme

vektoriant:
Ich habe in meinem Projekt ein globales typedef namens "UsualVector" als "typedef vector<3, float>". Wenn ich irgendwann merke, dass mir die Präzision nicht reicht, kann ich einfach dieses typedef ändern und float durch double ersetzen. Ich sehe da kein Problem, man muss ja nicht überall den ganzen Templatenamen hinschreiben.