Nous avons eu récemment une discussion très intéressante sur le GitHub de Zeste de Savoir concernant le refactoring d’un bout code gérant la "pagination" des contenus. En effet en haut d’une page article nous proposons de naviguer vers l’article suivant ou précédant. Cette fonctionnalité devait être implémenté pour les tribunes. Lors de la relecture du code de Situphen, je suis tombé sur un bout de code intéressant.
Le code qui gérait la sélection du contenu suivant et précédant en base de données était pour le suivant:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 | if self.object.type == 'ARTICLE': # fetch all articles in order to find the previous and the next one all_articles = \ [a for a in PublishedContent.objects .filter(content_type='ARTICLE', must_redirect=False) .order_by('publication_date') .all()] articles_count = len(all_articles) try: position = all_articles.index(self.public_content_object) except ValueError: pass # for an unknown reason, the article is not in the list. This should not happen else: context['paginate_articles'] = True context['previous_article'] = None context['next_article'] = None if position > 0: context['previous_article'] = all_articles[position - 1] if position < articles_count - 1: context['next_article'] = all_articles[position + 1] |
On peut reprocher ce code les choses suivantes:
- On réalise une requête sur tous les articles de la base pour connaître uniquement le précédent et le suivant. Autrement dit, on réalise un SELECT en SQL sur plusieurs dizaines (centaines un jour) d’articles pour en récupérer deux.
- On boucle sur l’ensemble de ces articles pour réaliser une liste, ce qui ne me semble pas utile ici.
- On compte les articles à l’aide de la méthode len() qui permet de renvoyer la taille d’une liste en Python.
- On récupère ensuite la position du contenu en cours de lecture dans la liste, puis on va chercher le contenu précédent et le contenu suivant, tout en faisant attention aux cas extrêmes. Notons qu’ici la stratégie choisie est de demander la permission (test de valeur avant l’accès), une autre solution aurait été de demander le pardon plutôt que la permission.
Maintenant que nous avons vu les points faibles de ce code, voyons comment améliorer tout cela: - On recherche des meilleurs performances, surtout avec le nombre de contenus qui augmentent - On recherche un code plus lisible et plus Pythonique, si possible.
Après plusieurs échanges avec motet-a nous arrivons au code suivant:
1 2 3 4 5 6 7 8 | if self.object.type == 'ARTICLE': queryset_pagination = PublishedContent.objects.filter(content_type='ARTICLE', must_redirect=False) context['previous_article'] = (queryset_pagination .filter(publication_date__lt=self.public_content_object.publication_date) .order_by('publication_date').first()) context['next_article'] = (queryset_pagination .filter(publication_date__gt=self.public_content_object.publication_date) .order_by('publication_date').first()) |
On peut déjà noter que ce code ne fait que quelques lignes au lieu d’une vingtaine dans le cas initial. On gagne donc en concision. Voici comment est réalisé l’opération.
- On prépare une requête de base qui contient tous les critères de sélection (ici les articles, sans redirections)
- On complète ensuite cette requête pour obtenir le premier contenu ayant une date de publication plus grande et le premier contenu ayant une date de publication moins grande. Ils correspondent aux contenus précédent et suivant du code initial.
Nous avons ici obtenu un code plus clair et qui ne va plus chercher en base l’ensemble des articles. L’objectif semble donc réussit, vérifions cela à l’aide de quelques chiffres. J’ai réalisé sur mon instance locale un petit benchmark sur un ensemble de jeu de 120 articles publiés, ce qui me parait assez proche de la réalité. L’ancien code met un temps moyen de 12.9ms sur 5 essais. Le nouveau code, dans les même conditions s’exécute en 2ms en moyenne, soit environ 6 fois plus rapidement.
Au delà du gain de performance (certes léger à cette échelle) il faut surtout considérer que le deuxième code est beaucoup plus scalable. En effet peu importe le nombre de contenus publiés, le temps d’exécution restera le même (a peu de choses près). En revanche l’ancien code sera de plus en plus long quand le nombre de contenus va augmenter.
On réalise le même test avec 960 articles publiés, voyons les résultats:
- Ancien code : 490ms en moyenne
- Nouveau code : 3ms en moyenne
On peut donc voir que la nouvelle méthode reste presque stable alors que l’ancien code mets 160 fois plus de temps à s’exécuter !
Un peu de SQL
Voyons au niveau SQL la différence entre l’ancien code (récupération de tous les articles) et la première requêtes du nouveau code (récupération de l’article précédent)
1 2 3 | SELECT CHAMPS WHERE ("tutorialv2_publishedcontent"."content_type" = ARTICLE AND "tutorialv2_publishedcontent"."must_redirect" = False) ORDER BY "tutorialv2_publishedcontent"."publication_date" ASC SELECT CHAMPS WHERE ("tutorialv2_publishedcontent"."content_type" = ARTICLE AND "tutorialv2_publishedcontent"."must_redirect" = False AND "tutorialv2_publishedcontent"."publication_date" < 2017-12-17 21:34:30.572639) ORDER BY "tutorialv2_publishedcontent"."publication_date" ASC' LIMIT 1 |
Les deux seules différences sont l’ajout d’un critère supplémentaire dans la clause WHERE (date de publication) et la limitation à 1 seul résultat. Petit inconvénient dans le deuxième code il faut réaliser deux requêtes à la place d’une seule. D’un point de vue performance il faut mieux néanmoins réaliser deux petites requêtes précises, qu’une énorme requête dont la majorité de résultats ne sont pas utilisés.
Pour conclure je préciserai que l’auteur original avait justement prédit cela dans son commit : "(fix #313, rip performances)" 
Enfin, il pourrait être intéressant de se pencher sur les choses suivantes
- Réaliser un test avec le vrai moteur de base de données utilisé en prod et non pas SQLite utilisé uniquement par les développeurs
- Vérifier grâce à des statistiques du serveur de prod que le temps nécessaire au chargement de cette vue a bien diminué.
- Monitorer de plus près les requêtes SQL gourmandes et inclure un décompte des requetes dans les tests de vues grâce à assertNumQueries.
Merci à motet-a, Situphen et pierre-24 pour leur contribution.
