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

Synthese - Travail sur les performances #560

Open
TheoLechemia opened this issue Feb 14, 2019 · 18 comments
Open

Synthese - Travail sur les performances #560

TheoLechemia opened this issue Feb 14, 2019 · 18 comments

Comments

@TheoLechemia
Copy link
Member

Un travail a été amorcé ces derniers jours sur les performances de la synthèse. Petit retour des enseignements tirés:

Côté front:

  • passage au mode "canvas" de leaflet (https://leafletjs.com/reference-1.0.0.html#map-prefercanvas). D'origine leaflet crée un élément de DOM par point créé sur la carte; en mode canvas, les points sont dessinés dans une balise html "canvas", ce qui soulage très largement le navigateur (j'ai réussi à afficher 50 000 points sans plantage)
  • Non utilisation du composant 'geojson' dans la synthese. On avait créé un composant pnx-geojson dans le coeur du front pour afficher des geoson sur nos cartes. C'est très pratique mais peu performant (il multiplie les boucles, et ça à l'air d'être inhérent aux fonction leaflet qu'il utilise - L.GeoJson() notamment). En construisant les points/lignes/polygones à la "main" (avec L.polyline, L.circleMarker() et L.polygon()) on est plus performant (voir: https://github.com/PnX-SI/GeoNature/blob/perfs/frontend/src/app/syntheseModule/synthese-results/synthese-carte/synthese-carte.component.ts#L133). Le temps de chargement a été réduit à 4 secondes pour 50 000 points. Avec le composant c'est plus autour de 10 sec...

Côté back:

  • En changeant la syntaxe de construction des requêtes ( select([my_modele]) VS DB.session.query(my_model) on améliore de 2 à 3 secondes une même requête avec un LIMIT 50000. Il semblerait que la 2ème syntaxe crée des dizaines d'alias sur les tables et colonnes et ralenti le tout. (Voir cette discussion: https://groups.google.com/forum/#!topic/sqlalchemy/IXins449qOo). La 1ère syntaxe, plus rapide, a l'inconvenient de renvoyer des tuples et non des objets du modèle. Mais vu le point suivant, ce n'est pas un problème
  • la sérialisation en geojson est terriblement lente... Le décorateur "@geoserializable" qui permet de transformer nos modèles en geojson n'est pas performant ( et on y peut rien, c'est la libraire shapely'qui transforme un WKB en WKT puis en geojson qui fais ramer le tout). Un "st_asgeojson" en base est largement plus efficace. Il faut donc privilégier une vue côté base qui fait un maximum de travail (au niveau géographique en tout cas), plutôt que déporter le travail côté python.

En améliorant tout ça on a une synthese qui utilise moins nos outils génériques mais qui est beaucoup plus rapide.
Pour la requete GET qui revoie toutes les obs de la synthèse, on est passé de 8 à 1,5 secondes !
De bout en bout, on passe d'environ 18 secondes à 7-8 secondes pour charger 50 000 obs (depuis l'appel de la requête à son chargement effectif sur la carte en front).

@camillemonchicourt
Copy link
Member

Super, et couplé avec l'ajout du plugin Leaflet Cluster, on peut maintenant afficher bien plus de résultats dans la Synthèse : #559

TheoLechemia added a commit that referenced this issue Feb 19, 2019
@TheoLechemia
Copy link
Member Author

Suite de l'amélioration des perfs sur les export de la synthese.

La vue v_synthese_decode_nomenclatures utilisé dans la vue des exports qui remet à plat toutes les nomenclatures utilise une fonction ref_nomenclatures.get_nomenclature_label(id). Cette fonction plombe complètement les performances. Elle a été retiré pour faire des jointures à la main.
On passe de 40s à 1s pour la génération de la vue.

Une autre piste évoqué par Gil est de mettre un index sur cette fonction. Voir: https://www.postgresql.org/docs/9.6/indexes-expressional.html et https://www.developpez.net/forums/d1711400/bases-donnees/postgresql/forcer-l-utilisation-d-index/

@camillemonchicourt
Copy link
Member

@gildeluermoz a travaillé sur les performances de l'export Synthèse notamment : 6633de4

explain analyse SELECT * FROM gn_synthese.v_synthese_for_export
WHERE "dateDebut" > '2018-01-01' and "dateDebut" < '2019-12-31';

Sur cette requête on est maintenant sous la seconde pour 42600 données (675ms).

En interface, c'est plus long car il faut télécharger le fichier (18 Mo dans cet exemple), et il y a aussi surement des optimisations de la sérialisation Python possibles. Voir #801

Sur les exports Occtax, il a aussi réalisé des optimisations dans ce commit (notamment avec la réduction du GROUP BY aux seules clés primaires des tables du FROM et JOIN), mais il lui reste encore des mystères :

explain analyse select * from pr_occtax.export_occtax_sinp
WHERE date_min > '2018-01-01' and date_min < '2019-12-31';

Sur cette requête on a 4.96s pour 31000 données.
La différence majeure entre ces 2 vues c'est que la synthèse est un modèle à plat et que Occtax répartit les observations dans 3-4 tables. Le GroupAggregate avec un "Group Key: ccc.id_counting_occtax, occ.id_occurrence_occtax, rel.id_releve_occtax, d.id_dataset" est fortement pénalisant.

On le voit ici : https://explain.depesz.com/s/VVJS

Sur l'explain on voit qu'il n'y a pas non plus d'usage des index des FK et ça je n'ai pas réussi à le corriger. Ça reste une énigme.

On dirait que PG rame à faire des jointures, qu'il n'utilise pas les index des PK et des FK. J'ai vérifié et créé les index manquants, ré-indexé les tables, fait des vacuum et des analyse. Pas de changement.

@jbrieuclp
Copy link
Contributor

Y a un truc zarb dans cette requête (à moins quelle ne soit pas à jour chez moi), c'est cette jointure LEFT JOIN taxonomie.taxref tax ON tax.cd_nom = occ.cd_nom car cette table tax n'est pas utilisée dans le SELECT (et de fait ni dans le GROUP BY), en revanche c'est ça qui est employé pour récuperer le cd_ref taxonomie.find_cdref(occ.cd_nom) AS "cdRef", là ou tax.cd_ref AS "cdRef", doit faire le même taf... #shadock ?

Aussi est-ce que faire les multiples jointures avec la table ref_nomenclatures.t_nomenclatures pour récuperer les id plutôt que d'utiliser de nombreuse fois la fonction ref_nomenclatures.get_cd_nomenclature() n'optimiserait pas la requête.

C'est des hypothèses, j'ai pas test !.. Mais j'ai du mal à penser que faire appel plusieurs fois à des fonctions qui réalise en arrière plan des SELECT avec jointure et condition soit super optimisée.

@camillemonchicourt
Copy link
Member

Les dernières versions des requêtes revues par @gildeluermoz sont visibles dans son commit d'hier : 6633de4#diff-b085f02ab00ad5314adf9b708c23b98c

@jbrieuclp
Copy link
Contributor

Ok, ben mon commentaire reste d'actualité !

@amandine-sahl
Copy link
Contributor

Effectivement, par contre je suis suprise que l'emploi de fonction plutôt que de join augmente les performances.

@jbdesbas
Copy link
Contributor

Salut,
Pour la sérialisation en geoJson, nouveauté sur Postgis 3 (https://postgis.net/2019/10/20/postgis-3.0.0/) :

1833, ST_AsGeoJSON(row) generates full GeoJSON Features (Joe Conway)

Si c'est bon en perf, ca pourrait valoir le coup de passer à postgis 3 vu l'utilisation régulière des geojson.

@jbdesbas
Copy link
Contributor

A t'il déjà été évoqué d'utiliser le système héritage de table de PGSQL pour la nomenclature ? Je n'ai jamais vraiment utilisé ça, mais, en lisant la doc, ca semble coller très bien à nos besoins et voici les avantages dont on pourrait bénéficier :

  • Les requêtes UPDATE/SELECT/DELETE sur la table mère (t_nomenclatures) continuerait à fonctionner normalement. On peut toujours interroger la globalité de la nomenclature via la table mère. Il n'est donc pas obligatoire que tout les type_nomenclature ait une table dédié.
  • Les nombreuses jointures sur les tables filles (ex : t_nomenclatures_statut_obs) seraient beaucoup plus performantes (car tables de quelques lignes seulement)
  • Les requêtes sembleront plus claires (plutôt que d'avoir 10 jointures sur t_nomenclature, on aurait sur t_nomenclature_nivvalid, t_nomenclature_naturalness, etc..)
  • On remplace les "CHECK ref_nomenclatures.check_nomenclature_type_by_mnemonique(...." (qui sont en fait des pseudo clés étrangères) par des vrais clé étrangères vers les tables filles (performance ++ ?)
  • Il serait possible d'ajouter des colonnes sur un type de nomenclature seulement (je ne vois pas encore de cas d'usage cependant)

La seule grosse limite que j'ai identifié, c'est qu'une clé étrangère référençant la table mère n'autorise PAS les valeurs des tables filles (mais le problème sera facilement détectable).

Est-ce que quelqu'un à déjà utilisé l'héritage ? Qu'en pensez vous ?

@camillemonchicourt
Copy link
Member

Sur l'héritage je ne sais pas.
Par contre, si on doit créer une table fille pour chaque type de nomenclature, il me semble qu'on perd toute la généricité et la souplesse de centraliser les nomenclatures dans une table et un référentiel.

@jbdesbas
Copy link
Contributor

En partie oui, mais la nomenclature générique reste interrogeable sur la table mère. Les tables filles peuvent ne concerner que les types nomenclatures officiels. La généricité est déjà partiel aujourd'hui, étant donné que les valeurs type_nomenclature sont en dure dans les contraintes des tables qui référence t_nomenclature (gn_synthese entre autres).

@jbdesbas
Copy link
Contributor

jbdesbas commented Jun 5, 2020

Salut,
La compression gzip des fichiers JSON n'est pas activée par défaut sur apache2.
Il suffit d'ajouter la ligne suivante AddOutputFilterByType DEFLATE application/json dans /etc/apache2/mods-enabled/deflate.conf et de relancer apache2.
Je n'ai pas fait de benchmark a proprement parlé, mais les fichiers (geo)json étant généralement très redondants, je constate bien une réduction de 90% de la taille des fichiers envoyés au client, ce qui n'est probablement pas négligeable en perf.

@TheoLechemia
Copy link
Member Author

Interessant.
Il y a aussi une lib flask qui permet de le faire, pour les moins expert en administration de serveur : https://github.com/colour-science/flask-compress
J'ai testé sur un geojson de 50 000 obs, on passe de 26.7mb à 2.6 !
Qu'est-ce que vous en pensez ? Lib python integrée au code ou conf apache ?

@camillemonchicourt
Copy link
Member

Intéressant. Dans ce cas là librairie Python me semble une solution plus pérenne.

@jbdesbas
Copy link
Contributor

jbdesbas commented Jun 5, 2020

Je suis tombé sur Flask-compress aussi, mais voir l'avertissement ici : https://pypi.org/project/Flask-Compress/

The preferred solution is to have a server (like Nginx) automatically compress the static files for you. If you don't have that option Flask-Compress will solve the problem for you.

Probablement que la compression Python est moins efficace ?

@camillemonchicourt
Copy link
Member

OK dans ce cas, si c'est plus performant de cette manière, on peut imaginer le faire directement dans la configuration Apache proposée par défaut dans GeoNature.

@gildeluermoz
Copy link
Contributor

J'ai testé l'ajout de la ligne AddOutputFilterByType DEFLATE application/json dans /etc/apache2/mods-enabled/deflate.conf sur notre serveur de prod.
Avec une requête en synthèse qui renvoie 26500 obs, je passe de 15.6s à 1.54s pour la réception du json... Le temps d'attente (préparation de la réponse par le serveur) est identique.
On est donc bien sur un facteur 10.
Merci @jbdesbas

@jpm-cbna jpm-cbna changed the title Travail sur les performances Synthese - Travail sur les performances Dec 20, 2022
@camillemonchicourt
Copy link
Member

La compression des json et geojson a été activée dans la configuration Apache fournie par défaut, depuis la version 2.12 : #2266

La mise en place d'agrégation des géométries dans la Synthèse (#1881) a nécessité de ne plus utiliser le fonctionnement de construction des GeoJSON qui avait été mis en place précédemment.

Cette agrégation améliore aussi bien les performances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants