-
Notifications
You must be signed in to change notification settings - Fork 29.3k
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
Speed up mac builds #116984
Speed up mac builds #116984
Conversation
d403e01
to
d60cba9
Compare
AZURE_STORAGE_ACCESS_KEY="$(ticino-storage-key)" \ | ||
AZURE_STORAGE_ACCESS_KEY_2="$(vscode-storage-key)" \ | ||
node build/azure-pipelines/common/createAsset.js \ | ||
"darwin-$(VSCODE_ARCH)" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For x64 it is just darwin
due to historical reason, changing this key will not push out this update.
x64 - darwin
arm64 - darwin-arm64
universal - darwin-universal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added case statement
656e92e
to
c4c515d
Compare
So the most recent run of this is still going and it's already at 1hr 30min... which means there's like 30min of variance in this build due to:
I feel like based on my testing that this one, on average, will take less time than @deepak1556's but it won't always be the case... so really I think the PR that should be accepted should be the more readable one. But maybe @joaomoreno has other opinions. |
condition: and(succeeded(), ne(variables.NODE_MODULES_RESTORED, 'true'), eq(variables['ENABLE_TERRAPIN'], 'true')) | ||
displayName: Switch to Terrapin packages | ||
|
||
- script: | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that signing step does not require any fancy node script except for createAsset
, can you remove the node module cache steps and simplify the yarn step to just compile src/build
packages.
$ cd build
$ yarn && yarn compile
Trimming down the yarn step in the signing stage should reduce the timing a bit, lets see how much that improves for a final number comparison. |
Last 2 successful runs has taken 1hr 5min. |
9b14eae
to
5b418bc
Compare
749fc40
to
ded050f
Compare
ded050f
to
b744827
Compare
🥳 |
Fixes #115263
This has some minor improvements over #116739
that job took 1hr 13min: https://dev.azure.com/monacotools/Monaco/_build/results?buildId=105100&view=results
this change took 54min*: https://dev.azure.com/monacotools/Monaco/_build/results?buildId=105610&view=results
*EDIT: Shoot... I just realized that @deepak1556's change ran the tests and mine didn't... that adds probably another 10min to mine... (so 1hr 4min)
Basically the refactor is to split up signing and build jobs.
Now it works like this:
x64 build
arm build
universal build (waits on x64 and arm)
x64 publish (waits on x64 build)
arm publish (waits on arm build)
universal publish (waits on universal build)
Then we can be sure that the signing is done in parallel. For example, the arm build finishes in 10 min - that can go right to signing when it's done rather than waiting for the universal build to sign everything.