Skip to content

CI: Travis: less VM isolation for speed-up. Remove matrix. - #2743

Merged
lgirdwood merged 1 commit into
thesofproject:masterfrom
marc-hb:smaller-travis-matrix
Apr 9, 2020
Merged

CI: Travis: less VM isolation for speed-up. Remove matrix.#2743
lgirdwood merged 1 commit into
thesofproject:masterfrom
marc-hb:smaller-travis-matrix

Conversation

@marc-hb

@marc-hbmarc-hb commented Apr 8, 2020

Copy link
Copy Markdown
Collaborator

Looking at any recent Travis build log:

  1. more than half the time is spent in the exact same "docker pull"
    command,
  2. The qemuboottest stage rebuilds again the exact same thing than the
    previous test stage.

Fix 1. by re-using the same docker instance for multiple platforms.
Fix 2. by dropping from the test stage builds performed again in
the qemuboottest stage.

Random sample before:

 Total (VM) time 1 hr 15 min
Real time 25 min (depends on current Travis load)

After:

 Total (VM) time 30 min
Real time 10 min (depends on current Travis load)

The price to pay for this matrix reduction and speed up is coarser
reports in case of failure. Considering these tests are the most basic
possible one expects them to be rarely ever broken.

Remove the top-level matrix expansion as it was becoming impractical for
these heterogeneous builds ("PLATFORM=tools"?!). The combination of the
matrix and YAML anchors was not very obvious. Use YAML anchors
exclusively.

Rename default stage "test" to "buildonly"

Signed-off-by: Marc Herbert marc.herbert@intel.com

@monocodusmonocodusBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is autogenerated code-style review, all good, approving

@marc-hb
marc-hb marked this pull request as ready for review April 8, 2020 06:41
@marc-hb
marc-hb requested review from singalsu and xiulipanApril 8, 2020 06:41
@marc-hb

Copy link
Copy Markdown
CollaboratorAuthor

Travis before:

Real time 25 min
Total (VM) time 1 hr 15 min

After:

Real time 10 min
Total (VM) time 30 min

@lgirdwoodlgirdwood left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a good improvement, but can we also give "test" more meaningful title e.g. test->buildtest

Comment thread.travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

jsl (should) and sue (probably) have qemu support too IIRC.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

OK, I didn't want to hide adding these in this unrelated and large (LoC) refactoring. Break only one thing a time ;-)

This PR makes more obvious that they were left out.

Comment thread.travis.yml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Testtbench is showing platform = sue which is wrong as it runs on host.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

For now it is showing "byt" because of the way Travis matrix works, also wrong.

I'll see if I can do something about this.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Done with a name:.

Note Travis never shows both a name: and env: on the same line

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Done.

@marc-hbmarc-hb left a comment

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

can we also give "test" more meaningful title e.g. test->buildtest

Will do.

Comment thread.travis.yml Outdated

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

OK, I didn't want to hide adding these in this unrelated and large (LoC) refactoring. Break only one thing a time ;-)

This PR makes more obvious that they were left out.

Comment thread.travis.yml Outdated

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

For now it is showing "byt" because of the way Travis matrix works, also wrong.

I'll see if I can do something about this.

@marc-hb
marc-hbforce-pushed the smaller-travis-matrix branch from 0d27c10 to f98098dCompareApril 9, 2020 07:09

@monocodusmonocodusBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is autogenerated code-style review, all good, approving

Looking at any recent Travis build log:
1. more than half the time is spent in the exact same "docker pull"
command,
2. The qemuboottest stage rebuilds again the exact same thing than the
previous test stage.
Fix 1. by re-using the same docker instance for multiple platforms.
Fix 2. by dropping from the test stage builds performed again in
the qemuboottest stage.
Random sample before:
Total (VM) time 1 hr 15 min
Real time 25 min (depends on current Travis load)
After:
Total (VM) time 30 min
Real time 10 min (depends on current Travis load)
The price to pay for this matrix reduction and speed up is coarser
reports in case of failure. Considering these tests are the most basic
possible one expects them to be rarely ever broken.
Remove the top-level matrix expansion as it was becoming impractical for
these heterogeneous builds ("PLATFORM=tools"?!). The combination of the
matrix and YAML anchors was not very obvious. Use YAML anchors
exclusively.
Rename default stage "test" to "buildonly"
Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
marc-hbforce-pushed the smaller-travis-matrix branch from f98098d to 404f64aCompareApril 9, 2020 07:16

@monocodusmonocodusBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is autogenerated code-style review, all good, approving

@marc-hb

Copy link
Copy Markdown
CollaboratorAuthor

The fix for the doxygen failure is waiting (more) reviews in PR #2746

@monocodusmonocodusBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is autogenerated code-style review, all good, approving

@marc-hbmarc-hb changed the title CI: Travis: regroup and remove redundant builds to speed things upCI: Travis: less VM isolation for speed-up. Remove matrix.Apr 9, 2020

@monocodusmonocodusBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is autogenerated code-style review, all good, approving

@marc-hb
marc-hb requested a review from lgirdwoodApril 9, 2020 07:20
@lgirdwood
lgirdwood merged commit d626291 into thesofproject:masterApr 9, 2020
@marc-hb
marc-hb deleted the smaller-travis-matrix branch April 9, 2020 17:00
Sign up for freeto 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

@marc-hb@lgirdwood