Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Streamline CI setup, and reenable macOS credits #3697

Merged
merged 8 commits into from
Mar 10, 2021
Merged

Streamline CI setup, and reenable macOS credits #3697

merged 8 commits into from
Mar 10, 2021

Conversation

stuartmorgan
Copy link
Contributor

@stuartmorgan stuartmorgan commented Mar 10, 2021 •
edited
Loading

This makes a few improvements to CI throughput:

  • Moves the flutter channel+upgrade step into each step, rather then doing it for both channels in the setup step. This avoids pointlessly doing twice the work in every setup (downloading two channels, only one of which will be used)
  • Re-enables credits for macOS tasks. These were disabled in a recent chance since I was unifying configurations, but it turns out that without credits there is no parallelization of macOS tasks, which makes the end-to-end time very long.
  • Dials back the Linux VM requirements based on looking at the new CPU and memory usage graphs for runs:
    • No task was ever reaching 4 CPUs or 12 GB of memory, so use those values for heavy workload; this will double concurrency of the heavy-workload tasks and should have no impact on run length.
    • Linux build+drive was extremely low usage, so move it to the light workload group.

This will increase credit usage relative to the last week or so, but not relative to most of the recent repository history.

Pre-launch Checklist

  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

@google-cla google-cla bot added the cla: yes label Mar 10, 2021
@stuartmorgan
Copy link
Contributor Author

@jmagman FYI, since this is relevant to compute credit discussions.

@jmagman
Copy link
Member

jmagman commented Mar 10, 2021 •
edited
Loading

Moves the flutter channel+upgrade step into each step, rather then doing it for both channels in the setup step. This avoids pointlessly doing twice the work in every setup (downloading two channels, only one of which will be used)

I also remember thinking this was pointless in 2019:
https://github.com/jmagman/plugins/blob/pod/.cirrus.yml
e9cd4ef
54e4bbe
693b1bf

A bunch of stuff failed then, though I can't remember the details at all. And there's no info in the PRs that introduced this.
#2211

If it works it makes sense though. Sorry I don't remember more.

jmagman
jmagman reviewed Mar 10, 2021
View reviewed changes
script/set_channel.sh Outdated Show resolved Hide resolved
.cirrus.yml Outdated Show resolved Hide resolved
@stuartmorgan
Copy link
Contributor Author

@jmagman Thanks for the Cirrus-fu pointers! This is much cleaner, and seem to be working as intended.

jmagman
jmagman approved these changes Mar 10, 2021
View reviewed changes
Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM if removing the extra channel change works.

.cirrus.yml Outdated
build_script:
- flutter channel $CHANNEL
- ./script/incremental_build.sh build-examples --linux
- xvfb-run ./script/incremental_build.sh drive-examples --linux

task:
# Xcode 12 task
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind removing this comment while you're here? I should have removed it with #3653.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

.cirrus.yml
Comment on lines +137 to +138
cpu: 4
memory: 12G
Copy link
Member

Choose a reason for hiding this comment

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

Might have to play with this if you start seeing OOM failures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I figured this was safe since I wasn't seeing it going above 10 in general, but we can certainly adjust it back up if necessary.

@stuartmorgan stuartmorgan merged commit 79dd06a into flutter:master Mar 10, 2021
@stuartmorgan stuartmorgan deleted the ci-streamlining branch March 10, 2021 20:51
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 10, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 10, 2021
@engine-flutter-autoroll
79dd06a Streamline CI setup, and reenable macOS credits (flutter/plug…
df23fac 
…ins#3697)
@jmagman jmagman mentioned this pull request Mar 10, 2021
Merged
11 tasks
yasargil added a commit to yasargil/plugins that referenced this pull request Mar 18, 2021
@yasargil
Merge branch 'master' into gmain
f6f7fdd 
* master: (49 commits)
  Prep for alignment with Flutter analysis options (flutter#3703)
  [google_maps_flutter_platform_interface] Mark constructors as const for ids (flutter#3718)
  [image_picker] Endorse image_picker_for_web (flutter#3717)
  Add missing licenses, and add a check (flutter#3720)
  [ci] Add libgcrypt to Docker image. (flutter#3711)
  Reorder the checkboxes in the PR template (flutter#3666)
  Re-endorse connectivity_for_web (flutter#3708)
  Fix missing declaration of windows' default_package (flutter#3705)
  Typos in comments (flutter#2846)
  Skip flutter upgrade for pod linting Cirrus task (flutter#3700)
  [cross_file] Delete. (flutter#3698)
  [tool] Improve check version ci so that it enforces the version in CHANGELOG and pubspec matches. (flutter#3678)
  Streamline CI setup, and reenable macOS credits (flutter#3697)
  [video_player] fixed misleading size and aspect ratio documentation (flutter#3668)
  [image_picker] Implemented 2860 and added Unit Test to test functionality (flutter#3685)
  [shared_preferences] Fix concurrent modification of the shared preferences on Android (flutter#3684)
  [extension_google_sign_in_as_googleapis_auth] Deleted. (flutter#3694)
  Skip pod lint tests (flutter#3692)
  [Video_Player] Remove the deprecated API reference. (flutter#3669)
  [google_sign_in] fix test(flutter#3690)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jmagman jmagman jmagman approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stuartmorgan @jmagman