Expérimente le cache des paramètres

[fpagnoux]

• Amélioration technique (expérimentale)
• Détails :
  • Cache les paramètres de la législation pour accélérer son chargement

---

Ces changements:

• Modifient des éléments non fonctionnels de ce dépôt

---

Cette PR n'est pas en l'état prête à être mergée, mais ouverte pour discussion.

Problème

La législation modélisée dans ce dépôt étant fonctionnellement étendue, elle contient un grand nombre de paramètres législatifs, déclarés dans des fichiers YAML. Le parsing de ces fichiers par le moteur OpenFisca est couteuse en terme de performances, un problème régulièrement exprimé par certains utilisateurs (cc @guillett).

Proposition

Cette PR propose de cacher l'arbre des paramètres parsé dans un pickle pour pouvoir le re-charger plus rapidement.

Impact

L'impact en termes de performances est assez significatif:

Temps de chargement de la législation d'OpenFisca-France:
(moyenne de 3 essais chargeant chacun 10 fois la législation, sur une machine plutôt performante)

Avec cache	Version actuelle (Avec libyaml1)	Version actuelle (Sans libyaml1)
0.66s	1.3s	4.8s

¹ libyaml est une librairie système (non python) à installer pour accélérer le parsing des fichiers YAML. OpenFisca affiche un warning quand elle n'est pas installée, mais la procédure d'installation est non-triviale, et plateforme-dépendante

Discussion

Les gains de performances semblent prometteurs, mais introduire un cache complexifie le chargement et comporte toujours un risque d'être confronté à des débuggages pénibles ("Mais pourquoi mes changements ne sont pas pris en compte?!"). Cf https://martinfowler.com/bliki/TwoHardThings.html.

Vu les ordres de grandeurs des gains en perfs, j'ai l'impression que ça vaut le coup. Qu'est-ce que vous en pensez ?

[Morendil]

Qu'est-ce qui manque pour que ça puisse être mergé?

Pour moi les +/- sont:

• (+) c'est succinct en nombre de lignes et très localisé
• (-) je suis un peu gêné par l'absence de tests unitaires, et inquiet d'ajouter des choses au constructeur qui en fait déjà trop (le constructeur de TaxBenefitSystem devrait faire le minimum, le chargement automatique à partir d'un répertoire devrait être le job d'une factory method prenant le répertoire en paramètre)

[benjello]

A priori OK

[benjello]

@fpagnoux :

• quels sont les cas où un utilisateur lambda pourrait être piégé ?
• Pourquoi utiliser appsdir plutôt que pyxdg ?

[Morendil]

@fpagnoux L'app Codacy signale l'utilisation de Pickle comme une vulnérabilité, il semble que c'est bien connu. Les avantages de Pickle en termes de performance ne semblent pas si évidents, il est possible que les gains de performance que tu relèves ne viennent pas du changement de parser mais du fait de regrouper toutes les données et d'éviter des inefficacités dans le traitement des I/O. On pourrait envisager MessagePack comme alternative, les benchmarks le donnent nettement plus rapide.

[sandcha]

Passée de POC en WIP suite à l'accord en revue.

[bonjourmauko]

Hey @fpagnoux et @Morendil, avez-vous envisagé d'utiliser lru_cache ? Je l'utilise comme suit :

from functools import lru_cache
from openfisca_france import FranceTaxBenefitSystem

CachedFranceTaxBenefitSystem = lru_cache()(FranceTaxBenefitSystem)
tbs = CachedFranceTaxBenefitSystem()

@guillett

[sandcha]

La dernière proposition (lru_cache) vient a priori répondre à la vulnérabilité que tu as soulevée @Morendil. Qu'en dis-tu ?

[Morendil]

@sandcha Mon objection est devenue caduque: n'étant plus engagé sur la maintenance de France, c'est aux personnes qui vont l'assurer de se prononcer.

Cela dit pour donner une simple opinion, je ne crois pas que ce que propose @maukoquiroga aie le même effet que le code de cette PR. Si j'ai bien compris, le cache LRU de functools est stocké en mémoire; ce que cherchait à faire @fpagnoux c'est un cache persistant stocké sur disque.