Skip to content

Improve checkout performance on Windows runners by upgrading @actions/github dependency - #1246

Merged
fhammerl merged 3 commits into
actions:mainfrom
BrettDong:octokit
Apr 12, 2023
Merged

Improve checkout performance on Windows runners by upgrading @actions/github dependency#1246
fhammerl merged 3 commits into
actions:mainfrom
BrettDong:octokit

Conversation

@BrettDong

Copy link
Copy Markdown
Contributor

Re: #1186

@dscho discovered that the checkout action could stall for a considerable amount of time on Windows runners waiting for PowerShell invocations made from 'windows-release' npm package to complete.

Then I studied the dependency chain to figure out where 'windows-release' was imported:

'@actions/checkout'
  <- '@actions/github'@2.2.0
    <- '@octokit/endpoint'@6.0.1
    <- '@octokit/graphql'@4.3.1
    <- '@octokit/request'@5.4.2
    <- '@octokit/rest'@16.43.1
      <- 'universal-user-agent'@4.0.1
        <- 'os-name'@3.1.0
          <- 'windows-release'@3.1.0

'universal-user-agent' package dropped its dependency on 'os-name' in v6 (https://github.com/gr2m/universal-user-agent/releases/tag/v6.0.0).

'@actions/github' v3 removed dependency on '@octokit/rest'@16.43.1 and allows users to move away from the old 'universal-user-agent' v4. (actions/toolkit#453)

This pull request attempts to update the version of '@actions/github' used in the checkout action to avoid importing 'windows-release'.

Based on testing in my own repositories, I can see an improvement in reduced wait time between entering the checkout action and git actually starts to do useful work.

@BrettDong
BrettDong requested a review from a team as a code owner March 26, 2023 11:30
dscho added a commit to dscho/git-for-windows-automation that referenced this pull request Mar 26, 2023
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

@dscho dscho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I confirm that this fixes the abysmal slowness on Windows that was reported in #1186 and that I was able to reproduce.

Well done, @BrettDong!

@briantist

Copy link
Copy Markdown

Eagerly awaiting a fix for this; for those of us paying for runners it's not just a convenience, we end up paying significantly more because the checkout step takes the majority of the runner's time, and with a workflow that has a lot of parallel runs, this adds up really quickly.

@briantist

Copy link
Copy Markdown

I've been using this and it was faster, but I just had a run with it that took 1m41s 😖

Comment thread src/github-api-helper.ts Outdated
@BrettDong

Copy link
Copy Markdown
Contributor Author

Hi @thboop , could you please re-approve the CI checks? Thanks!

@fhammerl fhammerl self-assigned this Apr 6, 2023
Comment thread src/github-api-helper.ts Outdated
@fhammerl

fhammerl commented Apr 6, 2023

Copy link
Copy Markdown
Contributor

Hi @BrettDong, thanks for this PR! Updating to actions/github@v5 is both something we want to do and it sidesteps the whole issue by eliminating our dependency on windows-release by removing universal-user-agent all together, so thanks for taking that approach :)

A few notes / questions:

  1. Do you have a minimal repro that only requires one of windows-release? E.g. I tried to run console.log(windowsRelease()) on a larger runner but it did not cause the same stalling behaviour. Running (Get-CimInstance -ClassName Win32_OperatingSystem).caption directly in powershell is also instantaneous
  2. I could only reproduce this issue on larger runners, not on windows-latest or windows-2019. Do you experience it there too?

@BrettDong

Copy link
Copy Markdown
Contributor Author

The delay from windows-release is only part of the reason checkout is slow on Windows. This pull request solves specifically this single issue. In majority of cases the delay of windows-release is of single digit seconds; in rarer cases it takes longer time. I don't know why the time is indeterministic. But at least this pull request completely eliminates the PowerShell invocation and saves at least a couple of seconds in each checkout run.

Apart from the windows-release problem discovered by @dscho, another source of stall I discovered is extremely slow disk I/O for loading NodeJS and Git executable and libraries: #1186 (comment). But I don't think it's something can be improved by external users like us.

…/github dependency

Re: #1186

@dscho discovered that the checkout action could stall for a
considerable amount of time on Windows runners waiting for PowerShell
invocations made from 'windows-release' npm package to complete.

Then I studied the dependency chain to figure out where
'windows-release' was imported:

'@actions/checkout'@main
  <- '@actions/github'@2.2.0
    <- '@octokit/endpoint'@6.0.1
    <- '@octokit/graphql'@4.3.1
    <- '@octokit/request'@5.4.2
    <- '@octokit/rest'@16.43.1
      <- 'universal-user-agent'@4.0.1
        <- 'os-name'@3.1.0
          <- 'windows-release'@3.1.0

'universal-user-agent' package dropped its dependency on 'os-name' in
https://github.com/gr2m/universal-user-agent/releases/tag/v6.0.0 .

'@actions/github' v3 removed dependency on '@octokit/rest'@16.43.1 and
allows users to move away from the old 'universal-user-agent' v4.
(actions/toolkit#453)

This pull request attempts to update the version of '@actions/github'
used in the checkout action to avoid importing 'windows-release'.

Based on testing in my own repositories, I can see an improvement in
reduced wait time between entering the checkout action and git actually
starts to do useful work.
@BrettDong

Copy link
Copy Markdown
Contributor Author

I could only reproduce this issue on larger runners, not on windows-latest or windows-2019. Do you experience it there too?

I never used large runners and I don't understand what issue you are referring to.

@thboop

thboop commented Apr 10, 2023

Copy link
Copy Markdown
Contributor

Hey @BrettDong , thanks for putting this together, I'd love to get this merged and released.

It looks like the breaking change in the new major version of actions/github is causing this new behavior to fail. Do you have some time to update this pr, or should we take it over?

@BrettDong

Copy link
Copy Markdown
Contributor Author

I did not run npm build last time and it might be the cause of test failures. @thboop: Could you approve CI checks?

Comment thread src/github-api-helper.ts

@fhammerl fhammerl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@fhammerl
fhammerl merged commit 40a16eb into actions:main Apr 12, 2023
@fhammerl

Copy link
Copy Markdown
Contributor

Thanks for your contribution @BrettDong

Small side-note, did you need to manually recognise the MIT license of .licenses/npm/@actions/http-client-1.0.8.dep.yml or did licensed work the first time?

@BrettDong

Copy link
Copy Markdown
Contributor Author

Thanks for your contribution @BrettDong

Small side-note, did you need to manually recognise the MIT license of .licenses/npm/@actions/http-client-1.0.8.dep.yml or did licensed work the first time?

I manually updated the yaml, following licensed documentation.

Comment thread src/github-api-helper.ts
ZauberNerd added a commit to ZauberNerd/checkout that referenced this pull request Apr 13, 2023
The PR actions#1246 replaced the `getOctokit` method from the
`octokit-provider.ts` file with the `getOctokit` method from the
`@actions/github` package.
The octokit-provider was previously responsible for creating an Octokit
instance and setting the `baseUrl` via the `getServerApiUrl` helper
function. This function calls `getServerUrl` which reads the server url
from the `GITHUB_SERVER_URL` environment variable, which on GHES is set
to the enterprise instance.
This commit restores the previous behaviour by calling `getServerApiUrl`
in all places where an octokit instance is created.

Co-authored-by: Markus Wolf <mail@markus-wolf.de>
@ZauberNerd

Copy link
Copy Markdown

This PR breaks GHES compatibility in case actions/checkout is used in an environment where no git binary exists. See also: #1288