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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃彈 Matrixify Experiment Build/Tests on CircleCI #32932

Merged
merged 2 commits into from Feb 26, 2021

Conversation

danielrozenberg
Copy link
Member

@danielrozenberg danielrozenberg commented Feb 25, 2021

This PR does DRY

Description from previous version of this PR that included splitting integration and e2e tests into separate jobs:

Reduces total running time of CircleCI for PRs from ~20.5 minutes (example) to ~18.5 minutes (example), at the cost of about an extra ~8.5 minutes of total VM time. Not sure if it's worth it, wdyt?

@danielrozenberg danielrozenberg marked this pull request as ready for review February 25, 2021 22:13
@amp-owners-bot
Copy link

amp-owners-bot bot commented Feb 25, 2021

Hey @rsimha! These files were changed:

.circleci/config.yml

@rsimha
Copy link
Contributor

rsimha commented Feb 25, 2021

Nice exploration! Unfortunately the additional cost probably makes this not worth it.

A few reasons:

  • CircleCI gives our entire org a max of 80 parallel VMs at any given time. If demand exceeds that, queueing will ensue.
  • Each extra VM we use incurs a $$ cost for CPU minutes spent during start up (~2 mins per new VM, as things stand today)
  • In just the past 24h, our peak parallelization has risen from 9 to 11, and this PR will make it 14. That means fewer people will be able to run builds at a time.

image

Alternative challenge: If we can bring down the running time of package installs from ~1 min to a few seconds (I'm working on this), gulp dist from ~8 mins to ~2 mins (@samouri is doing this), and gulp e2e from ~10 mins to ~5 mins (needs a volunteer), we can get down to sub ~15 minute builds without any increase in VM consumption.

@danielrozenberg
Copy link
Member Author

ok! So I'll revert the 2nd commit for this PR and keep it around in case we improve the overhead enough to make it worth it

@danielrozenberg danielrozenberg changed the title 馃彈 Matrixify Experiment Build/Tests on CircleCI and split integration/e2e tests into separate jobs 馃彈 Matrixify Experiment Build/Tests on CircleCI Feb 26, 2021
@danielrozenberg
Copy link
Member Author

Done, PTAL

Copy link
Contributor

@rsimha rsimha left a comment

Choose a reason for hiding this comment

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

馃憦馃憦馃憦 LGTM! (I'm kind of disappointed in myself that I didn't do it this way to begin with.)

@danielrozenberg
Copy link
Member Author

LGTM! (I'm kind of disappointed in myself that I didn't do it this way to begin with.)

Hey, you switched an entire CI system... figuring out the subtleties comes later ;)

@danielrozenberg danielrozenberg merged commit 2b9b5ee into ampproject:master Feb 26, 2021
@danielrozenberg danielrozenberg deleted the e2e-split branch February 26, 2021 18:49
Copy link
Member

@samouri samouri left a comment

Choose a reason for hiding this comment

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

TIL You could do this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants