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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: make CI faster, magic contained within #21086

Merged
merged 39 commits into from Nov 28, 2019
Merged

Conversation

@MarshallOfSound
Copy link
Member

MarshallOfSound commented Nov 12, 2019

So I tried a lot of things here, mostly based off of the idea @jkleinsc had around caching and restoring the out directory. On linux that works really well, on macOS the network speed is lower (appears to be only gigabit) so we have to make trade offs around out size vs cache upload / download time.

I think I found the sweet spot. Raising this PR for 馃憖 now, I just reset the cache keys so the first run on this PR will be the old slow path. The second run will be ultra-fast.

Build time estimates are:
Linux ~13 minutes from start to tests complete
macOS ~28 minutes from start to tests complete

Notes: no-notes

@MarshallOfSound MarshallOfSound changed the title Idk this was johns idea build: make CI faster, magic contained within Nov 12, 2019
@MarshallOfSound

This comment has been minimized.

Copy link
Member Author

MarshallOfSound commented Nov 12, 2019

Hm, looks like there's a double-build going on due to my size reduction logic. Will fix it up tomorrow.

鈥ore zipping
@MarshallOfSound MarshallOfSound force-pushed the idk-this-was-johns-idea branch from 6d81491 to fda4b25 Nov 27, 2019
@MarshallOfSound MarshallOfSound force-pushed the idk-this-was-johns-idea branch from 93548f0 to fa10ec4 Nov 27, 2019
@MarshallOfSound MarshallOfSound force-pushed the idk-this-was-johns-idea branch from fa10ec4 to 9f9895b Nov 27, 2019
@MarshallOfSound MarshallOfSound force-pushed the idk-this-was-johns-idea branch from 403cc09 to cdb575e Nov 27, 2019
@MarshallOfSound MarshallOfSound force-pushed the idk-this-was-johns-idea branch 3 times, most recently from b0d25e1 to 8781c70 Nov 27, 2019
@MarshallOfSound MarshallOfSound force-pushed the idk-this-was-johns-idea branch from 8781c70 to ad866c3 Nov 27, 2019
鈥eaders on macOS
@MarshallOfSound

This comment has been minimized.

Copy link
Member Author

MarshallOfSound commented Nov 27, 2019

Ok I got this working. I could explain how it works but I'm using a bunch of new tricks in here that would be quite a write up.

image

Copy link
Contributor

jkleinsc left a comment

LGTM. Excellent work @MarshallOfSound!!!!

@@ -684,10 +694,18 @@ step-maybe-restore-git-cache: &step-maybe-restore-git-cache
paths:
- ~/.gclient-cache
keys:
- v2-gclient-cache-{{ arch }}-{{ checksum "src/electron/.circle-sync-done" }}-{{ checksum "src/electron/DEPS" }}
- v2-gclient-cache-{{ arch }}-{{ checksum "src/electron/.circle-sync-done" }}
- v2-gclient-cache-{{ checksum "src/electron/.circle-sync-done" }}-{{ checksum "src/electron/DEPS" }}

This comment has been minimized.

Copy link
@nornagon

nornagon Nov 27, 2019

Member

why is the arch no longer part of the cache key?

This comment has been minimized.

Copy link
@MarshallOfSound

MarshallOfSound Nov 28, 2019

Author Member

Because it's just a git blob, not arch specific. Also we checkout macos on a linux machine anyway 馃し鈥嶁檪

"//tools/v8_context_snapshot:v8_context_snapshot_generator",
":licenses",
]
data_deps = mksnapshot_deps

This comment has been minimized.

Copy link
@nornagon

nornagon Nov 27, 2019

Member

maybe just make this depend on electron_mksnapshot?

This comment has been minimized.

Copy link
@MarshallOfSound

MarshallOfSound Nov 28, 2019

Author Member

I did that first, it was not impressed with the idea.

@MarshallOfSound MarshallOfSound merged commit 01f5e9c into master Nov 28, 2019
16 of 18 checks passed
16 of 18 checks passed
electron-arm-testing Build #20191127.12 failed
Details
electron-arm-testing #20191127.12 failed
Details
Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-woa-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm64-testing Build #20191127.12 succeeded
Details
electron-arm64-testing #20191127.12 succeeded
Details
electron-woa-testing Build #20191127.11 succeeded
Details
electron-woa-testing #20191127.11 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

release-clerk bot commented Nov 28, 2019

No Release Notes

@MarshallOfSound MarshallOfSound deleted the idk-this-was-johns-idea branch Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can鈥檛 perform that action at this time.