blog · git · desktop · images · contact
for
, goto
, Lesbarkeit2019-12-29
nullprogram.com ist eines der besten Blogs, die ich kenne. Quasi jeder Artikel ist interessant und ich lerne etwas neues.
Heute hat er einen Artikel zu purgeable memory veröffentlicht. Kurze Zusammenfassung des Konzepts: Es geht um Speicher, der vom Kernel freigegeben (nicht geswappt!) werden kann, falls es die globale Arbeitslast im System erfordert. Der eigene Code muss damit natürlich umgehen können.
Die Idee dahinter ist: Eine der Berechnungen, die der Code durchführt, dauert eine Weile und nimmt spürbar Speicher in Anspruch. Das Ergebnis will man mehrfach verwenden, zwischen den Verwendungen liegen aber größere zeitliche Pausen. Außerdem ist es schneller, die Berechnung einfach nochmal durchzuführen, als auf den Kernel zu warten, bis der irgendwas aus dem Swap wieder rausgeholt hat.
Als Beispiel gibt es diesen Codeschnipsel:
/* Version 0: Original code */
uint32_t *texture = 0;
struct png *png = png_load("texture.png");
if (!png) die();
/* ... */
for (;;) {
if (!texture) {
texture = purgeable_alloc(png->width * png->height * 4);
if (!texture) die();
png_decode_rgba(png, texture);
} else if (!purgeable_lock(texture)) {
purgeable_free(texture);
texture = 0;
continue;
}
glTexImage2D(
GL_TEXTURE_2D, 0,
GL_RGBA, png->width, png->height, 0,
GL_RGBA, GL_UNSIGNED_BYTE, texture
);
purgeable_unlock(texture);
break;
}
Eine Textur wird geladen und in einem OpenGL-Kontext verwendet.
texture
ist also eine langlebige Variable. Man denke beispielsweise an
ein Menü in einem Spiel: Das Menü wird nur ab und zu mal aufgerufen und
zeigt immer dasselbe Hintergrundbild – unsere texture
. Das Menü soll
sich schnell öffnen, also würde man die nötigen Daten gerne im Speicher
behalten. Auf der anderen Seite: Wenn das Menü nur selten aufgerufen
wird, muss das dann wirklich im Speicher bleiben? Wäre nett, zwingend
notwendig ist es aber eben nicht. Also könnte man purgeable memory dafür
verwenden.
Mir geht es jetzt um die for
-Schleife da oben.
Die hat mich am Anfang ziemlich verwirrt. Warum ist da eine Schleife?
Warum ist sie endlos? Müssen wir da vielleicht etwas mehrfach probieren,
weil es fehlschlagen könnte? Einen Moment später sind mir dann
continue
und break
aufgefallen und dann hat’s geklingelt.
Was eigentlich passiert, ist das (Pseudocode):
if (texture not loaded)
load texture;
else /* texture was once loaded */
if (kernel has purged the memory in the meantime)
mark texture as "not loaded";
TRY AGAIN FROM THE TOP;
use texture in OpenGL;
mark texture memory as purgeable;
Mit anderen Worten, die for
-Schleife wird benutzt, um ein goto
zu
vermeiden.
Ein Vorteil dabei ist, dass die geschweiften Klammern unmissverständlich
klarmachen, wo dieser Abschnitt beginnt und endet. Es kann keine
unerwarteten Sprünge von woanders im Code geben, also wird eine der
größten Fallen von goto
ausgeschlossen.
Auf der anderen Seite habe ich den Eindruck, dass dieses Konstrukt nicht unbedingt „besseren“ Code erzeugt. Man muss es zweimal lesen und es ist auch gar keine Schleife. Es ist eher ein „probier’s nochmal“-Mechanismus.
Wenn man’s mal verstanden hat, ist es trivial. Es ist aber eines dieser Dinge, die so eine kleine „Barriere“ erzeugen. Wenn solche Tricks an sehr vielen Stellen benutzt werden, wird der Code unzugänglicher und es kann auch recht schwer für neue Leute im Team werden. Ich bin da natürlich keine Ausnahme und mache sowas auch. Klar, wenn man Code selbst schreibt, versteht man immer alles und alles ist trivial. Man vergisst zu leicht, dass man selbst auch irgendwann wieder „neu im Team“ ist, einfach weil man nach ein paar Jahren, in denen man den Code nicht angefasst hat, diese Tricks auch alle wieder vergessen hat. Das ist jetzt keine neue Erkenntnis, aber sie gerät eben allzu leicht in Vergessenheit.
Mit einem goto
sähe es so aus:
/* Version 1: goto */
load_texture:
if (!texture) {
texture = purgeable_alloc(png->width * png->height * 4);
if (!texture) die();
png_decode_rgba(png, texture);
} else if (!purgeable_lock(texture)) {
purgeable_free(texture);
texture = 0;
goto load_texture;
}
glTexImage2D(
GL_TEXTURE_2D, 0,
GL_RGBA, png->width, png->height, 0,
GL_RGBA, GL_UNSIGNED_BYTE, texture
);
purgeable_unlock(texture);
Ist das besser? Ich denke schon. Es gibt keine Verwirrung über eine
Schleife mehr. Die unzutreffenden Vokabeln continue
und break
sind
weg. Man liest das Label zuerst, ist also gewahr, dass da ein goto
kommen wird. Label und goto
sind auch recht nah beieinander. In meinen
Augen ist das durchaus besser verständlich.
Ich benutze goto
ab und zu, bin aber immer vorsichtig. Es muss einen
guten Grund dafür geben. Es muss dabei helfen, Code zu schreiben, der
besser verständlich ist. In diesem Fall hier wäre das in meinen Augen
gegeben.
Die offensichtliche Frage ist aber, ob wir den Sprung überhaupt brauchen. Wie wäre es damit:
/* Version 2: Restructured code */
if (texture && !purgeable_lock(texture)) {
purgeable_free(texture);
texture = 0;
}
if (!texture) {
texture = purgeable_alloc(png->width * png->height * 4);
if (!texture) die();
png_decode_rgba(png, texture);
}
glTexImage2D(
GL_TEXTURE_2D, 0,
GL_RGBA, png->width, png->height, 0,
GL_RGBA, GL_UNSIGNED_BYTE, texture
);
purgeable_unlock(texture);
Der Code wurde umstrukturiert und es gibt jetzt kein „probier’s nochmal“ mehr. Der Pseudocode dazu sieht so aus:
if (kernel has purged our memory)
reset pointer;
if (texture not loaded)
load texture;
use texture in OpenGL;
mark texture memory as purgeable;
Ein bisschen hässlich ist, dass der Check für texture == NULL
dupliziert ist. Es ist außerdem wichtig, dass man es nicht als else if
schreibt, weswegen ich die leere Zeile drinhabe.
Geht es noch prägnanter? Was wir ja eigentlich machen wollen, sieht so aus:
if (texture not available)
load texture;
use texture in OpenGL;
mark texture memory as purgeable;
Es gibt keine Schleife, keinen Sprung, nichts. Die Absicht ist klar formuliert. Kriegen wir das hin? Im Moment nicht. Nehmen wir aber mal an, die API der Bibliothek würde sich leicht ändern:
NULL
-Pointer unterstützen: purgeable_lock()
würde einfach
False
oder 0
zurückgeben.Dann könnte man das machen:
/* Version 3: Altered API */
if (!purgeable_lock(texture)) {
texture = purgeable_alloc(png->width * png->height * 4);
if (!texture) die();
png_decode_rgba(png, texture);
}
glTexImage2D(
GL_TEXTURE_2D, 0,
GL_RGBA, png->width, png->height, 0,
GL_RGBA, GL_UNSIGNED_BYTE, texture
);
purgeable_unlock(texture);
Das ist die knappste und prägnanteste Variante, die ich finden konnte.
Auf der anderen Seite ist das wieder eine Grauzone. In Variante 3 muss man nämlich mehr mitdenken:
texture == NULL
ist, tut die Bibliothek nichts und returned
False
. Wir müssen jetzt das PNG decoden.texture != NULL
ist:Je älter ich werde, desto mehr stört mich sowas. Es ist nur ein
Funktionsaufruf, ich muss aber mehrere Situationen und Codepfade im Kopf
durchsimulieren. Wenn man nicht aufpasst, vergisst man den Fall mit
NULL
vielleicht sogar. Selbst, wenn wir die Bibliothek also wie oben
geändert hätten, würde ich den Code eher so schreiben:
/* Version 4: Altered API and explicit check */
if (!texture || !purgeable_lock(texture)) {
texture = purgeable_alloc(png->width * png->height * 4);
if (!texture) die();
png_decode_rgba(png, texture);
}
glTexImage2D(
GL_TEXTURE_2D, 0,
GL_RGBA, png->width, png->height, 0,
GL_RGBA, GL_UNSIGNED_BYTE, texture
);
purgeable_unlock(texture);
Die Prüfung auf texture == NULL
ist dabei nicht zwingend notwendig,
weil das ja auch die Bibliothek für uns macht. Ich glaube trotzdem, dass
es sinnvoll ist, das explizit auszuschreiben, weil der Leser jetzt
sofort weiß, dass es bei diesem if
um zwei verschiedene Fälle geht:
Das PNG wird decoded, wenn es das allererste Mal ist oder falls der
Speicher weggeworfen wurde.
Als Schlussbemerkung: Ich schreibe lieber texture == NULL
statt
!texture
, aus genau denselben Gründen. Es ist ein Pointer und wir
prüfen, ob er NULL
ist. Länger, aber dafür glasklar.