Komplizierte Konstrukte auflösen ohne Tempoeinbußen

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Huhu!

Ich habe beispielsweise sowas hier:

          StringReq::Char::Paint::R_new[ObjPos] = StringReq::Char::Paint::R_fgrnd[ObjPos] * (StringReq::Char::Paint::alpha_last[ObjPos] * 16)
            / StringReq::Char::Paint::R_bkgrnd[ObjPos] * (255 - (StringReq::Char::Paint::alpha_last[ObjPos] * 16));
          StringReq::Char::Paint::G_new[ObjPos] = StringReq::Char::Paint::G_fgrnd[ObjPos] * (StringReq::Char::Paint::alpha_last[ObjPos] * 16)
            / StringReq::Char::Paint::G_bkgrnd[ObjPos] * (255 - (StringReq::Char::Paint::alpha_last[ObjPos] * 16));
          StringReq::Char::Paint::B_new[ObjPos] = StringReq::Char::Paint::B_fgrnd[ObjPos] * (StringReq::Char::Paint::alpha_last[ObjPos] * 16)
            / StringReq::Char::Paint::B_bkgrnd[ObjPos] * (255 - (StringReq::Char::Paint::alpha_last[ObjPos] * 16));

          System::SetPixel(StringReq::FrameBuffer[ObjPos],
            StringReq::Char::CursorPosition[ObjPos] + (x_byte * 2) + 1, y_pxl,
            StringReq::Char::Paint::R_new[ObjPos], StringReq::Char::Paint::G_new[ObjPos], StringReq::Char::Paint::B_new[ObjPos]);

Für die Lesbarkeit ist dies natürlich eine Katastrophe biblischen 
Ausmaßes. In einer bestimmten Klasse sieht das durchgehend so schlimm 
aus.
Ich könnte natürlich erst mal auf die relevanten Stellen Pointen, aber 
wie schaut es da mit dem Tempo aus? Im Debugbuild wird das wohl alles so 
wie es ist kompiliert. Wie schaut es da aber im Releasebuild mit -03 
aus? Mir ist das Tempo an dieser Stelle wichtiger als die Lesbarkeit.
Dann bleibt auch noch die Möglichkeit zusätzliche Methoden aufzurufen um 
alles sich Wiederholende da reinzupacken. Zusammen mit den Pointern wäre 
das für mich die optimalste Lösung, WENN das Tempo darunter nicht leidet 
(wie gesagt, im Releasebuild). Aber werden Funktionsmethoden überhaupt 
optimiert?

Also im Prinzip: Ist es möglich die Lesbarkeit zu verbessern, ohne, dass 
das Tempo darunter leidet? Falls ja, worauf muss geachtet werden?


Vielen lieben Dank schonmal :)
Grüße
Reggie


Edit: achso, gcc 5.3.0 und läuft auf einem stm32

[Peter II (Gast)]

wenn ein Term 6mal vorkommt, dann würde ich ihn zwischenspeichern.

Auto xx StringReq::Char::Paint& StringReq::Char::Paint;
Auto x = xx::alpha_last[ObjPos] * 16;


xx::R_new[ObjPos] = xx::R_fgrnd[ObjPos] * x / xx::R_bkgrnd[ObjPos] * (255 - x);

xx::G_new[ObjPos] = xx::G_fgrnd[ObjPos] * x / xx::G_bkgrnd[ObjPos] * (255 - x );

xx::B_new[ObjPos] = xx::B_fgrnd[ObjPos] * x / xx::B_bkgrnd[ObjPos] * (255 - x );

ist jetzt zwar C++, aber sollte ich C auch kein Problem sein. Das ist 
zumindest besser lesbar. Langsamer ist es auf jeden Fall auch nicht. Ob 
es schneller ist hängt vom Compiler ab.

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Peter II schrieb:
> Auto xx StringReq::Char::Paint& StringReq::Char::Paint;
Peter II schrieb:
> Langsamer ist es auf jeden Fall auch nicht.
Ist es nicht so, dass während des Programmablaufs erst jedes mal xx 
initialisiert wird?

[Peter II (Gast)]

Reginald L. schrieb:
> Ist es nicht so, dass während des Programmablaufs erst jedes mal xx
> initialisiert wird?

ja und nein. Der wert wird in ein Register gespeichert, das muss der 
Compiler aber sowieso machen. Im schlimmsten fall macht er es bei deiner 
Version 6mal (wenn nicht genügend Register frei sind).

Wenn der Compiler der Meinung ist, das er xx gar nicht braucht kann er 
es ja immer noch wegoptimieren.

Wenn du dir nicht sicher bist, dann musst du im ASM code nachschauen.

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Peter II schrieb:
> ja und nein. Der wert wird in ein Register gespeichert, das muss der
> Compiler aber sowieso machen. Im schlimmsten fall macht er es bei deiner
> Version 6mal (wenn nicht genügend Register frei sind).
Es bleibt einem aber auch nichts erspart, da muss ich mich wohl in die 
Materie einlesen :>

Peter II schrieb:
> dann musst du im ASM code nachschauen.
Hehe, dachte ich mir schon, dass es darauf hinausläuft.


Vielen Dank erstmal, ich schließe daraus: Ich muss lernen wie die CPU 
arbeitet :>

[Peter II (Gast)]

Reginald L. schrieb:
> Vielen Dank erstmal, ich schließe daraus: Ich muss lernen wie die CPU
> arbeitet :>

nein, die Frage ist wie schlau der Compiler ist.

Ich habe bis jetzt immer gute Erfahrung mit Referenzen bei solchen 
Konstukten gemacht, manchmal stellen sich die Compiler sonst zu dumm an.

[nicht“Gast„ (Gast)]

Moin,

Sind das alles etwa namespaces oder hast du Schlingel einen Haufen 
statische Methoden?

Für die Lesbarkeit müsstest du ein wenig mehr Posten. Meistens ist es 
kein einzelner Abschnitt, den man nur verbessern muss, sondern es ist 
etwas mehr rework fällig.

Beschreiben doch mal den Projekt etwas genauer. Also nur den Teil, mit 
dem du zeichnen möchtest. Dann ist es auch einfacher einen sinnvollen 
Tip abzugeben.

Grüße

[Oliver S. (oliverso)]

Peter II schrieb:
> ein, die Frage ist wie schlau der Compiler ist.

Für die paar einfachen Berechnungen ist der allemal schlau genug.
Um den Quelltext lesbarer zu machen, könnte das "using"-Keyword helfen.

Oliver

[Torsten R. (Firma: Torrox.de) (torstenrobitzki)]

Reginald L. schrieb:
> Also im Prinzip: Ist es möglich die Lesbarkeit zu verbessern, ohne, dass
> das Tempo darunter leidet?

Ausprobieren! Optimieren bedeutet: Code ändern. Code möchtest Du aber 
nur ändern, wenn er lesbar ist und damit die Änderungen nachvollziehbar 
sind und die Gefahr, dass etwas kaput geht minimal ist (im Idealfall 
hast Du Unit-Tests, die nahelegen, dass nix kaput gegangen ist).

Fang mit lesbarem und korrekten Code an (und nein, lesbar bedeutet nicht 
naive). Dann misst Du einfach mit einem Profiler, optimierst und misst 
wieder mit dem Profiler. Fertig!

Die Mikro-Optimierungen, über die Du Dir hier Gedanken machst, bekommen 
die Compiler alle von alleine hin. (Interessanter Talk zum Thema auf der 
Metting C++ 2015 ("Understanding Compiler Optimization"): 
https://www.youtube.com/watch?v=FnGCDLhaxKU)

[Peter II (Gast)]

Oliver S. schrieb:
> Für die paar einfachen Berechnungen ist der allemal schlau genug.

leider nicht. Genau bei solchen code wie oben, habe ich schon oft erlebt 
das er die Offset von den Objekten jedes mal neu berechnet.

[Peter II (Gast)]

Torsten R. schrieb:
> Dann misst Du einfach mit einem Profiler, optimierst und misst
> wieder mit dem Profiler. Fertig!

halte ich für eine schlechte Idee. Der Profiler sagt mir nicht wo sich 
der Compiler eventuell etwas dumm anstellt. Das sieht man aber recht 
schnell im ASM code.

Leider fehlen wirklich ein paar Infos. Niemand weis ob das alle Static 
ist oder zu Laufzeit erzeugt wurde.

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

nicht“Gast„ schrieb:
> Sind das alles etwa namespaces oder hast du Schlingel einen Haufen
> statische Methoden?
Sowie als auch :)

nicht“Gast„ schrieb:
> Für die Lesbarkeit müsstest du ein wenig mehr Posten.
Besser nicht, sonst wird der Thread nur geflamed ;)

nicht“Gast„ schrieb:
> Beschreiben doch mal den Projekt etwas genauer.
Es werden statische Buffer befüllt. Die werden später von der 
GUI-Messageloop ausgelesen.

namespace GUI {


  class Drawing
  {
  public:
    static bool FillFrameBuffer();
  private:
    static bool HandleDMA2DRequests();
    static bool HandleSoftwareRequests();
  private:
    static bool HandleString(uint16_t ObjPos);
    static void WriteChar(uint16_t ObjPos, char NextChar);

    // Hardware accelerated draw methods
  public:
    static void FillBackground(Resource::Brush color);
    static void FillRect(uint16_t Xstart, uint16_t Ystart, uint16_t width, uint16_t height, Resource::Brush color);
  private:
    struct DMA2DReq
    {
      static uint16_t Amount;
      static uint16_t TransferID;
      static uint16_t NextToTransfer;
      static uint32_t FrameBuffer[GUI_MAX_DMA2D_QUEUE_OBJECTS];
      static uint16_t xStart[GUI_MAX_DMA2D_QUEUE_OBJECTS];
      static uint16_t yStart[GUI_MAX_DMA2D_QUEUE_OBJECTS];
      static uint16_t Width[GUI_MAX_DMA2D_QUEUE_OBJECTS];
      static uint16_t Height[GUI_MAX_DMA2D_QUEUE_OBJECTS];
      static Resource::Brush Color[GUI_MAX_DMA2D_QUEUE_OBJECTS];
    };

    // Software draw methods
  public:
    static void WriteString(char* text, uint16_t Xstart, uint16_t Ystart, uint16_t width,
      Resource::Brush backgroundclr, Resource::Brush fontclr, Memory::Resource::FontInfo* font);
  private:
    struct StringReq
    {
      static uint16_t Amount;
      static uint16_t TransferID;
      static uint16_t NextToTransfer;
      static uint32_t FrameBuffer[GUI_MAX_STRING_QUEUE_OBJECTS];
      static char* TextString[GUI_MAX_STRING_QUEUE_OBJECTS];
      static uint16_t TextWidth[GUI_MAX_STRING_QUEUE_OBJECTS];
      static uint16_t TextLength[GUI_MAX_STRING_QUEUE_OBJECTS];
      static uint16_t xStart[GUI_MAX_STRING_QUEUE_OBJECTS];
      static uint16_t yStart[GUI_MAX_STRING_QUEUE_OBJECTS];
      static Resource::Brush bgColor[GUI_MAX_STRING_QUEUE_OBJECTS];
      static Resource::Brush ftColor[GUI_MAX_STRING_QUEUE_OBJECTS];
      static Memory::Resource::FontInfo* ft[GUI_MAX_STRING_QUEUE_OBJECTS];

      struct Char
      {
        static uint8_t Height[GUI_MAX_STRING_QUEUE_OBJECTS];
        static uint16_t StringPosition[GUI_MAX_STRING_QUEUE_OBJECTS];
        static uint16_t CursorPosition[GUI_MAX_STRING_QUEUE_OBJECTS];

        struct Paint
        {
          static uint8_t R_fg[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t G_fg[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t B_fg[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t R_bg[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t G_bg[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t B_bg[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t R[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t G[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t B[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t AlphaFirst[GUI_MAX_STRING_QUEUE_OBJECTS];
          static uint8_t AlphaLast[GUI_MAX_STRING_QUEUE_OBJECTS];
        };
      };
    };
  };

Ja, ich weiß, das sieht furchtbar aus :) Auf die Schnelle ist mir 
gestern nichts Besseres eingefallen :>
Bisher mache ich es so, dass ich alle Objekte die während des gesamten 
Programms nur eine Instanz (ist ja dann eigentlich keine) benötigen, auf 
static setze.

[Torsten R. (Firma: Torrox.de) (torstenrobitzki)]

Peter II schrieb:
> halte ich für eine schlechte Idee. Der Profiler sagt mir nicht wo sich
> der Compiler eventuell etwas dumm anstellt. Das sieht man aber recht
> schnell im ASM code.

Wenn Du wirklich C++ code hast, und den optimierst, dann wirst Du kaum 
noch den Quellcode mit dem Assembler zusammen bekommen.

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Torsten R. schrieb:
> Die Mikro-Optimierungen, über die Du Dir hier Gedanken machst, bekommen
> die Compiler alle von alleine hin.
Muss in eine Funktion nicht erst "reingesprungen" werden? also kommt da 
nicht eine zusätzliche Instruktion für die CPU hinzu? Genauso wie für 
die Variableninitialisierung?

[Torsten R. (Firma: Torrox.de) (torstenrobitzki)]

Reginald L. schrieb:
> Bisher mache ich es so, dass ich alle Objekte die während des gesamten
> Programms nur eine Instanz (ist ja dann eigentlich keine) benötigen, auf
> static setze.

Und warum machst Du das?

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Torsten R. schrieb:
> Wenn Du wirklich C++ code hast, und den optimierst, dann wirst Du kaum
> noch den Quellcode mit dem Assembler zusammen bekommen.
Wenn ich bei mir Disassemble zeigt er mir aber an in welcher Stelle im 
Quellcode ich gerade (ungefähr) bin.

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Torsten R. schrieb:
> Und warum machst Du das?
Gute Frage, nächste bitte.
Daran habe ich mich zu Anfang gewöhnt, weil ich in die ISRs nur statics 
mitnehmen konnte. Irgendwas war da mal. Wobei auf meiner Liste steht, 
dass ich die ISRs zukünftig in die Klassen mit reinpacken wollte, habe 
mich dazu aber noch nicht genug eingelesen.

[Peter II (Gast)]

Reginald L. schrieb:
> static uint8_t R_bg[GUI_MAX_STRING_QUEUE_OBJECTS];

warum nicht noch eine RGB struct?

also eine Array auf R G B . Dann würde es logischer sein und sich noch 
etwas mehr optimieren lassen.

Das R G B in 3 verschiedenen  Arrays halte ich für nicht schlau.

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Peter II schrieb:
> warum nicht noch eine RGB struct?
Danke für den Tipp, logisch!

Ich pack jetzt mal einen Sack pointer und Funktionen aus und schau mir 
das im disassembler mal an was sich da tut.

[Torsten R. (Firma: Torrox.de) (torstenrobitzki)]

Reginald L. schrieb:
> Muss in eine Funktion nicht erst "reingesprungen" werden? also kommt da
> nicht eine zusätzliche Instruktion für die CPU hinzu? Genauso wie für
> die Variableninitialisierung?

Spannend: Du machst nicht den Eindruck, zu wissen, was ein Compiler kann 
oder nicht kann, bist aber der Meinung, dem Compiler in jeder Zeile Code 
unter die Arme greifen zu müssen ;-)

Sehr ernst gemeint: Schreib korrekten_ und _lesbaren Code. Wenn der 
Code dann macht, was er soll, dann suchst Du die 5% Code bei denen es 
wirklich wichtig ist, dass der schnell ist und optimierst ihn. Und das 
machst Du auch nur dann, wenn Du feststellst, dass der Code wirklich zu 
langsam ist.

Ich arbeite schon seit 20 Jahren als Software-Entwickler und habe vor'm 
Profiling natürlich auch immer eine Vermutung, welcher Teil einer 
Software am besten zu optimieren wäre, ich liege aber eigentlich immer 
falsch.

Berufsanfänger überschätzen (fast) immer die Notwendigkeit von 
Performance. Korrektheit ist immer wichtiger und Lesbarkeit ist fast 
immer wichtiger.

Je schneller Du zu einer korrekten und lesbaren Software kommst, um so 
mehr Zeit bleibt Dir für's Optimieren.

mfg Torsten

P.S.: Zeitlos richtig: "premature optimization is the root of all evil" 
Knuth, 
http://web.archive.org/web/20130731202547/http://pplab.snu.ac.kr/courses/adv_pl05/papers/p261-knuth.pdf

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Torsten R. schrieb:
> Spannend: Du machst nicht den Eindruck, zu wissen, was ein Compiler kann
> oder nicht kann, bist aber der Meinung, dem Compiler in jeder Zeile Code
> unter die Arme greifen zu müssen ;-)
Hab ich auch niemals behauptet, programmiere auch noch nicht so lang.

Torsten R. schrieb:
> Sehr ernst gemeint: Schreib korrekten_ und _lesbaren Code. Wenn der
> Code dann macht, was er soll, dann suchst Du die 5% Code bei denen es
> wirklich wichtig ist, dass der schnell ist und optimierst ihn. Und das
> machst Du auch nur dann, wenn Du feststellst, dass der Code wirklich zu
> langsam ist.
Das ist mal ne Ansage! Mach ich ab jetzt mal so. Ich merke schon, dass 
ich ziemlich viel Zeit ins Nachgrübeln investiere: "Wie mache ich das 
jetzt am besten, damit das fix abgearbeitet wird?"

Torsten R. schrieb:
> Berufsanfänger
Ich bin ein Hobbyanfänger :)

[Peter D. (peda)]

Ich hab das auch schon öfter erlebt, daß sich der Compiler bei 
mehrfachen Indirektionen immer wieder neu nen Wolf rechnet.
Dann kann man ihm mit einem Pointer auf die Sprünge helfen.
So ein Pointer macht auch den Code viel lesbarer, weil sich auch der 
Leser dann nicht bei jeder Variable mühsam durch alle Indirektionen 
durchhangeln muß.

Wenn man viele ähnliche Sachen durch Zusammenfassen verkürzen kann, wird 
oft auch der Code kleiner und schneller.

[Hannes J. (Firma: _⌨_) (pnuebergang)]

Peter D. schrieb:
> Ich hab das auch schon öfter erlebt, daß sich der Compiler bei
> mehrfachen Indirektionen immer wieder neu nen Wolf rechnet.
> Dann kann man ihm mit einem Pointer auf die Sprünge helfen.

Zuvor fängt man doch mit den Grundlagen an. Statt ein struct von Feldern 
zum Beispiel ein Feld mit einem struct nehmen. Duplizierte 
Datenstrukturen (RGB oder die struct-Teile mit FrameBuffer, xStart, 
yStart, Width, Height, Brush, Color) in eigene Datenstrukturen packen 
(Neudeutsch refactoring). Die hinrnrissige Verschachtelung auflösen 
(notfalls mit inline Accessoren und Manipulator-Methoden, 
Operator-Overloading (Hilfe!), und/oder using).

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Hab jetzt das ganze mal etwas anders aufgebaut, zusätzliche Klassen 
erzeugt für RGB, die auch mit Übergabeparametern die Berechnung 
vornimmt, das von vorhin sieht jetzt so aus:

          System::SetPixel(StringReq::FrameBuffer[ObjPos],
            StringReq::Char::CursorPosition[ObjPos] + (x_byte * 2), y_pxl,
            newcolor.CreateBlending(StringReq::ftColor[ObjPos], StringReq::bgColor[ObjPos], ftalpha));

Zumindest augenscheinlich sind weniger Zeilen im Dissasembler zu sehen, 
bis der Rücksprung erfolgt. Heißt das jetzt, dass er besser optimiert 
hat als ich, oder? :)

[guest (Gast)]

Nur mal so als Idee

struct RGB
{
  uint8_t r;
  uint8_t g;
  uint8_t b;
}

struct DATA
{
  uint32_t FrameBuffer;
  char* TextString;
  uint16_t TextWidth;
  uint16_t TextLength;
  uint16_t xStart;
  uint16_t yStart;
  Resource::Brush bgColor;
  Resource::Brush ftColor;
  Memory::Resource::FontInfo* ft;

  uint8_t Height;
  uint16_t StringPosition;
  uint16_t CursorPosition;

  RGB fg;
  RGB bg;
  RGB color;
  uint8_t AlphaFirst;
  uint8_t AlphaLast;
};

static struct
{
  uint16_t Amount;
  uint16_t TransferID;
  uint16_t NextToTransfer;

  DATA data[GUI_MAX_STRING_QUEUE_OBJECTS];
} StringReq;

und dann sowas in der Art:

DATA& actData = StringReq.data[ObjPos];
System::SetPixel( actData.FrameBuffer
                  , actData.CursorPosition + ( x_byte * 2 ), y_pxl
                  , newcolor.CreateBlending(actData.ftColor, actData.bgColor, ftalpha )
                );

[Reginald L. (Firma: HEGRO GmbH) (reggie)]

Danke guest, das sieht vernünftiger aus, setze ich gleich so um.