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

Mise en place d'un cache #51

Merged
merged 9 commits into from Jul 6, 2014
Merged

Mise en place d'un cache #51

merged 9 commits into from Jul 6, 2014

Conversation

paternal
Copy link
Contributor

@paternal paternal commented Jul 2, 2014

Lors de l'analyse d'une chanson toto.sg (avec plasTeX), si aucun fichier de cache toto.sg.cache n'est présent (ou si toto.sg a changé, ou si toto.sg.cache a été généré par une ancienne version de songbook), l'analyse est faite, et une version simplifiée de l'objet Song est enregistrée dans toto.sg.cache. Au contraire, si ce fichier toto.sg.cache est correcte, l'analyse plasTeX n'est pas faite, et c'est la version enregistrée dans le cache qui est utilisée.

Quelques statistiques (avec l'ensemble des chansons de patacrep-data) :

  • Branche master (sans le cache) :

    $ time songbook data/books/songbook.sb --steps=tex
    [patacrep.build] INFO: Building 'songbook.tex'…
    real    0m41.100s
    user    0m40.892s
    sys     0m0.136s
  • Première exécution avec le cache (donc toutes les chansons doivent être cachées). C'est plus long, mais pas énormément.

    $ time songbook data/books/songbook.sb --steps=tex
    [patacrep.build] INFO: Building 'songbook.tex'…
    real    0m47.636s
    user    0m41.100s
    sys     0m0.432s
  • Seconde exécution avec le cache : aucune chanson n'est analysée à nouveau, le cache est utilisé à chaque fois. Comme on le voit, il y a un gain de temps assez important.

    $ time songbook data/books/songbook.sb --steps=tex
    [patacrep.build] INFO: Building 'songbook.tex'…
    real    0m0.836s
    user    0m0.692s
    sys     0m0.140s

La branche n'est pas prête à être mergée : il reste quelques warnings disgracieux.

Ça pourra être particulièrement utile pour patanet.

@paternal
Copy link
Contributor Author

paternal commented Jul 2, 2014

Du coup, si cette branche est finalisée et acceptée, je pense que #23 peut être abandonné.

@Luthaf
Copy link
Contributor

Luthaf commented Jul 2, 2014

J'aime beaucoup !

Pourquoi ne pas faire plutôt un cache par datadir, dans un dossier song/__cache__ (ou song/__patacache__ ^^) par exemple ? Cela éviterai de polluer les dossiers manipules par les utilisateur (dossier song).

@@ -4,16 +4,50 @@
"""Song management."""

from unidecode import unidecode
import cPickle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je crois qu'il est conseille de faire plutot

try:
    import cPickle as pickle
except ImportError:
    import pickle

@paternal
Copy link
Contributor Author

paternal commented Jul 2, 2014

Pourquoi ne pas faire plutôt un cache par datadir, dans un dossier song/__cache__ (ou song/__patacache__ ^^) par exemple ? Cela éviterai de polluer les dossiers manipules par les utilisateur (dossier song).

On n'a pas l'information sur le datadir dans lequel on est au moment de la création du cache, mais on peut l'avoir en modifiant deux lignes de code. Le problème, c'est qu'avec le plugin cwd, cette notion de datadir n'existe plus vraiment (puisqu'on peut alors importer des chansons qui viennent d'un répertoire arbitraire).

Du coup, pour ne pas alourdir le code, on peut :

  • interdire à cwd d'aller chercher des chansons hors d'un datadir ;
  • utiliser un fichier de cache global ~/.patacrep/songcache (je ne sais pas à quoi ça ressemblerait sous windows) ;
  • prendre la version actuelle, en préfixant les fichiers de cache par un point pour les cacher (sous GNU/Linux ; je ne sais pas si on peut faire quelque chose de similaire sous Windows).

Pas de préférence (peut-être une légère pour la première, qui me paraît la plus simple). Convainquez-moi !

@Luthaf
Copy link
Contributor

Luthaf commented Jul 2, 2014

C'est dommage d'interdire à cwd d'aller en dehors des datadirs, alors qu'il est principalement fait pour cela.

Je propose une solution mixte : si le fichier contient songs dans son chemin, on suppose que c'est un datadir, et on met tout là. Sinon, on fait un dossier de cache par dossier utilisé par cwd ou tout autre plugin.

Le point au début du nom de fichier peut être bien aussi.

@paternal
Copy link
Contributor Author

paternal commented Jul 2, 2014

Je propose une solution mixte : si le fichier contient songs dans son chemin, on suppose que c'est un datadir, et on met tout là. Sinon, on fait un dossier de cache par dossier utilisé par cwd ou tout autre plugin.

Pas la peine de deviner : on a la liste des datadirs. Il suffit donc de tester si c'est un sous-chemin de notre chanson.

On peut aussi dire que les chansons hors d'un datadirs ne sont pas mises en cache.

@Luthaf
Copy link
Contributor

Luthaf commented Jul 3, 2014

Ca me va comme solution.

@paternal
Copy link
Contributor Author

paternal commented Jul 5, 2014

  • Prise en compte des dernières avancées de master.
  • Les chansons sont mises en cache dans <datadir>/.cache/songs. J'ai laissé le songs, comme ça, si plus tard, on a envie de metter autre chose en cache, on n'aura pas de conflit de nom.
  • Les chansons n'appartenant pas à un datadir ne sont pas mises en cache.

Pas encore prêt à merger : toujours quelques warnings lors de la génération de l'index.

@paternal
Copy link
Contributor Author

paternal commented Jul 6, 2014

Si ça vous convient, c'est bon pour merger.

Luthaf added a commit that referenced this pull request Jul 6, 2014
Mise en place d'un cache
@Luthaf Luthaf merged commit a3beec2 into master Jul 6, 2014
@Luthaf Luthaf deleted the cache branch July 6, 2014 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants