Skip to content

Use fast D: drive if available #567

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jan 2, 2023
Merged

Use fast D: drive if available #567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66641f4
Use fast D: drive if available
dennisameling Dec 28, 2022
6f5c3c0
npm run build && npm run package
dscho Jan 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions dist/index.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map
View file
Open in desktop

Large diffs are not rendered by default.

25 changes: 23 additions & 2 deletions main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,24 @@ import * as fs from 'fs'
const flavor = core.getInput('flavor')
const architecture = core.getInput('architecture')

/**
* Some Azure VM types have a temporary disk which is local to the VM and therefore provides
* _much_ faster disk IO than the OS Disk (or any other attached disk).
*
* Hosted GitHub Actions runners also leverage this disk and do their work in D:/a/_work, so let's
* use it too if we can. It leads to a ~25% speed increase when doing heavy IO operations.
*
* https://learn.microsoft.com/en-us/azure/virtual-machines/managed-disks-overview#temporary-disk
*/
function getDriveLetterPrefix(): string {
if (fs.existsSync('D:/')) {
core.info('Found a fast, temporary disk on this VM (D:/). Will use that.')
return 'D:/'
}

return 'C:/'
}

Comment on lines +16 to +33
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could also leverage the $RUNNER_TEMP environment variable which resolves to D:\a\_temp for example. It depends on the directory where the Actions Runner is installed (e.g. D:\a or C:\a), meaning that we wouldn't have to use our own logic for this. WDYT? Any concerns about long path issues or something? I think D:\a\_temp\git-sdk-64-full vs D:\git-sdk-64-full isn't too much of a difference.

From the docs:

The path to a temporary directory on the runner. This directory is emptied at the beginning and end of each job. Note that files will not be removed if the runner's user account does not have permission to delete them. For example, D:\a\_temp

That should also allow us to get rid of our custom cleanup logic 👀🔥

Copy link
Member

Choose a reason for hiding this comment

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

My intuition is that using $RUNNER_TEMP could potentially cause problems, even inadvertent ones, because the TEMP part of the name suggests something ethereal, not-so-critical.

Long paths are not my concern here, either. But disk space is. I'd love to have the speed improvements, but we need to be sure that we're not accidentally running out of space. According to this documentation we have 14GB of SSD, which should be enough, even if the SDK weighs in with over 2GB.

Copy link
Member

Choose a reason for hiding this comment

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

I just checked and it seems that there is still a bit over 12GB free, so we're good to go!

async function run(): Promise<void> {
try {
if (process.platform !== 'win32') {
Expand All @@ -37,7 +55,8 @@ async function run(): Promise<void> {
architecture,
githubToken
)
const outputDirectory = core.getInput('path') || `C:/${artifactName}`
const outputDirectory =
core.getInput('path') || `${getDriveLetterPrefix()}${artifactName}`
let useCache: boolean
switch (core.getInput('cache')) {
case 'true':
Expand Down Expand Up @@ -153,7 +172,9 @@ function cleanup(): void {

const outputDirectory =
core.getInput('path') ||
`C:/${getArtifactMetadata(flavor, architecture).artifactName}`
`${getDriveLetterPrefix()}${
getArtifactMetadata(flavor, architecture).artifactName
}`

/**
* Shelling out to `rm -rf` is more than twice as fast as Node's `fs.rmSync` method.
Expand Down