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

ci: run zipapp tests on M1 macOS #13130

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

ichard26
Copy link
Member

@ichard26 ichard26 commented Dec 26, 2024

The macos-latest runner is significantly faster than even the ubuntu-latest runners (11 minutes vs 17 minutes).

Once #13129 is merged, the zipapp job will be one of the slowest jobs, keeping CI runs at 18+ minutes. With a 11 minute zipapp job, the slowest jobs will be the Intel macos-13 jobs. With just those remaining, we should have 151 minute CI ✨

image

Footnotes

  1. *although all of the intel macOS runners experience a LOT of run to run variation, so whether a PR takes 15 minutes to pass CI is up to luck

@ichard26 ichard26 added the skip news Does not need a NEWS file entry (eg: trivial changes) label Dec 26, 2024
Copy link
Member

@notatallshaw notatallshaw left a comment

Choose a reason for hiding this comment

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

Perhaps leave a comment on why it's running on the macos-runner in case in the far future there's a different faster runner it could be moved to?

@ichard26
Copy link
Member Author

ichard26 commented Dec 26, 2024

And for good measure, here are all of the jobs:

                    Last 50 CI runs on main                    
                                                               
  Job                            Mean    Minimum   Stdev       
 ───────────────────────────────────────────────────────────── 
  tests / 3.10 / macos-latest    6:25    5:44      0:34 (8%)   
  tests / 3.11 / macos-latest    6:40    6:03      0:39 (9%)   
  tests / 3.9 / macos-latest     6:57    6:03      0:55 (13%)  
  tests / 3.13 / macos-latest    6:59    6:15      0:37 (8%)   
  tests / 3.12 / macos-latest    7:09    6:14      1:01 (14%)  
  tests / 3.8 / macos-latest     7:11    6:32      0:31 (7%)   
  tests / 3.11 / ubuntu-latest   9:33    9:08      0:10 (1%)   
  tests / 3.10 / ubuntu-latest   10:04   9:43      0:14 (2%)   
  tests / 3.12 / ubuntu-latest   10:34   10:16     0:14 (2%)   
  tests / 3.13 / ubuntu-latest   10:35   10:10     0:12 (2%)   
  tests / 3.13 / Windows / 2     10:39   9:55      0:20 (3%)   
  tests / 3.8 / ubuntu-latest    11:02   10:41     0:14 (2%)   
  tests / 3.13 / Windows / 1     11:13   10:29     0:24 (3%)   
  tests / 3.9 / ubuntu-latest    11:44   11:20     0:19 (2%)   
  tests / 3.10 / macos-13        12:10   8:53      2:16 (18%)  
  tests / 3.11 / macos-13        12:59   9:09      2:34 (19%)  
  tests / 3.13 / macos-13        12:59   9:54      2:41 (20%)  
  tests / 3.9 / macos-13         13:10   10:09     2:42 (20%)  
  tests / 3.8 / macos-13         13:34   10:09     2:30 (18%)  
  tests / 3.12 / macos-13        13:55   9:54      3:23 (24%)  
  tests / 3.8 / Windows / 2      14:58   14:04     0:26 (2%)   
  tests / zipapp                 16:53   16:27     0:15 (1%)   
  tests / 3.8 / Windows / 1      17:37   16:40     0:30 (2%)

This PR addresses the zipapp job, and #13129 will improve the Windows' job performance. Thus, leaving the Intel macOS runners as our bottleneck.

The macos-latest runner is significantly faster than even the
ubuntu-latest runners (11 minutes vs 17 minutes). Once the Windows jobs
are made faster in a separate commit, we should have (on average) 15
minute CI. ✨
@ichard26
Copy link
Member Author

Perhaps leave a comment on why it's running on the macos-runner in case in the far future there's a different faster runner it could be moved to?

Good idea! I force pushed to add a comment explaining our choice of runner.

@ichard26
Copy link
Member Author

@pfmoore, as the author of the zipapp implementation, do you have any concerns with moving the zipapp tests to macOS?

@pfmoore
Copy link
Member

pfmoore commented Dec 27, 2024

No technical issues, although I agree with @notatallshaw that we should document the logic that determines where we choose to run them (which you have done). One reservation I have is that I thought there were limitations on the amount of MacOS resource projects were allowed to use - or is our entitlement such that this doesn't apply (the limitation I was aware of was on free-tier projects)?

But +1 from me.

@ichard26
Copy link
Member Author

One reservation I have is that I thought there were limitations on the amount of MacOS resource projects were allowed to use - or is our entitlement such that this doesn't apply (the limitation I was aware of was on free-tier projects)?

We can have at most 50 concurrent macOS runners with the plan we're on, we should be fine :)

@ichard26 ichard26 requested a review from pradyunsg December 27, 2024 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news Does not need a NEWS file entry (eg: trivial changes)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants