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

Optimisations #18

Closed
wants to merge 8 commits into from
Closed

Optimisations #18

wants to merge 8 commits into from

Conversation

blackheaven
Copy link
Contributor

Improving display time by concatening js and css file.

@benjaminplanche
Copy link
Member

Hey,

Actif dis donc ... ^^
Mes remarques :

  • Certaines images me semblent rétrécies au delà de la taille à laquelle elles sont généralement utilisées. A checker donc si on perd pas trop en qualité du coup.
  • Ta concaténation m'a fait découvrir qu'on incluait jquery-ui ... A vérifier, mais je crois pas que ce js / css soit utilisé au final, ou alors dans des modules non-livrés (CV) et/ou cas restreints (en tout cas jamais sur les pages statiques et l'annuaire). Donc ca peut valoir le coup de checker pour carrément supprimer ce qui n'est plus utilisé.
  • Si tu cherches à optimiser certaines performances, y a aussi le gzippage ou Closure Compiler, pour le js, qui peuvent être envisageables, mais tu connais probablement.

Je sais pas si j'aurai le temps ce week-end de checker tout cela (peut-être pourras-tu ?), mais ce que je te propose, c'est d'ici la semaine prochaine, on refait le tour des pull-requests, on valide ensemble, et on merge ... ?

Bonne journée, bye

@blackheaven
Copy link
Contributor Author

Bonjour,

  • J'ai rapidement vérifié, à priori il n'y a pas de changement sensible (si on compare une photos dans les deux versions on voit à peine la différence), mais je veux bien un second avis.
  • Il faudrait faire un audit pour savoir qu'est-ce qui est utilisé et qu'est-ce qui ne l'est pas, mais j'avoue que je n'est pas bien eu le temps, je vais voir ça en début de semaine.
  • Actuellement j'ai divisé par deux la taille du transfère et par 4 le temps d'affichage lors du premier chargement, les minimifier ne feront pas gagner grand chose, par contre assembler les images et jouer avec le CSS si.

Je suis en rush jusqu'à jeudi, on peut voir ça à ce moment là ?

@benjaminplanche
Copy link
Member

  • Je jetterai un oeil une fois sur mon PC. ;-)
  • Same.
  • Closure Compiler, par exemple, fait plus que minimifier. Il peut jouer sur les perf du codes, en inlinant des fonctions, et peut aussi aider à repérer des tournures louches (j'ai fait le test tout à l'heure sur annuaire.class.js qui m'a permis de capter des virgules de trop dans des objets, tsss ...). Mais yep, c'est juste du bonus.

Pas de soucis pour attendre. J'essayerai de couvrir quelques issues d'ici-là, pour profiter pleinement de la release.
Bon courage, 'toute.

@PandiPanda69
Copy link
Contributor

[Mode aigri]

  • Pourquoi s'occuper des performances alors qu'il y a pleins de Lorem
    ipsum à remplacer par du vrai texte?

[/Mode aigri]

JQuery-ui, il me semble que Daniel s'en servait au niveau simulation
d'entretien pour avoir un datepicker mais il est passé par une autre
solution.
Normalement, on peut le dégager.

Le 1 février 2013 07:46, Benjamin Planche [email protected] a
écrit :

  • Je jetterai un oeil une fois sur mon PC. ;-)
  • Same.
  • Closure Compiler, par exemple, fait plus que minimifier. Il peut
    jouer sur les perf du codes, en inlinant des fonctions, et peut aussi aider
    à repérer des tournures louches (j'ai fait le test tout à l'heure sur *
    annuaire.class.js* qui m'a permis de capter des virgules de trop dans
    des objets, tsss ...). Mais yep, c'est juste du bonus.

Pas de soucis pour attendre. J'essayerai de couvrir quelques issues
d'ici-là, pour profiter pleinement de la release.
Bon courage, 'toute.


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-12982829.

@blackheaven
Copy link
Contributor Author

  • Pourquoi s'occuper des performances alors qu'il y a pleins de Lorem ipsum à remplacer par du vrai texte?

Car c'est pas nous qui nous en occupons, si on savait quoi mettre, ça serait fait :/

@PandiPanda69
Copy link
Contributor

C'était un tacle gratuit, ne le prends pas pour toi personnellement ;-)
Vous vous bougez, c'est une bonne chose, même si vous avez pu commettre des
maladresses dans la communication qui ont été interprétées parfois comme
des attaques envers nous par rapport à notre travail passé.

Personnellement, c'est le peu de suivi du bureau qui m'a démotivé petit à
petit, donc pas votre faute ;-)
Si jamais tu peux les relancer afin qu'on puisse enfin mettre en ligne le
site dans sa globalité, ce serait un énorme pas en avant!

Le 1 février 2013 12:51, blackheaven [email protected] a écrit :

  • Pourquoi s'occuper des performances alors qu'il y a pleins de Lorem
    ipsum à remplacer par du vrai texte?

    Car c'est pas nous qui nous en occupons, si on savait quoi mettre, ça
    serait fait :/


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-12991095.

@PandiPanda69
Copy link
Contributor

Bill me dit que ça se voit pas que j'essaie d'être gentil =/

Bon alors si si! Le mail précédent est censé être gentil! Merci pour votre
dynamisme et je sais pas si vous pouvez le communiquer au bureau pour qu'on
puisse mettre en ligne le site totalement :-)

J'espère que ça fait plus gentil :o

Je termine même le message par poutoux, la ça doit tout changer!

Poutoux

Le 1 février 2013 12:58, Sébastien Mériot [email protected] a
écrit :

C'était un tacle gratuit, ne le prends pas pour toi personnellement ;-)
Vous vous bougez, c'est une bonne chose, même si vous avez
pu commettre des maladresses dans la communication qui ont été interprétées
parfois comme des attaques envers nous par rapport à notre travail passé.

Personnellement, c'est le peu de suivi du bureau qui m'a démotivé petit à
petit, donc pas votre faute ;-)
Si jamais tu peux les relancer afin qu'on puisse enfin mettre en ligne le
site dans sa globalité, ce serait un énorme pas en avant!

Le 1 février 2013 12:51, blackheaven [email protected] a écrit :

  • Pourquoi s'occuper des performances alors qu'il y a pleins de Lorem
    ipsum à remplacer par du vrai texte?

    Car c'est pas nous qui nous en occupons, si on savait quoi mettre, ça
    serait fait :/


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-12991095.

@benjaminplanche
Copy link
Member

Hey,

  • Le calendrier jquery-ui semble toujours utilisé pour les simulations d'entretiens. Au choix :
    • On ne met donc les dépendances jquery-ui que pour ces quelques pages concernées.
    • On trouve un moyen moins "overkill" d'avoir ce calendrier.
  • Après visualisation (vu que c'est mis en prod), je trouve perso vraiment dommage la qualité des images, surtout pour une page d'accueil. Pour une résolution de 1200px de large comme celle de mon PC, ca étire les images à 2 fois leur nouvelle taille (770px pour 385px réels), rendant le tout affreusement pixellisé et donc tâchant avec l'image pro voulue. Pour moi le gain en perf' ne vaut pas ce prix, mais ce n'est que mon avis. Si y a moyen, ca vaudrait le coup de demander au reste de l'AEDI peut-être ?

@PandiPanda69
Copy link
Contributor

Malheureusement, les simulations d'entretien ne verront pas le jour pour X
raisons. Je pense qu'on peut supprimer jquery-ui.

Les images sont effectivement hyper floues & pixelisées, même sur mon PC
portable (et sur mobile c'est l'apocalypse).

Le 3 février 2013 14:20, Benjamin Planche [email protected] a
écrit :

Hey,

  • Le calendrier jquery-ui semble toujours utilisé pour les simulations
    d'entretiens. Au choix :

    • On ne met donc les dépendances jquery-ui que pour ces quelques
      pages concernées.
    • On trouve un moyen moins "overkill" d'avoir ce calendrier.
      • Après visualisation (vu que c'est mis en prod), je trouve perso
        vraiment dommage la qualité des images, surtout pour une page d'accueil.
        Pour une résolution de 1200px de large comme celle de mon PC, ca étire les
        images à 2 fois leur nouvelle taille (770px pour 385px réels), rendant le
        tout affreusement pixellisé et donc tâchant avec l'image pro voulue. Pour
        moi le gain en perf' ne vaut pas ce prix, mais ce n'est que mon avis. Si y
        a moyen, ca vaudrait le coup de demander au reste de l'AEDI peut-être ?


    Reply to this email directly or view it on GitHubhttps://github.com/Optimisations #18#issuecomment-13046891.

@benjaminplanche
Copy link
Member

J'ai pas de mobile pour checker, mais c'est possible que là le problème vienne d'un code foireux que j'avais implémenté pour redimensionner les images. J'ai réglé le problème dans le pull-request en attente.
Ca change rien à la pixellisation, though.

@blackheaven
Copy link
Contributor Author

On enlève jquery-ui du coup ?
On pourrai virer les simulations au passage.

Quelle images voyez-vous pixelisées ?

@PandiPanda69
Copy link
Contributor

On enlève jquery-ui du coup ?

Si ça te fait plaisir

On pourrai virer les simulations au passage.

On pourrait.

Quelle images voyez-vous pixelisées ?

Quasiment toutes... surtout au niveau du caroussel....

@blackheaven
Copy link
Contributor Author

C'est louche, je vais faire des tests pour trouver un compromis.

@PandiPanda69
Copy link
Contributor

Daniel, l'auteur des simulations d'entretien a du recevoir le message je
pense. Il nous dira lui même s'il veut que son taf soit supprimé, mais vu
le temps passé dessus, je ne veux pas prendre la décision pour lui.

L'AEDI (en copie) nous dira également son opinion à savoir, doit-on
supprimer le module des simulations d'entretien du site, voir d'autres?
Avez-vous du texte pour remplacer les lorem ipsum d'ailleurs?

On y a passé du temps (des nuits blanches entre autre), je trouve ça
dommage de devoir supprimer notre travail pour le refaire sans aucun doute
dans 1 ou 2 ans car la nécessité se fera sentir. Marie Rosain espérait
beaucoup du module de simulation d'entretien et est assez déçue qu'il soit
tombé aux oubliettes.

Benjamin Bouvier, l'auteur des stages, me confirmait également hier qu'il
était un peu atristé de voir que la partie stage n'était pas utilisée. Il
s'exprimera lui même également.

Pour les images, elles sont floues chez toutes l'équipe, je laisse
également Benjamin Bouvier, Bill & co confirmer (perso, j'ai un petit 14"
non HD et c'est quand même flou).

Le 4 février 2013 13:56, blackheaven [email protected] a écrit :

C'est louche, je vais faire des tests pour trouver un compromis.


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-13075587.

@benjaminplanche
Copy link
Member

  • Pour le module :
    • Ben jusqu'à présent, les modules non intégrés n'ont pas été virés du dépot, juste pas inclus dans les lots livrés, ce qui est déjà le cas pour les simulations ... Du coup je propose juste qu'on vire jquery-ui du layout pour l'inclure uniquement pour les pages des entretiens, et laisser le tout prendre la poussière jusqu'à ce que quelqu'un déterre l'idée un jour ... ?
  • Pour les images :
    • Yep, toutes (bien que seules celles du carousel soient en prod' pour l'instant). Elles devront s'afficher la plupart du temps en plus de 750px de large, ca semble donc la taille limite à leur donner (taille fixée avant).

@blackheaven
Copy link
Contributor Author

Petite clarification, je parlais de virer dans le sens retirer de la production, il est bien évident que je ne compte pas purement et simplement supprimer le travail, je veux simplement éviter le code mort et dé-complexifier les sources.
J'ai effectivement un soucis de pixellisation (merci le cache qui se vide qu'avec un rm -Rf), je vais tenter de réduire le soucis.

@benjaminplanche
Copy link
Member

  • Yep, je supposais, mais le mot portait à confusion, du coup j'ai préféré avoir la certitude ... Du coup ca t'irait si on vire juste les includes UI du layout pour les mettre que pour le module concerné ?
  • J'ai vu que tu avais remis les images taille réelle en prod', c'est cool. Si y a des doutes sur la solution à choisir, consulte peut-être le reste de l'équipe ...

'toute

[Edit : correction "." --> "?"]

@blackheaven
Copy link
Contributor Author

Effectivement, je m'en excuse, je raisonnais par rapport à ce qui est accessible par le web.
Voilà ce que j'aimerais faire (dans l'ordre), dites moi si c'est possible ou non :

  • enlever (de la production) les modules non fonctionnels/utilisés (le but ici est de réduire la taille du code)
  • nettoyer le code (enlever les parties non-utilisées (exemple : jquery-ui), sécuriser cette partie (cf. Depreciation de mysql_escape_string #19 ))
  • optimiser les pages vues par l'entreprise (typiquement 5 secondes c'est trop long, si on pouvait passer la barre des 1.5 ça serait acceptable)
  • intégrer et nettoyer ( Depreciation de mysql_escape_string #19 ) les modules retirés en première phase

J'aimerais votre avis si le sujet.

@benjaminplanche
Copy link
Member

  • J'approuve, mais ce serait pas déja le cas ?
  • Of course
  • Hum, là j'ai plus de mal à voir ce qui peut être fait ... Genre la page sur les RIF coute 14ko avec le cache, ce qui me semble raisonnable ... Tu songeais à quoi ?
  • Of course

@PandiPanda69
Copy link
Contributor

  • C'est déjà le cas en effet, il y a juste des "coming soon" en prod
  • k
  • Alors j'avais pas fait gaffe, mais en effet, les images du carousel
    mettent du temps à se charger... Problème de débit mais à l'époque c'était
    pas aussi flagrant. D'ailleurs, pendant longtemps on utilisait un lien
    externe (bonjourlesifs) donc on peut rebasculer pour gagner en bandwidth
  • k

Le 4 février 2013 23:03, Benjamin Planche [email protected] a
écrit :

  • J'approuve, mais ce serait pas déja le cas ?

  • Of course

  • Hum, là j'ai plus de mal à voir ce qui peut être fait ... Genre la
    page sur les RIF coute 14ko avec le cache, ce qui me semble raisonnable ...
    Tu songeais à quoi ?

  • Of course


    Reply to this email directly or view it on GitHubhttps://github.com/Optimisations #18#issuecomment-13102295.

@blackheaven
Copy link
Contributor Author

  • Sans doute, je n'ai pas encore pris la main sur le code, je ne suis pas sur ce point
  • Je pensais plus à faire une glyph, c'est le temps d'accès qui coûte cher (en temps), niveau bande passante, on pourrait faire mieux (pour l'instant on a 2 Mo à télécharger pour afficher la page d'accueil).
  • Les deux premiers points seront les plus compliqués, ça ira ensuite.

@benjaminplanche
Copy link
Member

Une idée pour diluer le temps de chargement de l'index serait de mettre en place une technique de lazy-load pour les images du carousel. C'est relativement simple à faire, voir cette proposition : http://stackoverflow.com/questions/10291729/twitters-boostrap-carousel-optimization-solution-needed (solution qui peut facilement être ameliorée, par exemple en préchargeant l'image suivante dans le défilement automatique juste avant la transition, pour ne pas avoir un "blanc" le temps du chargement).

Après il y a tellement de définition au mot "performance" pour le Web que je suis pas sûr si cette proposition répond à la problematique soulevée ... ? Reste que dans tous les cas, ca peut être un plus, rapide à implémenter ...

[Edit: Un exemple de solution pour ce que je proposais entre parenthèses : http://stackoverflow.com/questions/11303075/bootsrap-carousel-lazy-loader?rq=1. Et plus j'y songe et plus je me dis que le lazy-load offrirait un gain non-négligeable, notamment dans les nombreux cas où les utilisateurs ne prendront pas le temps de survoler tout le carousel ...]

@benjaminplanche
Copy link
Member

Rien à faire au boulot, du coup j'ai bouquiné la doc de html5-boilerplate, et j'ai trouvé des choses qui m'ont semblé interessantes sur les performances coté serveur cette fois, en bidouillant le .htaccess :

Ils montrent notamment une facon clean et dynamique de concaténer le js ou css, comment améliorer le cache, gzipper, configurer les ETags, ...
Perso je suis pas back-end, donc c'est peut-etre des évidences pour vous, mais ca coute rien de partager.

@blackheaven
Copy link
Contributor Author

Pour le lazy-loading : bonne idée
Pour la configuration du serveur : j'ai configuré nginx au max déjà (gzip en 9/9), ça a fait gagner pas mal, mais ça ne suffit pas :/

@PandiPanda69
Copy link
Contributor

On parle de perf on on gzip, je comprends pas tout... Dans tous les cas, le
zippage de png est inutile puisqu'il y a déjà une compression LZW.

Lazy-loading, je suis frileux car ça ne règlera pas le véritable problème
de bandwidth mais à tester.

Le 6 février 2013 15:44, blackheaven [email protected] a écrit :

Pour le lazy-loading : bonne idée
Pour la configuration du serveur : j'ai configuré nginx au max déjà (gzip
en 9/9), ça a fait gagner pas mal, mais ça ne suffit pas :/


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-13184869.

@PandiPanda69
Copy link
Contributor

Ignorez mon message

Le 6 février 2013 15:51, Sébastien Mériot [email protected] a
écrit :

On parle de perf on on gzip, je comprends pas tout... Dans tous les cas,
le zippage de png est inutile puisqu'il y a déjà une compression LZW.

Lazy-loading, je suis frileux car ça ne règlera pas le véritable problème
de bandwidth mais à tester.

Le 6 février 2013 15:44, blackheaven [email protected] a écrit :

Pour le lazy-loading : bonne idée

Pour la configuration du serveur : j'ai configuré nginx au max déjà (gzip
en 9/9), ça a fait gagner pas mal, mais ça ne suffit pas :/


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-13184869.

@benjaminplanche
Copy link
Member

Le lazy-loading peut en effet avoir des inconvénients ... J'implémenterai et ferai des tests dans la semaine.
Par contre, @blackheaven, si t'es sur le serveur, tu pourrais jeter un oeil / répondre pour #10 et #24 s'il te plaît ? Ca me semble relativement prioritaire (bugs en prod'). Thanks !

@blackheaven
Copy link
Contributor Author

Bah grosso-modo dans le htaccess passé (c'est pénible à lire) tu as quelques headers + une compression gzip (pour économiser la bande passante), j'ai déjà activé ces headers ainsi que la compression sous nginx (au maximum).

@PandiPanda69
Copy link
Contributor

Je suis un peu ennuyé pour merger cette PR. Est-ce que c'est ok ou pas?

Est-ce que ce serait possible à l'avenir d'avoir des PR plus unitaire afin que ce soit plus facile de situer les modifications et pouvoir tester? :-)

@benjaminplanche
Copy link
Member

Hum, cette PR me semble pas entièrement cohérente avec ce qui est en production (je songe notamment aux images, encore réduites au max avec cette PR), donc perso je suis réservé ...

@blackheaven
Copy link
Contributor Author

laisse cette PR en suspens, je la corrigerais en fin de semaine

@PandiPanda69
Copy link
Contributor

ok, je peux basculer en prod l'ensemble des merges que je viens de faire?

2013/2/6 blackheaven [email protected]

laisse cette PR en suspens, je la corrigerais en fin de semaine


Reply to this email directly or view it on GitHubhttps://github.com//pull/18#issuecomment-13186775.

@blackheaven
Copy link
Contributor Author

ecrase tout alors

@blackheaven blackheaven closed this Feb 8, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants