-
Notifications
You must be signed in to change notification settings - Fork 801
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 CI Build #996
Speed up CI Build #996
Conversation
markmandel
commented
Aug 13, 2019
•
edited
Loading
edited
- Run more things in parallel.
- Cache C++ build output, as that is the longer path.
Build Succeeded 👏 Build Id: 2fe02d02-92ed-479e-85ea-70725daac70e The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
cloudbuild.yaml
Outdated
- name: "make-docker" | ||
id: push-images | ||
waitFor: | ||
- build | ||
- build-images | ||
dir: "build" | ||
args: [ "-j", "3", "push"] |
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.
We build 4 images now, so this can be -j 4
.
First pass seemed to scrape off 3 minutes. Not as much as I would like. |
@googlebot rescan |
1d1dd4d
to
eb92bdf
Compare
Build Succeeded 👏 Build Id: 7af13475-17bb-403c-9826-f24aa1d7e186 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
- Run more things in parallel. - Cache C++ build output, as that is the longer path.
eb92bdf
to
9e658c6
Compare
Build Succeeded 👏 Build Id: 8662c7b1-7c90-464e-8cb2-c6a20a4f91fa The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
That's better - a 14 minute build. |
Cool gain in execution time. Line 140 in c5fa6ef
You could see from logs that Step 15 |
Why does it need to be removed? Could we not gitignore it instead? |
we could, I need to recap pros and cons as well as reason behind adding clean step for Node JS. |
Looking at the code - it's already .gitignored. So I can remove the clean step. Ran it locally, and git did not complain about anything not being added 👍 |
Build Succeeded 👏 Build Id: 96ff6b54-bcd0-41e9-abb7-a412b0dc0a01 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
One thing I don't understand. I upped the e2e test parrallel count to 64 (from 32), no increase in performance. Seems odd. |
Build Succeeded 👏 Build Id: 7720793f-ba41-41dd-b787-6a2971a3c797 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Just removed 4 minutes from the e2e test! 13 minute run! |
@@ -56,7 +56,10 @@ func TestFleetScaleUpEditAndScaleDown(t *testing.T) { | |||
fixtures := []bool{true, false} | |||
|
|||
for _, usePatch := range fixtures { | |||
usePatch := usePatch |
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.
I figure someone will ask about this:
https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721
@@ -137,7 +137,6 @@ run-sdk-conformance-no-build: ensure-build-sdk-image | |||
run-sdk-conformance-test: | |||
$(MAKE) run-sdk-command COMMAND=build-sdk-test | |||
$(MAKE) run-sdk-conformance-no-build | |||
$(MAKE) run-sdk-command COMMAND=clean |
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.
Note that there is also a clean step in Rust. Will check if Rust package files are gitignored.
I suggest adding a new make target for clean. Otherwise it would be not visible from sdk.mk
that we have a special way to remove package files.
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.
Rust target dir have appropriate .gitignore. https://github.com/googleforgames/agones/blob/master/test/sdk/rust/.gitignore
So the only request for adding new target for clean left.
Could be:
clean-sdk-conformance-tests:
$(MAKE) run-all-sdk-command COMMAND=clean
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.
Created a PR linked to this.
Build Succeeded 👏 Build Id: cac641e8-737e-48b4-b702-79b23f9d1e31 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|