Skip to content

Leidenschaftlicher Programmierer

Aufgrund des lieben Kommentars von Jens habe ich mich dazu entschieden, auf den zugegeben nicht ganz so lieben Kommentar von Manni zu "antworten" - bzw. ganz allgemein zu dem Thema.

Ich bin seit meinem 13. Lebensjahr Programmierer. Und ich behaupte, dass ich von Anfang an mit Leidenschaft dabei bin (und mit Leidensbereitschaft, mit 14 war C++ dran).

Programmieren ist für mich keine Last, sondern eine wahnsinnig große Freude, die mich zugleich erfüllt und entspannt. Nachts oder früh morgens in (m)einem ruhigen Büro mit guter Instrumentalmusik zu sitzen und zu programmieren, ist für mich besser als jede Netflix-Session. Ich bin dann quasi glückselig.

In 99% der Fälle programmiere ich dabei "irgendwas" für manitu. Und ja, auch "CEO eines global agierenden Unternehmens" (Zitat Manni) tue ich das. Vielleicht auch gerade deswegen. Ich möchte "auch als CEO" nicht den Bezug zur Basis verlieren. Ich arbeite genauso im Support mit, und programmiere genauso mit. Bei fast jedem Projekt.

Ich gebe zu, dass ich gerade beim Programmieren sehr penibel bin. Da ich täglich sehe, was Code von tausenden anderen (z.B. die unzähligen Pakete in jeder Linux-Distribution) anrichten kann, lege ich Wert auf kleinste Details. Manchmal entscheiden diese kleinen Dinge über "Leben oder Tod".

Und genauso lege ich Wert auf Ressourcen sparenden Code. Ja, man muss Code nicht bis zum letzten Detail durchoptimieren, er soll v.a. für einen selbst und andere (!) lesbar sein. Alles, was der Lexer eh wegoptimiert, muss man nicht selbst optimieren.

Aber um einmal in dem konkreten Beispiel zu bleiben: Wenn man aus dem Paket php-iban z.B. nur die Funktion iban_to_machine_format() verwenden möchte, muss man eben die php-iban.php einbinden - und das liest automatisch die genannte registry.txt (quasi als Art von "main"-Funktion der Datei). Obwohl sie für diesen Funktions-Aufruf nicht verwendet wird.

Das wäre vielleicht nicht ganz so schlimm, wenn es eben nicht üblich wäre, solche Bibliotheken via Composer zu verwenden und auch aktuell zu halten. Somit wird die Datei in jedem Projekt, das die Bibliothek verwendet, mitgeladen. Beim Aufruf jeder PHP-Datei, die den Composer-Autoloader nutzt. Also vermutlich jeder.

Mein Patch bestand aus ganzen 3 Zeilen. Ich habe das Laden registry.txt einfach ausgelagert an die 2 einzigen Stellen, an denen sie nötig ist. Der Autor des Pakets hatte in seiner _iban_load_registry() bereits Code drin, der ein doppeltes Laden der Datei vermeidet.

Seht selbst:

Ich hatte gesehen, dass der Autor Patches nicht sehr aufgeschlossen ist, daher hatte ich meinen Patch so klein wie möglich gehalten. Quasi ohne Impact - auch optischer Natur ;-)

Normalerweise hätte ich, wie es auch in einigen Kommentaren hier bereits gesagt wurde, vorgeschlagen, die Datei in ein PHP-Code umzuwandeln und ggf. sogar noch bedingt via require_once() einzubinden, dann hätte der PHP-OPCache sie mitopitmiert.

Schade, dass der Autor dem Patch nicht sehr aufgeschlossen zu sein scheint. Vielleicht möchtet Ihr ihn ja nochmal mit Kommentaren im Pull-Request dazu animieren ;-) ?

Kommentare

Ansicht der Kommentare: Linear | Verschachtelt

Jo

Nunja, composer lädt es nicht immer, wenn der autoloader geden wird, sondern wenn der Autoloader für eine Klasse aus der Datei geladen wird. Kann mich imme zu viel sein. Ja.

Aber Problem mit der vorgeschlagenen Lösung ist, da die zwei Funktionen jetzt Seiteneffekte bekommen. Das ist problematisch, da man z.B. eine neue Funktion einführen könnte, die das nutzt und den Call vergessen könnte oder in einem späteren refactoring nimmt man den "unnötig" Call raus und je nachdem in welcher Reihenfolge die Funktionen aufgerufen werden funktioniert es ... oder nicht.

Das "gut" zu machen wäre aber wohl in der Tat deutlich mehr Aufwand.

Manuel Schmitt

Ich kann weder beim 2. noch beim 3. Blick einen Seiteneffekt sehen.

Es gibt genau 2 Funktionen, die sich darauf verlassen, dass die globale Variable gefüllt ist:
- _iban_country_get_info()
- iban_countries()

In beide Funktionen habe ich in meinen Patch genau das Laden aufgenommen, und den globalen "immer Laden"-Teil rausgenommen.

Das hat keine Seiteneffekte, und niemand muss beim Aufruf an irgendwas denken.

Und zu Composer: Da die IBAN-Funktionen in der composer.json der Lib

https://github.com/globalcitizen/php-iban/blob/master/composer.json

via

"autoload": {
"files": ["oophp-iban.php","php-iban.php"]
}

immer geladen werden, wird bei jeder PHP-Datei, die die Composer-Autoload-Funktion verwendet, die .txt-Datei der php-iban gelesen.

Schau nochmal genau hin ;-)

Fabian

Jo das würde nur dann ein Problem sein, wenn jemand eine eigene Funktion zum abrufen der Daten aus dem Array erstellt. Aber anstatt das Rad ständig neu zu erfinden, kann man auch einfach auf bestehende Funktionen zurück greifen. Wie Manuel schon schrieb, greifen nur die beiden Funktionen auf die Variable zurück. Und mit diesen beiden Funktionen erhält man eigentlich alle Daten, die man benötigt. Man kann also dann als Entwickler auf diese Funktionen zurück greifen, anstatt zusätzlich eine eigene Routine zu schreiben (die nämlich dann wieder die Performance zusätzlich verschlechtert).

Wer dennoch eine solche Sonderanpassung benötigt, wird wohl sich auch Gedanken darum gemacht haben, wie denn die Daten in das Array kommen...

onli

Mich verwundert es ja, dass solches Engagement negativ gesehen werden kann. Auf mich macht es einen sehr positiven Eindruck, wenn von dir so direkt zu freier Software beigetragen wird. Passt doch auch einfach gut zu allem, womit manitu sonst überzeugen will.

MK

PHP? In 2019? Ernsthaft? ;) Aber die Änderung macht durchaus Sinn und die Reaktion von globalcitizen ist etwas erbärmlich...

Felix

Klar, die Antwort hätte auf den PR hätte man weniger harsch formulieren können. Aber warum jetzt einen länglichen Blogpost schreiben anstatt ein/zwei Timings zu machen und den Code zu vergleichen?

So absurd ist die Frage ja nun nicht - denn während das Laden der globalen Variable bislang implizit war, muss man nach der Änderung daran denken, `_iban_load_registry` aufzurufen, wenn man beispielsweise eine neue Funktion einführt, die darauf zugreifen soll. Das ist nicht die Welt, aber genau genommen natürlich schon ein "maintenance burden". Schöner wäre vielleicht, die Registry nicht global zu machen, sondern beispielsweise als statische Variable in einer Funktion (`get_registry` o.ä.) zu verstecken.

Unabhängig davon scheint es mir aber zumindest schlechter Stil zu sein, andere Leute dazu zu animieren, auf Github Stimmung zu machen. Wäre ich der Maintainer, würde mich das wahrscheinlich eher davon abhalten, meine Entscheidung zu überdenken.

Jens

Timings zu vergleichen wird hier nicht von großem Wert sein, weil in beiden Versionen (Manuels sowie in der Originalen) die besagte Textdatei zur Benutzung der Methoden geladen werden muss.
Eher würde es Sinn machen, den Speicherverbrauch zu vergleichen - hier liegt die Ersparnis aber quasi schon auf der Hand: Die größe der Textdatei + ein paar Headerbytes des Arrays. Gerade weil hier der Vorteil so offensichtlich auf der Hand liegt, ist es sehr fragwürdig warum darüber soviel diskutiert wird. In einem C++ Programm würde man auch nicht ohne Not einfach mal ein "new char[32768]" irgendwo ungenutzt rumgammeln lassen - aber so "unabstrakt" haben vermutlich nicht mehr viele programmiert, um sowas "automatisch" im Hinterkopf zu haben.

Stefan

Der Gewinn entsteht ja dann, wenn die Datei nicht geladen wird. Das kann man hoffentlich in einem passenden Usecase testen und messen.

Ich würde auch Messwerte fordern, bevor ich Code akzeptiere, der eine Performanceverbesserung verspricht. Jede Änderung ist Risiko, und manche Änderungen fordern zusätzlichen Wartungsaufwand für die Zukunft.

Und gerade Performance ist etwas, das auch gerne von Anfängern kommt: die haben dann irgendwo gelesen, dass - mal als Beispiel - Countdown-Schleifen effizienter seien, weil man da eine "decrement-and-jump-if-not-zero" Instruktion benutzen kann, und bauen dir alle for-Schleifen von ++i auf --i um. Im großen anonymen Internet sieht man halt nicht, ob der first-time-contributor ein solcher Anfänger ist. Es betrifft auch nicht nur Anfänger, ich hab hier durchaus schon nervenzehrend mit Leuten geredet, die länger im Geschäft waren, die mir einen 1000 Zeilen Patch reindrücken wollten und einfach nicht glauben wollten, dass eine gesparte Nanosekunde pro Iteration auf 100 Iterationen keine 10 Sekunden sparen kann.

Manuel Schmitt

Ich verstehe Deine Argument.

In dem Fall war die Komplexität aber eh schon gegeben, da der Ursprungsautor ein "global $VARIABLE" überall dort, wo das gebraucht wird, gemacht hat (genau an 2 Stellen). An genau diesen 2 Stellen habe ich das Laden hinzugefügt.

Selbst bei Copy&Paste wird kein Fehler entstehen.

Und wer nur das global'en einer Variable im Kopf hat, der hat eh ein Problem...

Felix

Dass die Variable via global-Anweisung ohnehin explizit "angefordert" werden muss ist natürlich ein Argument was die Komplexität angeht.

Würde trotzdem dazu raten - gerade wenn der Maintainer einen konfrontativen Stil an den Tag legt - mit mehr Fingerspitzengefühl vorzugehen. Das erhöht die Chancen, dass der PR akzeptiert wird deutlich.

Kommentar schreiben

Umschließende Sterne heben ein Wort hervor (*wort*), per _wort_ kann ein Wort unterstrichen werden.
Standard-Text Smilies wie :-) und ;-) werden zu Bildern konvertiert.
Die angegebene E-Mail-Adresse wird nicht dargestellt, sondern nur für eventuelle Benachrichtigungen verwendet.
BBCode-Formatierung erlaubt
Formular-Optionen