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

multiple-pause-resume.sh: lower the default loop count from 5 to 1 #1219

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Jul 10, 2024

There is a combinatorial explosion in this test because it tries by default to test all pairs of available pipelines. This typically blows up and times out in NOCODEC configurations see #706 and others for earlier discussions. Internally, this combinatorial explosion issue was raised as early as November 2020! (Issues 40, 581,...)

This tiny commit does not fix the root cause but it makes a dead simple, "quality of life" adjustment: it simply lowers the default number of iterations from 5 to 1. There is no obvious reason why the default should be 5 and every test plan can raise that value back up if it wants to. Unlike #706 which was abandoned because it reduces test coverage, this very small change merely reduces the default test duration without changing any of its logic.

Also, log the combination tested. Sample output:

declare -a pipeline_combine_lst=([0]="0 1" [1]="0 2" [2]="0 3" [3]="0 4"
[4]="0 5" [5]="0 6" [6]="0 7" [7]="0 8" [8]="0 9" [9]="0 10" [10]="1 2"
[11]="1 3" [12]="1 4" [13]="1 5" [14]="1 6" [15]="1 7" [16]="1 8"
[17]="1 9" [18]="1 10" [19]="2 3" [20]="2 4" [21]="2 5" [22]="2 6"
[23]="2 7" [24]="2 8" [25]="2 9" [26]="2 10" [27]="3 4" [28]="3 5"
[29]="3 6" [30]="3 7" [31]="3 8" [32]="3 9" [33]="3 10" [34]="4 5"
[35]="4 6" [36]="4 7" [37]="4 8" [38]="4 9" [39]="4 10" [40]="5 6"
[41]="5 7" [42]="5 8" [43]="5 9" [44]="5 10" [45]="6 7" [46]="6 8"
[47]="6 9" [48]="6 10" [49]="7 8" [50]="7 9" [51]="7 10" [52]="8 9"
[53]="8 10" [54]="9 10")

There is a combinatorial explosion in this test because it tries by
default to test all pairs of available pipelines. This typically blows
up and times out in NOCODEC configurations see thesofproject#706 and others for
earlier discussions. Internally, this combinatorial explosion issue was
raised as early as November 2020! (Issues 40, 581,...)

This tiny commit does not fix the root cause but it makes a dead simple,
"quality of life" adjustment: it simply lowers the default number of
iterations from 5 to 1. There is no obvious reason why the default
should be 5 and every test plan can raise that value back up if it wants
to. Unlike thesofproject#706 which was abandoned because it reduces test coverage,
this very small change merely reduces the default test duration without
changing any of its logic.

Also, log the combination tested. Sample output:

```
declare -a pipeline_combine_lst=([0]="0 1" [1]="0 2" [2]="0 3" [3]="0 4"
[4]="0 5" [5]="0 6" [6]="0 7" [7]="0 8" [8]="0 9" [9]="0 10" [10]="1 2"
[11]="1 3" [12]="1 4" [13]="1 5" [14]="1 6" [15]="1 7" [16]="1 8"
[17]="1 9" [18]="1 10" [19]="2 3" [20]="2 4" [21]="2 5" [22]="2 6"
[23]="2 7" [24]="2 8" [25]="2 9" [26]="2 10" [27]="3 4" [28]="3 5"
[29]="3 6" [30]="3 7" [31]="3 8" [32]="3 9" [33]="3 10" [34]="4 5"
[35]="4 6" [36]="4 7" [37]="4 8" [38]="4 9" [39]="4 10" [40]="5 6"
[41]="5 7" [42]="5 8" [43]="5 9" [44]="5 10" [45]="6 7" [46]="6 8"
[47]="6 9" [48]="6 10" [49]="7 8" [50]="7 9" [51]="7 10" [52]="8 9"
[53]="8 10" [54]="9 10")
```

Signed-off-by: Marc Herbert <[email protected]>
@marc-hb

This comment was marked as outdated.

@marc-hb marc-hb marked this pull request as ready for review July 10, 2024 06:44
@marc-hb marc-hb requested a review from a team as a code owner July 10, 2024 06:44
@fredoh9
Copy link
Collaborator

fredoh9 commented Jul 10, 2024

LNLM_RVP_NOCODEC takes 8-9 minutes for multiple-pause-resume.sh with this pr, loop_count=1
I this this is reasonable duration for PR test.

Copy link
Collaborator

@fredoh9 fredoh9 left a comment

Choose a reason for hiding this comment

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

default loop count is 1
And thank you for printing pipeline_combine_lst also.

@fredoh9 fredoh9 merged commit 9834162 into thesofproject:main Jul 10, 2024
5 of 7 checks passed
@marc-hb marc-hb deleted the multiple-pause-loop-once branch July 10, 2024 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants