Compare la peformance d'un commit à celle de master #1012

Anna-Livia · 2018-06-11T16:20:14Z

~~⚠️ Attention : cette branch est pour le moment comparée à test_perf~~

Connected to #1002
Connected to #1010

Changement mineur.
Détails :
- Ajoute un test de performance sur les tests de CircleCI
- Ce test peut être lancé avec la commande:
  python performance_tests/test_perf_circleci.py 1717 1716

Ces changements :

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

bonjourmauko · 2018-06-12T16:30:19Z

Hi @Anna-Livia, test_perf a déjà été mergé.

bonjourmauko · 2018-06-12T16:31:05Z

performance_tests/test_perf_circleci.py

+"""
+This script takes in two CircleCI build numbers
+(usually the `build_python2` and `build_python2` of the same workflow)
+and outputs if their runtime in in the same ball park as the master branche's


and outputs if their runtime is in the same ball park as the master branche's

bonjourmauko · 2018-06-12T16:31:57Z

performance_tests/test_perf_circleci.py

+def get_master_branch_performance():
+    """
+    Accesses the CircleCI API and gets the info for all available master branch builds.
+    :return: an Dict with the master branch build time statistics


bonjourmauko · 2018-06-12T16:34:14Z

performance_tests/test_perf_circleci.py

+            builds.append(response['build_time_millis'])
+
+    if len(builds) < 5:
+        logging.warning('two few master branch builds')


two few => too few => not enough ?

bonjourmauko · 2018-06-12T16:38:31Z

Autre question, quelle est la relation entre cette implementation et celle proposée dans #1010 ?

Anna-Livia · 2018-06-12T16:44:29Z

@maukoquiroga ce sont deux façons d'estimer la performance des tests :

Ajoute un test de perfomance des tests #1010 propose de comparer un deux sous-ensemble important des tests YAML avec un build local et une baseline écrite en dur.
Compare la peformance d'un commit à celle de master #1012 compare des builds entiers sur circle ci et compare à la branche master.

sandcha · 2018-06-12T18:27:33Z

performance_tests/test_perf_circleci.py

@@ -0,0 +1,98 @@
+"""
+This script takes in two CircleCI build numbers
+(usually the `build_python2` and `build_python2` of the same workflow)


'build_python2' and 'build_python3'?

Déplacer ce fichier dans openfisca_france/scripts/ ?
Et supprimer test_perf_tests.py qui a été traité par #1010 ?

bonjourmauko · 2018-06-12T18:52:17Z

@Anna-Livia Je faisais référence au fait qu'il fallait pointer la PR à master.

sandcha

En l'état, le point bloquant est l'emplacement du script (et test_perf_tests.py qui devient obsolète).

sandcha · 2018-06-12T18:32:24Z

performance_tests/test_perf_circleci.py

@@ -0,0 +1,98 @@
+"""
+This script takes in two CircleCI build numbers
+(usually the `build_python2` and `build_python2` of the same workflow)


Déplacer ce fichier dans openfisca_france/scripts/ ?
Et supprimer test_perf_tests.py qui a été traité par #1010 ?

sandcha · 2018-06-12T18:45:56Z

performance_tests/test_perf_circleci.py

+    def mean(numbers):
+        return sum(numbers, 0.0) / len(numbers)
+
+    def standard_deviation(numbers):


sandcha · 2018-06-12T18:50:25Z

performance_tests/test_perf_circleci.py

+            builds.append(response['build_time_millis'])
+
+    if len(builds) < 5:
+        logging.warning('too few master branch builds')


'Too few circle builds on master branch'?

sandcha · 2018-06-12T18:55:21Z

performance_tests/test_perf_circleci.py

+current_build = get_current_build_performance(python2_build_number, python3_build_number)
+
+
+if current_build > master_branch['mean'] * 1.10:


Fixer à 20% comme pour le test de tests ?

sandcha · 2018-06-12T18:56:37Z

performance_tests/test_perf_circleci.py

+    sys.exit('This build may be making the test performance worst')
+
+elif current_build < master_branch['mean'] + master_branch['standard_deviation']:
+    print('Performances are good')


Indiquer l'écart (valeur de standard_deviation) avec master ?

sandcha · 2018-06-12T18:57:52Z

performance_tests/test_perf_circleci.py

+import json
+import logging
+
+python2_build_number = sys.argv[1]


En l'absence de deux arguments, ajouter une trace indiquant ce qu'attend le script ?

bonjourmauko

Super boulot, je suis super content de #1010 et ce cette pull request. Parce que :

cela montre que le problème est complexe et inconnu, donc on ne connait pas encore la meilleure métrique
nous avons besoin de plusieurs options pour voir quelles sont les plus adaptées pour résoudre Rendre les tests plus performants #926
les deux peuvent être encore refactored, et cela est parfait, car il s'agit de trouver le chemin le plus court pour implémenter Rendre les tests plus performants #926

Nous ne savons pas encore quelle est la meilleure façon de tester la performance, et on va le découvrir en l'améliorant 😃.

À mon avis cela closes #1001 et #1002, et on peut passer à #1005.

Attention, pourtant, à prendre en compte l'impact des refactorings dans la performance des tests (il faire attention à diminuer l'impact d'un changement de la méthode pour observer sur la performance de l'observation elle-même).

bonjourmauko · 2018-06-12T19:28:12Z

En l'état, le point bloquant est l'emplacement du script (et test_perf_tests.py qui devient obsolète).

Mais on veut utiliser les deux non ?

Anna-Livia requested review from bonjourmauko, fpagnoux and sandcha June 11, 2018 16:21

bonjourmauko reviewed Jun 12, 2018

View reviewed changes

sandcha reviewed Jun 12, 2018

View reviewed changes

bonjourmauko changed the base branch from test_perf to master June 12, 2018 18:51

sandcha requested changes Jun 12, 2018

View reviewed changes

bonjourmauko approved these changes Jun 12, 2018

View reviewed changes

bonjourmauko assigned Anna-Livia Jun 13, 2018

bonjourmauko added meta:performance meta:developer-experience kind:investigation flow:team:doing labels Jun 13, 2018

Anna-Livia force-pushed the perf_circle_ci branch from 7ed276e to ab37d07 Compare June 13, 2018 08:04

Anna-Livia requested a review from sandcha June 13, 2018 14:20

sandcha approved these changes Jun 13, 2018

View reviewed changes

Anna-Livia and others added 6 commits June 13, 2018 18:42

Compares a couple of circleci build runtime with the master branch

66f6d87

Bump version

362e73f

Correct typos

fecaa15

Change place of files and change the calculations

be2232d

Explicite la configuration du test de performance des builds

e88bfb2

Clarifie la documentation du test de test_circleci_builds

e03480a

sandcha force-pushed the perf_circle_ci branch from b35db99 to e03480a Compare June 13, 2018 16:42

sandcha merged commit 1885f2a into master Jun 13, 2018

sandcha deleted the perf_circle_ci branch June 13, 2018 18:31

bonjourmauko changed the title ~~Compare les peformances d'un commit à celles de la branch Master~~ Compare la peformance d'un commit à celle de master Jul 23, 2018

bonjourmauko removed the flow:team:doing label Jul 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compare la peformance d'un commit à celle de master #1012

Compare la peformance d'un commit à celle de master #1012

Anna-Livia commented Jun 11, 2018 •

edited by bonjourmauko

Loading

bonjourmauko commented Jun 12, 2018

bonjourmauko Jun 12, 2018 •

edited

Loading

bonjourmauko Jun 12, 2018

bonjourmauko Jun 12, 2018

bonjourmauko commented Jun 12, 2018

Anna-Livia commented Jun 12, 2018

sandcha Jun 12, 2018

sandcha Jun 12, 2018

bonjourmauko commented Jun 12, 2018

sandcha left a comment

sandcha Jun 12, 2018

sandcha Jun 12, 2018

sandcha Jun 12, 2018

sandcha Jun 12, 2018

sandcha Jun 12, 2018 •

edited

Loading

sandcha Jun 12, 2018

bonjourmauko left a comment

bonjourmauko commented Jun 12, 2018

		current_build = get_current_build_performance(python2_build_number, python3_build_number)


		if current_build > master_branch['mean'] * 1.10:

Compare la peformance d'un commit à celle de master #1012

Compare la peformance d'un commit à celle de master #1012

Conversation

Anna-Livia commented Jun 11, 2018 • edited by bonjourmauko Loading

bonjourmauko commented Jun 12, 2018

bonjourmauko Jun 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonjourmauko commented Jun 12, 2018

Anna-Livia commented Jun 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonjourmauko commented Jun 12, 2018

sandcha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandcha Jun 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonjourmauko left a comment

Choose a reason for hiding this comment

bonjourmauko commented Jun 12, 2018

Anna-Livia commented Jun 11, 2018 •

edited by bonjourmauko

Loading

bonjourmauko Jun 12, 2018 •

edited

Loading

sandcha Jun 12, 2018 •

edited

Loading