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: use RAM disk for Windows tests #23327

Merged
merged 4 commits into from
Jun 13, 2022

Conversation

alan-agius4
Copy link
Collaborator

@alan-agius4 alan-agius4 commented Jun 9, 2022

Windows is IO bound which causes tests to take significantly longer than Linux.

With this change we introduce the use of RAM disk on Windows instead of a physical disk to store tests projects files which reduces this overhead.

While Circle CI docs mention to use ImDisk https://support.circleci.com/hc/en-us/articles/4411520952091-Create-a-windows-RAM-disk, we use a different tool Arsenal Image Mounter aim_ll due to following Node.js issue: nodejs/node#6861

The aim_ll CLI is based on ImDisk and is parameter compatible.

From testing a full run of the Windows E2E full test suit goes from ~56min to ~35mins

With RAM disk
https://app.circleci.com/pipelines/github/angular/angular-cli/23286/workflows/4b1dc425-f7ed-49d6-aeba-b2c503d08756/jobs/309423/parallel-runs/2?filterBy=ALL

Without RAM Disk
https://app.circleci.com/pipelines/github/angular/angular-cli/23256/workflows/3f551c3c-fbb0-445f-80f9-1801d4adc664/jobs/309312

@alan-agius4 alan-agius4 force-pushed the ci-win-ram-disk branch 19 times, most recently from ab09107 to 5b2f159 Compare June 9, 2022 12:57
@alan-agius4 alan-agius4 added the target: minor This PR is targeted for the next minor release label Jun 9, 2022
@alan-agius4 alan-agius4 added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jun 9, 2022
@alan-agius4 alan-agius4 changed the title ci: test win ram disk ci: use RAM disk for Windows tests Jun 9, 2022
@alan-agius4 alan-agius4 requested a review from clydin June 9, 2022 12:59
@alan-agius4 alan-agius4 marked this pull request as ready for review June 9, 2022 12:59
@alan-agius4 alan-agius4 force-pushed the ci-win-ram-disk branch 3 times, most recently from ba0ca87 to 883ce7f Compare June 9, 2022 13:36
Windows is IO bound which causes tests to take significantly longer than Linux.

With this change we introduce the use of RAM disk on Windows instead of a physical disk to store tests projects files which reduces this overhead.

While Circle CI docs mention to use `ImDisk` https://support.circleci.com/hc/en-us/articles/4411520952091-Create-a-windows-RAM-disk, we use a different tool Arsenal Image Mounter `aim_ll` due to following Node.js issue: nodejs/node#6861

The `aim_ll` CLI is based on `ImDisk` and is parameter compatible.

From testing a full run of the Windows E2E full test suit goes from ~56min to ~35mins

With RAM disk
https://app.circleci.com/pipelines/github/angular/angular-cli/23286/workflows/4b1dc425-f7ed-49d6-aeba-b2c503d08756/jobs/309423/parallel-runs/2?filterBy=ALL

Without RAM Disk
https://app.circleci.com/pipelines/github/angular/angular-cli/23256/workflows/3f551c3c-fbb0-445f-80f9-1801d4adc664/jobs/309312
When using ramdisk we now get different errors some times

```
ERROR: The process with PID 3804 (child process of PID 7516) could not be terminated.
Reason: The operation attempted is not supported.
```
@@ -83,6 +83,25 @@ commands:
at: *workspace_location
setup_windows:
steps:
- run:
Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we could put these into a cache so that we don't have to reset this up on every run?

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Jun 9, 2022

Choose a reason for hiding this comment

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

You mean the Arsenal installation files and drivers? This step is pretty fast, it takes around 12s-16s

But overall, I think we should "cache" some steps, including the yarn and node installation. While we can probably cache the files in the CI cache. I think it might be better to create a custom docker image that extends the one provided by circle.

We already have a dockerhub account which is owned by Alex Eagle.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah a custom docker image might be a better way to go about it.

I am fine with whatever you would like to do, I just wanted to make sure we were thinking about it at least.

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Jun 12, 2022

Choose a reason for hiding this comment

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

I'd prefer if we land this as is and do the docker image in a separate PR. As we still need to figure out the process for pushing an image, create credentials for a dockerhub account etc...

Copy link
Collaborator Author

@alan-agius4 alan-agius4 Jun 13, 2022

Choose a reason for hiding this comment

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

Actually @clydin correctly pointed out that on Windows they don't use docker. Ended up using Circle CI file cache
cc90eba

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good!

@@ -83,6 +83,25 @@ commands:
at: *workspace_location
setup_windows:
steps:
- run:
Copy link
Member

Choose a reason for hiding this comment

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

Yeah a custom docker image might be a better way to go about it.

I am fine with whatever you would like to do, I just wanted to make sure we were thinking about it at least.

@alan-agius4 alan-agius4 force-pushed the ci-win-ram-disk branch 3 times, most recently from 40cb3c3 to f144306 Compare June 13, 2022 13:19
This commits adds a content hash match for the Arsenal Image Mounter files to validate that the content of the files we download are as expected.
@alan-agius4 alan-agius4 force-pushed the ci-win-ram-disk branch 3 times, most recently from 7293ccf to 595cefe Compare June 13, 2022 13:44
This commits add logic to cache files used to create a Windows RAM disk to avoid extra HTTP requests.
@alan-agius4 alan-agius4 added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jun 13, 2022
@alan-agius4 alan-agius4 merged commit 14929e2 into angular:main Jun 13, 2022
@alan-agius4 alan-agius4 deleted the ci-win-ram-disk branch June 13, 2022 14:20
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants