Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP : Profilage performances #303

Open
Alkarex opened this issue Nov 30, 2013 · 17 comments
Open

PHP : Profilage performances #303

Alkarex opened this issue Nov 30, 2013 · 17 comments
Labels
Feature Request ideas for new features System care Everything related to the system care
Milestone

Comments

@Alkarex
Copy link
Member

Alkarex commented Nov 30, 2013

Après le profilage JavaScript, CSS, HTTP, et SQL qui ont été en bonne partie traités, il reste à faire le profilage du code PHP lui-même, par exemple avec xdebug http://xdebug.org/docs/profiler

Alkarex referenced this issue Dec 15, 2013
…lasses

C'est parti de changements pour
#255 et finalement
j'ai continué la refactorisation...

Ajout de préfixes FreshRSS_ et Minz_ sur le modèle de SimplePie_.
Toutes les classes sont maintenant en chargement automatique (devrait
améliorer les performances en évitant de charger plein de classes
inutilisées, et faciliter la maintenance).
Suppression de set_include_path().
Si souhaité, certaines classes de Minz pourraient être déplacées dans un
sous-répertoire, par exemple les exceptions.

Tests et relecture nécessaires.
@Alkarex
Copy link
Member Author

Alkarex commented Dec 22, 2013

Voir aussi #260 (PHP : Booléens plutôt que chaînes de texte)

Alkarex added a commit that referenced this issue Dec 28, 2013
Implémente #260
(évite les comparaisons de chaînes au profit des vrais booléens et
entiers)
Grosse simplification et réduction du code relatif à la configuration.
Supprime ConfigurationDAO.
Permet de simplifier considérablement configureController.
Évite de multiples copies des mêmes données en mémoire.
Évite de garder plusieurs versions de la configuration en mémoire
(auparavant : dans un tableau au niveau de ModelArray + au niveau de
FreshRSS_Configuration + en Session + des copies temporaires comme
ConfigurationDAO).
Ne stocke plus 'conf' en Session (n'était presque pas utilisé).
Évite de recharger plusieurs fois Translate inutilement.
Contribue à #303
Alkarex added a commit that referenced this issue Dec 30, 2013
+ Légère optimisation de Minz_View.
+ Encore plus de tests de bibliothèques dans install.php
Contribue à #126 et
#303
Alkarex added a commit that referenced this issue Dec 30, 2013
Alkarex referenced this issue Jan 1, 2014
Minz ne prenait pas en charge OPcache (cache PHP) http://php.net/opcache
activé par défaut depuis PHP5.5.
Ce fut un peu dur d'isoler ce bug :-/
Il faut penser à appeler opcache_invalidate avant de ré-utiliser un
fichier par include().
Aussi, le mécanisme de lock() n'est plus approprié ni nécessaire.
Pour FreshRSS, évite l'utilisation de ModelArray car il ne restait que
quelques lignes d'utiles, et évite un héritage + appel de classe, ce qui
est toujours ça de gagné.
@Alkarex
Copy link
Member Author

Alkarex commented Jan 2, 2014

Un peu d'inspiration :-)
http://fgallaire.flext.net/arretez-decrire-des-classes/

@Alkarex
Copy link
Member Author

Alkarex commented Mar 22, 2014

@marienfressinaud : Le paramètre Minz_Request::$reseted semble être assez cher, et être utilisé uniquement dans le cas de Minz_Request::forward() quand la fonction est appelée avec redirect = false, ce qui n'est que rarement le cas (presque uniquement pour le login et pour Minz_Error::error()), et quand c'est le cas, ce n'est utile que si un contenu de réponse a déjà commencé à être généré.

De plus, à part la compression (qui peut être faite ailleurs), Minz_Request::$reseted semble être la seule raison pour l'utilisation en interne des fonctions ob_*. Supprimer les ob_* internes aiderait entre autres pour #163

En résumé, je pense qu'il serait possible d'améliorer les performances en supprimant Minz_Request::$reseted et en s'assurant que les erreurs principales (celles qui appellent Minz_Error::error()) soient envoyées avant de générer le contenu de la réponse.

Alkarex added a commit that referenced this issue Mar 22, 2014
#303 (comment)
#163
* Remove Minz_Response (not needed anymore)
* Move Minz_Request::reseted to Minz_Dispatcher::reset()
@Alkarex
Copy link
Member Author

Alkarex commented Mar 22, 2014

@marienfressinaud J'ai fait un patch expérimental, qui supprime une couche de ob_ et du code devenu inutile. Cela semble bien marcher. J'ai gardé Minz_Request::$reseted mais en déplaçant ce concept dans une fonction Minz_Dispatcher::reset(). Dis-moi ce que tu en penses.

@Alkarex
Copy link
Member Author

Alkarex commented Mar 22, 2014

@marienfressinaud Minz_View::partial() et Minz_View::renderHelper() font la même chose. Que dirais-tu de les fusionner (garder uniquement partial()) ?
Il y aurait le répertoire app/views/helpers à bouger dans app/layout/helpers par exemple.

@marienfressinaud
Copy link
Member

Je ne suis pas contre la suppression des ob_ par contre je ne suis pas convaincu par la suppression du système de reset. J'ai du mal à voir en quoi cette opération est coûteuse.

Le problème de bouger app/views/helpers dans app/layout/helpers c'est au niveau du sens. Ce qui est contenu dans app/views/helpers ne correspond pas vraiment au "layout" donc je propose plutôt de garder les deux fonctions mais qui en appellent une interne et prenant en paramètres

  1. Ce qui correspond à $part ou $helper
  2. self::LAYOUT_PATH_NAME ou '/views/helpers/' (qu'on pourrait au passage mettre dans un constante de classe self::HELPER_PATH_NAME) pour indiquer dans quel répertoire on doit taper.

@Alkarex
Copy link
Member Author

Alkarex commented Mar 23, 2014

Dans ce patch expérimental, je n'ai pas supprimé le système de reset, mais je l'ai refactorisé en Minz_Dispatcher::reset() pour éviter une variable statique publique que je ne trouvais pas très propre. C'est la couche de ob_ supplémentaire qui est coûteuse (avec plusieurs copies de toute la sortie texte), pas la variable reset en elle-même.

Je n'ai pas bien compris la classification "layout" ou "helper", par exemple en comparant le contenu de views/helpers/view/global_view.phtml et app/layout/nav_entries.phtml.

Si ça ne peut pas être fusionné, autant laisser les fonctions telles qu'elles sont plutôt que d'ajouter une indirection supplémentaire.

@marienfressinaud
Copy link
Member

Ok donc c'est bon pour moi pour les modifs au niveau des ob_*

Pour la classification ça peut effectivement ne pas être très clair :s Je considère que ce qui va dans layout a rapport avec tout ce qui n'est pas lié directement au contenu. Il s'agit de la structure de la page et des outils de navigation. Les helpers par contre sont directement liés au contenu. Après c'est sujet à discussion effectivement et certains fichiers auraient leur place dans chacun des deux répertoires.

@marienfressinaud
Copy link
Member

Ah par contre, pourquoi avoir fait tes modifs directement dans la branche 163-export ? C'est effectivement utile dans cette branche mais ça aurait tout à fait sa place dans dev

@Alkarex
Copy link
Member Author

Alkarex commented Mar 23, 2014

J'ai un peu hésité en effet, mais j'ai démarré ces patchs à la suite des discussions sur ob_ liées à l'export

@Alkarex
Copy link
Member Author

Alkarex commented Mar 23, 2014

Pour être sûr d'être clair sur le ob_, la modification expérimentale améliore les performances, mais ne permet plus de faire un reset après avoir commencé à envoyer la sortie texte. Je n'ai néanmoins pas l'impression que cela soit utilisé, et ce n'est pas très propre dans tous les cas.

@marienfressinaud
Copy link
Member

Il n'y a pas de soucis, le reset est géré lors d'une redirection qui ne devrait être faite que dans le Controller. Et en principe aucun echo ne devrait être fait à ce niveau là

@Alkarex
Copy link
Member Author

Alkarex commented Mar 23, 2014

Il reste un point à regarder à propos de la compression. Chez moi, c'est Apache (voir mod_deflate dans p/.htaccess) qui fait la compression des sorties PHP (pas vérifié, mais je pense intuitivement que cela doit être plus efficace). Néanmoins, cela peut être remis côté PHP, par exemple dans p/i/index.php au niveau de la fonction httpConditional qui accepte un paramètre permettant d'activer la compression.

P.S.:
http://php.net/ob-gzhandler

note that using zlib.output_compression is preferred over ob_gzhandler()

http://stackoverflow.com/questions/1862641/compressing-content-with-php-ob-start-vs-apache-deflate-gzip

@marienfressinaud
Copy link
Member

Je pense qu'on peut se contenter de la compression par Apache d'autant plus que je ne pense pas qu'on puisse s'en sortir pour les fichiers JS / svg / etc. autrement… Après on est dépendant d'Apache mais ce n'est pas non plus critique et je suppose que Nginx propose quelque chose de similaire.

@Alkarex
Copy link
Member Author

Alkarex commented Mar 24, 2014

Ok. J'ai ajouté une constante pour facilement activer la compression par PHP si souhaité.

Alkarex added a commit that referenced this issue Mar 24, 2014
As suggested
#163 (comment)

At the same time, removes a bunch of (almost) dead code such as
Minz_Router (the few remaining lines being moved to Minz_FrontController
to avoid a class)

Contributes to #303
@marienfressinaud marienfressinaud modified the milestones: 2.0.0, 0.10-dev Dec 9, 2014
@math-GH
Copy link
Contributor

math-GH commented Sep 4, 2022

Any news here or ready to close it?

@math-GH math-GH added the Vote to close Maybe it's time to close that issue! label Sep 4, 2022
@math-GH math-GH added System care Everything related to the system care Feature Request ideas for new features labels Sep 4, 2022
@Alkarex
Copy link
Member Author

Alkarex commented Sep 4, 2022

Profiling our PHP code for the most common requests would still be nice to do (finding the most costly calls / lines)

@math-GH math-GH removed the Vote to close Maybe it's time to close that issue! label May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request ideas for new features System care Everything related to the system care
Projects
None yet
Development

No branches or pull requests

3 participants