contao / core

Contao 3 → see contao/contao for Contao 4
GNU Lesser General Public License v3.0
490 stars 214 forks source link

yace backport: __autoload() #3089

Closed leo-unglaub closed 12 years ago

leo-unglaub commented 12 years ago

Hallo Leo, wie per E-Mail angekündigt möchte ich dir ein paar Tweaks auf den Yace Projekt anbieten um diese auch im Contao Core einzubauen. Als erstes habe ich hier einen patch für die function __autoload(). Aktuell hat diese Funktion das Problem, dass wenn die Klasse nicht unter system/libraries liegt, alle Modulordner pro Methoden-Aufruf durchlaufen werden müssen. Ich habe mal die music_academy zum testen herangezogen und pro FE Aufruf muss der autoloader bis zu 40 mal nach einer Klasse suchen. Das ist enorm viel IO.

Daher habe ich das ganze gecached. Dabei mache ich nichts weiter als mir Sessionübergreifend zu merken wo ein File lag und dann einfach zu includen. Mein Autoloader lernt automatisch alle Pfade und kann diese auch wieder "vergessen" wenn es die Klasse nicht mehr gibt. Durch diesen dynamischen Aufbau kann ich bereits nach 5 bis 10 Klicks im Backend 99% aller Anfragen cachen. Das spart unter Last extrem viel Ressourcen.

Anbei ein patch und einmal die vollständige Datei. Ich habe im Patch das cache file unter system/config/autoloader.php gespeichert. In Yace habe ich eine andere Ordner-Struktur (ich habe dort kein Frontend, sondern nur das BE) daher musst du das vielleicht noch anpasen.

Wenn Rückfragen sind, entweder hier oder auch sehr gerne im IRC. Viele Grüße Leo

Download the attachments

--- Originally created on May 18th, 2011, at 03:40am (ID 3089)

leo-unglaub commented 12 years ago

Nachtrag: Die Schaltfläche "weitere Datei hochladen" ging im FF gerade nicht, daher hier die vollständige Datei. Viele Grüße Leo

--- Originally created on May 18th, 2011, at 03:42am

ghost commented 12 years ago

Hallo Leos,

habe die neue functions.php von leo.unglaub gerade getestet und kann bestätigen, dass diese Änderung speziell auf lahmen Servern mit vielen Extensions, Contao ordendlich beschleunigt. Ich würde jedoch der include in Zeile 40 noch ein @ spendieren, dann werden keine Fehlermeldungen ausgeworfen falls die autoload.php, aus welchen Gründen auch immer, nicht exisitiert.

--- Originally created by N.Parker on May 18th, 2011, at 08:14am

leo-unglaub commented 12 years ago

Hallo ! Das @ ist wahrscheinlich gar keine so dumme Idee. In Yace ist diese Datei immer da, das überwacht ein cron daemon, aber in Contao wäre das durchaus praktisch. Viele Grüße Leo

--- Originally created on May 18th, 2011, at 12:43pm

Toflar commented 12 years ago

Zwei Dinge:

  1. Vermutest Du, dass das Deserialisieren schneller ist? Da habe ich so meine Zweifel irgendwie. Wieso machst Du nicht direkt ein var_export()? Nur so ein Vorschlag, müsste man mal vergleichen.
  2. und meiner Meinung nach noch viel wichtiger: Laufen wir da nicht in Race Conditions? Wenn zwei Instanzen gleichzeitig deine autoload.php schreiben wollen, was passiert?

--- Originally created on May 18th, 2011, at 01:07pm

leo-unglaub commented 12 years ago

1: ja, serialisieren ist schneller und sicherer, da Leo Feyer da auch einen Cache eingebaut hat und zum anderen bekomme ich so wirklich nur das Array das ich will. Würde bei deiner Methode irgend wer was in das File injecten würde das auch ausgeführt werden.

2: ehrlich gesagt keine Ahnung. Eigentlich ist es das gleiche wie wenn 2 Besucher gleichzeitig die Seite aufrufen und die localconfig.php neu geschrieben wird. Eigentlich sollte das OS das File mit einem lock versehen und 2 gleichzeitige Schreibzugriffe sperren. Das ist jedoch nur eine Vermutung, um mir die fopen und fwrite in der PHP implementierung anzuschauen fehlt mir aktuell die Zeit.

--- Originally created on May 18th, 2011, at 02:40pm

ghost commented 12 years ago

Würde bei deiner Methode irgend wer was in das File injecten würde das auch ausgeführt werden. Wenn in die autoload.php irgend etwas injeziert wird, ist es egal, ob die Daten serialisiert sind oder nicht. Füg doch spaßeshalber ein die(); vor dem ?> ein. Das geht dann immer. Somit würde ein var_export das serialize unnötig machen und somit ggf. tatsächlich noch minimal schneller sein.

Was anderes ist es beim Speichern. Deine Funktion nutzt das direkte Speichern (fopen, fwrite, ...) damit sollte es eigentlich kein Problem geben und das OS evtl. "querschüsse" verhindern. Anders sieht es bei benötigtem SMH aus. Wenn du die File-Klassen von Leo nutzt, dann werden weitere Klassen nachgeladen und wir kommen in eine race condition, soweit ich das jetzt sehe. Nur mal so als Überlegung.

Grundsätzlich halte ich dein cachen aber für effektiv.

--- Originally created by N.Parker on May 18th, 2011, at 03:22pm

leo-unglaub commented 12 years ago

1: EPIC FAIL meinerseits, bitte einfach ignorieren g 2: Die File Klassen von Leo verwende ich da mit absicht nicht. Wenn jemand den SMH braucht, ist der für mich gestorben. Ein Server der einen SMH braucht ist in meinen Augen falsch konfiguriert und gehört vom Netz genommen. In Yace habe ich diesen Fall daher nicht. Aber man könnte dem File 777 geben, dann würde es gehen und es wäre nicht wirklich gefährlich.

--- Originally created on May 18th, 2011, at 03:40pm

ghost commented 12 years ago

Hab eine leicht veränderte Version erstellt, die auch für SMH funktioniert (wird dann halt nicht gecached) und ohne serialize arbeitet. Initial sollte dann die autoload.php schon alle Klassen des core enthalten, so dass das dann auch bei SMH schon leichte vorteile bringt. Weitere Klassen werden bei aktivem SMH nicht mehr ergänzt.

--- Originally created by N.Parker on May 23rd, 2011, at 09:25am

ghost commented 12 years ago

Und hier noch der diff. (Die Schaltfläche "weitere Datei hochladen" ging in meinem FF auch nicht)

--- Originally created by N.Parker on May 23rd, 2011, at 09:26am

leofeyer commented 12 years ago

Ich habe zunächst mal einen Session-Cache implementiert (siehe 27be80ee8d5be5737e1fd5b8286af738). Wirklich nennenswerte Unterschiede konnte ich jedoch nicht feststellen, denn prinzipiell übernimmt jeder PHP-Cache wie z.B. der Zend Optimizer oder der APC genau dieselbe Aufgabe. Aber mal die Rückmeldungen abwarten.

--- Originally created on May 24th, 2011, at 12:21pm

leo-unglaub commented 12 years ago

So bringt das nichts. Daher wirst du so auch keine Performence Steigerung feststellen können. Wenn eine Klasse schon mal geladen wurde und PHP diese kennt, wird __autoload ja nie mehr aufgerufen. Daher bringt der aktuelle Cache nur, dass der Speicherverbrauch höher wird.

Das geile an meinem Cache ist ja, dass PHP nicht bei jedem Aufruf die Klassen wieder neu suchen muss.

--- Originally created on May 24th, 2011, at 12:25pm

leofeyer commented 12 years ago

dass PHP nicht bei jedem Aufruf die Klassen wieder neu suchen muss.

Das trifft doch auf den Session-Cache genauso zu. Ab dem zweiten Aufruf muss __autoload() nicht mehr alle Verzeichnisse durchsuchen, weil der Pfad bereits in der Session gespeichert ist. Der einzige Unterschied zu Deiner Implementation ist, dass beim allerersten Aufruf noch gesucht werden muss und nicht direkt auf einen Session-übergreifenden Cache zurückgegriffen werden kann.

--- Originally created on May 24th, 2011, at 12:27pm

leo-unglaub commented 12 years ago

Ich cache ja nicht in der Session. Bei deiner Lösung oder auch bei der aktuellen Version ist es so, dass Contao bei jedem Aufruf einer Seite immer alle Pfade neu suchen muss. Da diese ja irgend wo im Modulordner liegen ist das viel IO. In meiner Version wird das in ein File gecached. Also muss Contao in 99% der Fälle nicht mehr nach dem File in allen Modulordnern suchen, sondern einfach includen. Das ist eine enorme Ersparniss.

--- Originally created on May 24th, 2011, at 12:30pm

leofeyer commented 12 years ago

Bei deiner Lösung oder auch bei der aktuellen Version ist es so, dass Contao bei jedem Aufruf einer Seite immer alle Pfade neu suchen muss.

Nein, das stimmt nicht. Bei meiner Lösung werden ab dem zweiten Aufruf alle bekannten Klassen aus dem Session-Array geladen und die Suche entfällt.

--- Originally created on May 24th, 2011, at 12:31pm

leo-unglaub commented 12 years ago

okay, stimmt. Trotzdem werden ja zu Hauf neue Sessions eröffnet. Somit muss Contao immer noch sehr oft alles durchsuchen. Der Vorteil des Cachings ist damit mehr oder weniger hinüber.

--- Originally created on May 24th, 2011, at 12:41pm

leofeyer commented 12 years ago

Wie gesagt ist der Effekt der Umstellung sowieso kaum messbar, sobald ein PHP-Cache am Start ist. Und das dürfte bei so ziemlich allen Providern der Fall sein. Deswegen warten wir erstmal die Rückmeldungen ab und können dann bei Bedarf immer noch auf einen Session-übergreifenden Cache umstellen.

--- Originally created on May 24th, 2011, at 12:44pm

ghost commented 12 years ago

@leo: Da muss ich leo.unglaub recht geben. Wenn man das ganze in einer Datei schreibt, bekommen Erstbesucher die Optimierung zu spüren. Bei der $_SESSION-Variante dagegen nicht.

Edit: Unter Windows könnte die Änderung besser messbar sein, da dort Festplattenoperationen teurer sind...

--- Originally created by SunBlack on May 24th, 2011, at 12:46pm

leofeyer commented 12 years ago

Wie gesagt ist der Effekt der Umstellung sowieso kaum messbar

--- Originally created on May 24th, 2011, at 12:46pm

leo-unglaub commented 12 years ago

Wie gesagt ist der Effekt der Umstellung sowieso kaum messbar

Das würde ich so pauschal nicht sagen. Selbst auf meinen Servern ist der Unterschied durchaus bemerkbar. Des weiteren darfst du ja nicht davon ausgehen, dass alle so laubere Installationen haben wie wir hier. Wenn du die durchschnitts contao Seite hernimmst heißt das so viel wie: "40 bis 50 Extensions reingepackt und auf einem 2,99 € Hoster gespeichert". Da gibts dann meistens auch keinen PHP Cache oder sonstiges und dann merkt man den unterschied enorm.

BTW: @php cache: ich habe bei mir auch keinen am Laufen. Gab zu oft Probleme und stand kaum in einer Relation zum Aufwand. Viele Grüße Leo

--- Originally created on May 24th, 2011, at 12:49pm

leo-unglaub commented 12 years ago

@sunblack: das kannste auch unter Linux messen. Haue einfach mal mit ab -n 100000 -c 200 auf den vhost drauf. Da krachts dann aber ordnetlich wenn man sich nebenbei anschaut das der kjournald machen muss. :)

--- Originally created on May 24th, 2011, at 12:51pm

ghost commented 12 years ago

Hab die Varianten auf einem 3,50€ Hoster getestet und konnte zwischen den Varianten von leo.unglaub und meiner modifikation keinen reproduzierbaren unterschied feststellen (Meine Version berücksichtigt aber schon den SMH ;-). Beide Versionen waren jedoch schneller als mit der Original-Version (V2.9.5). Leos Version wirkt zwar auch, aber immer nur pro Session. Bei mehreren Besuchern mit wenig klicks hält sich der Vorteil in Grenzen. Interessanter weise war der Effekt im Backend bei mir deutlicher zu Merken als im Frontend. Aber da möchte ich meine Hand nicht ins Feuer legen bei einem 3,50€ Hoster. Persönlich würde ich immer einer statischen Variante nach dem Muster von leo.unglaub der Vorzug geben. Würde dann aber auch in der Systemwartung ein Bereinigen der system/config/autoload.php spendieren.

--- Originally created by N.Parker on May 24th, 2011, at 02:30pm

leo-unglaub commented 12 years ago

Würde dann aber auch in der Systemwartung ein Bereinigen der system/config/autoload.php spendieren.

Das ist eine gute idee. Ich hatte es in Yace so gemacht, dass die per Cron im Weekly einfach geleert wird. Rein Theoretisch ist die Datei ja selbstwartend, aber so ganz traut man dem dann doch nicht g

--- Originally created on May 24th, 2011, at 02:33pm

leofeyer commented 12 years ago

Hab doch noch ein gravierendes Argument gegen den Session-Cache gefunden :/ Im Frontend gab es keine Möglichkeit, die Sessiondaten zu leeren, daher wurden neu angelegte Templates nicht übernommen. Also habe ich eine FileCache-Klasse geschrieben und speichere die Informationen nun doch Session-übergreifend (siehe 6bf0e49871b9a0fe8902056262421715).

--- Originally created on May 25th, 2011, at 02:14pm

leo-unglaub commented 12 years ago

Eine eigene Klasse dafür zu schreiben ist auch eine Idee. Jedoch ist das Speicherformat noch nicht ganz Optimal. Denn theoretisch ist ein "," im Pfad oder Dateinamen erlaubt und auch möglich. Natürlich sollte man solche unsinnigkeiten vermeiten, aber es wäre trotzdem besser den Pfad und auch den Klassennamen noch in " zu packen. Dann sollte das Problem nicht mehr akut sein. Viele Grüße Leo

--- Originally created on May 25th, 2011, at 02:29pm

leofeyer commented 12 years ago

Behoben in 12a6d33cde7c1b1ce35ce8740d836055.

--- Originally created on May 25th, 2011, at 02:31pm

leo-unglaub commented 12 years ago

Nachtrag: fgetcsv hat als Parameter eh enclosure und delimiter. Damit sollte das kein Problem sein

--- Originally created on May 25th, 2011, at 02:31pm

leo-unglaub commented 12 years ago

ganz stimmt das aber nicht.

"fe_page.html5','system/modules/frontend/templates/fe_page.html5"

sollte das nicht so lauten:

"fe_page.html5","system/modules/frontend/templates/fe_page.html5"

--- Originally created on May 25th, 2011, at 02:33pm

Toflar commented 12 years ago

http://dev.contao.org/projects/typolight/repository/revisions/746/entry/branches/contao-2.10/system/libraries/FileCache.php#L69

file_exists() wird da wohl auch immer failen, weil das 'rb' wahrscheinlich weiter unten zum fopen() gehört ;-)

--- Originally created on May 25th, 2011, at 02:36pm

leofeyer commented 12 years ago

Ich bin schon am nächsten Thema dran, daher das Gehudel :/ Jetzt erstmal Pause machen! In der f2615c6b737795148fc1a2a44c442233 sollte nun alles passen.

--- Originally created on May 25th, 2011, at 02:39pm

Toflar commented 12 years ago

Ach, mach dir bloss keinen Kopf, etwas Arbeit können wir dir ja abnehmen, auch wenn es nur Reviewen ist ;-) Pause ist ein gutes Stichwort =) r748 passt! =)

--- Originally created on May 25th, 2011, at 02:42pm

leo-unglaub commented 12 years ago

ja, nun sollte es passen !

--- Originally created on May 25th, 2011, at 02:43pm

xchs commented 12 years ago

Gibt es jetzt eigentlich performancetechnisch objektive Zahlen, welche eindeutig belegen, dass durch die neue FileCache-Klasse tatsächlich ein deutlich messbarer Vorteil im Betrieb von Contao gegeben ist?

Mein subjektives Empfinden beim Arbeiten im Backend der Demo-Installation ist jedenfalls: Es "flutscht" soweit alles recht flott! :)

--- Originally created on May 25th, 2011, at 04:46pm

leofeyer commented 12 years ago

Ich habe wie gesagt kaum Unterschiede spüren können, wobei sich im Code einwandfrei nachvollziehen lässt, dass durch die Anpassungen deutlich weniger Schleifen durch das system/modules-Verzeichnis gelaufen werden.

--- Originally created on May 25th, 2011, at 04:48pm

leo-unglaub commented 12 years ago

ich habe jetzt mal die serialisierte variante gegen die csv version gebenchmarked. Die serialisierte Version ist um einen Tick schneller, allerdings braucht diese auch ein bischen mehr RAM. Das dürfte am Caching der Funktion deserialize liegen. Das ist aber nicht weiter schlimm.

--- Originally created on June 14th, 2011, at 11:32am

leofeyer commented 12 years ago

Die Änderung ist schon seit gestern drin (siehe a7a52f4467c849c7fb3485d73935d91d).

--- Originally created on June 14th, 2011, at 11:36am

leo-unglaub commented 12 years ago

@Leo: jep, genau diese Revision habe ich gegen den Vorgänger gebenched. :)

--- Originally created on June 14th, 2011, at 11:44am

leofeyer commented 12 years ago

Alles klar. Die Array-Version ist tatsächlich effektiver, wobei der Unterschied erst bei 1000 Aufrufen so richtig zur Geltung kommt. Und wir haben ja nur einen :)

--- Originally created on June 14th, 2011, at 11:46am

leo-unglaub commented 12 years ago

Ich habe es auf einer Seite getestet wo ich beide Operationen ausgeführt habe. Diese habe ich dann mit

leo`leo-buero:~$ ab -n 100000 -c 50 http://www.foo.bar/bench-csv.html

und

leo`leo-buero:~$ ab -n 100000 -c 50 http://www.foo.bar/bench-serialized.html

getestet. Der Unterschied ist schon merkbar. :)

--- Originally created on June 14th, 2011, at 11:53am

leofeyer commented 12 years ago

--- Originally completed on May 24th, 2011, at 12:21pm