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

141 investigate the impact of pairing taskprogressionisinterruptrequested with periodictestinterruptionistestallowed #197

Conversation

marcboulle
Copy link
Collaborator

@marcboulle marcboulle commented Mar 19, 2024

Essentiellement, dans le commit principal (nettemenent plus détaillé dans les notes de commit)

  • TaskProgressionManager::IsInterruptionResponsive
    • on considère desormais que le test d'interruption n'est jamais "responsive", et on protège toujours par une timer
  • TaskProgression::IsRefreshNecessary
    • nouvelle méthode permettant de bufferiser la fabrication des messages pour DisplayTaskProgesssion et le test d'interruption utilisateur
    • utilise un modulo et les timers deja existant dans TaskProgression
    • remplace l'utilisation de PeriodicTest::IsTestAllowed
  • PeriodicTest: suppression de la classe et de tous ses usages
  • TaskProgression::Set|GetInterruptible
    • si GetInterruptible vaut false, la méthode TaskProgression::IsInterruptionRequested renvoie systematiquement false
      sauf si une interruption a été forcée par programme par la methode ForceInterruptionRequested (utilisee dans les CrashTests)
    • paramétrage
      • quand on lance les outils en mode batch (cf. UIObject::SetUIMode)

Plus deux petits commits indépendant:

  • corrections mineures dans les scripts de LearningTest
  • corrections mineures pour des warning de compilation dans la librairie Parallel (à valider par Bruno)

Devrait permettre de clore également l'issue #140

…type

- Bug in copy files that are in suite dirs but do not belong to any test dir
  Example: \LearningTest\TestKhiops\CrashTests\README.txt
- Bug get_context_computing_type: now return "sequential" if process_number is None or is equal to 1
Besoins
- amelioration de la gestion des interruptions utilisateurs
  - mieux gerer la degradation des performances
  - evaluer l'utilisation actuelle de TaskProgression::IsInterruptionRequested et periodicTestInterruption::IsTestAllowed
  - evaluer la possibilite des desactiver la gestion des interuption utilisateurs en mode batch

Audit rapide du code
- Stats de nombre d'utilisation de methodes critiques
  - TaskProgression::IsInterruptionRequested: 269
  - TaskProgression::DisplayTaskProgesssion: 85
  - PeriodicTest::IsTestAllowed: 47 (dont dont 17 fois IsTestAllowed(0))
- classe TaskProgression
  - DisplayTaskProgesssion
    - les affichages effectifs sont bufferises avec un timer interne (toutes les 0.3 s)
  - IsInterruptionRequested
    - les test effectifs sont effectues
      - immediatement si TaskProgressionManager::IsInterruptionResponsive vaut true
      - bufferises avec un timer interne sinon
- classe TaskProgressionManager
  - IsInterruptionResponsive: methode virtuelle
  - FileTaskProgressionManager: responsive=True (et IsInterruptionRequested renvoie toujours false)
  - UITaskProgression: responsive = false
    - coherent avec l'interface java
  - PLMPISlaveProgressionManager: IsInterruptionResponsive = true
    - non coherent avec l'utilisation des probe dans MPI
- PeriodicTest::IsTestAllowed
  - temporise
    - par un modulo
    - et un timer
  - utilise pour diminuer
    - le nombre de test d'interruption: redondant avec IsInterruptionRequested dans le cas protege par un timer
    - le nombre de DisplayTaskProgesssion: redondant avec IsInteDisplayTaskProgesssion toujours protege par un timer
    - le nombre de construction de message construire pour l'appel a DisplayTaskProgesssion

Evolution
- TaskProgressionManager::IsInterruptionResponsive: supression de la methode
  - on considere desormais que le test d'interruption n'est jamais "responsive", et on protege toujours par une timer
- TaskProgression::IsRefreshNecessary: nouvelle methode
  - permet de bufferiser la fabrication des messages pour DisplayTaskProgesssion et le test d'interruption utilisateur
  - utilise un modulo et les timers deja existant dans TaskProgression
  - remplace l'utilisation de PeriodicTest::IsTestAllowed
- PeriodicTest: supression de la classe et de tous ses usages
  - une cinquantaine d'impcat un peu partout, remplace par des appels a TaskProgression::IsRefreshNecessary, plus simples
- TaskProgression::Set|GetInterruptible
  - si GetInterruptible vaut false, la methode TaskProgression::IsInterruptionRequested renvoie systematiquement false
     - sauf si une interruption a ete forcee par programme par la methode ForceInterruptionRequested
       (utilisee dans les CrashTests et dans les tests unitaires de la librairie Parallel)
  - parametrage
    - quand on lance les outils en mode batch (cf. UIObject::SetUIMode)

 Tests:
 - validation prealable de tout LearningTest V10.5.0-a1, en mode sequentiel comme en parallele
   - une vingtaine de jeux de test sont concernes par une version parallel
 - archivage de la version de reference LearningTest V10.5.0-a1
 - apres prise en compte de l'évolution
   - verification des tests (de non regression (family=full)) en batch sur tout LearningTest, en sequentiel, en parallel
   - verification que les interruption utilisateurs sont testees dans les CrashTests, meme en batch
   - verification qu'aucune interruption 'est prise en compte en mode batch, sequentiel et parallele
   - verification que les interruptions utilisateurs sont prises en compte en mode iunterface graphique, en sequentiel et parallele
     - tests manuels sur Adult
Mainly:
- add variable initializasation in class constructors
- cast int operands in arithmetic operations when the result is a longint
- plus some minor formating errors detected during pre-commit
@marcboulle marcboulle force-pushed the 141-investigate-the-impact-of-pairing-taskprogressionisinterruptrequested-with-periodictestinterruptionistestallowed branch from 91fbf97 to 5148dd5 Compare March 19, 2024 16:55
Copy link
Contributor

@bruno-at-orange bruno-at-orange left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@folmos-at-orange folmos-at-orange left a comment

Choose a reason for hiding this comment

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

LGTM

@marcboulle marcboulle merged commit d0012ae into dev Mar 20, 2024
26 checks passed
@marcboulle marcboulle deleted the 141-investigate-the-impact-of-pairing-taskprogressionisinterruptrequested-with-periodictestinterruptionistestallowed branch March 20, 2024 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type/Enhancement New feature or request
Projects
None yet
3 participants