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.

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

Jump to bottom

added incremental build option to Makefile to speed up rebuilds #454

Merged
merged 1 commit into from
Dec 24, 2018
Merged

added incremental build option to Makefile to speed up rebuilds #454

merged 1 commit into from
Dec 24, 2018

Conversation

jkowalski
Copy link
Contributor

@jkowalski jkowalski commented Dec 21, 2018 •
edited
Loading

This partially addresses #453.

Before:

$ time make -C build -j4 build-images
real    2m33.772s
user    0m1.302s
sys     0m0.889s

Incremental re-builds:

$ time make -C build -j4 build-controller-image
real    0m1.837s
user    0m0.241s
sys     0m0.085s

$ time make -C build -j4 build-agones-sdk-image
real    0m5.352s
user    0m0.447s
sys     0m0.244s

$ time make -C build -j4 build-ping-image
real    0m1.318s
user    0m0.198s
sys     0m0.063s

$ time make -C build -j4 build-images INCREMENTAL_BUILD=1
real    0m5.506s
user    0m1.177s
sys     0m0.617s

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: fe9b74ed-408a-4662-a4fc-4ec70184d4c7

The following development artifacts have been built, and will exist for the next 30 days:

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/454/head:pr_454 && git checkout pr_454
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-9b7d27e

markmandel
markmandel reviewed Dec 21, 2018
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Nice improvements! Couple of thought below 🐇

build/Makefile Outdated
@@ -101,6 +106,13 @@ ifdef DOCKER_RUN
ensure-build-image += ensure-build-image
endif

ifdef INCREMENTAL_BUILD
Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking that this should be defaulted on - and then turned off for builds in do-release. I'm not seeing any downsides. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched default to incremental.

build/Makefile Outdated
@@ -101,6 +106,13 @@ ifdef DOCKER_RUN
ensure-build-image += ensure-build-image
endif

ifdef INCREMENTAL_BUILD
# keep a cache of files built by Go across docker invocations in a local directory.
common_mounts += -v $(CURDIR)/.gocache:/root/.cache/go-build
Copy link
Member

Choose a reason for hiding this comment

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

Rather than use CURDIR, which is the working directory, let's explicitly put it in the $(build_path), then it never moves, regardless of where people call the Makefile from.

Suggested change
common_mounts += -v $(CURDIR)/.gocache:/root/.cache/go-build
common_mounts += -v $(build_path)/.gocache:/root/.cache/go-build

Then we can also add a mkdir -p to the ensure-build-config target, so that the directory always is created before being mounted (I thought it should throw an error if it's not there - has that stopped happening?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Docker appears to be creating the directory automatically.

@jkowalski jkowalski changed the title added INCREMENTAL_BUILD option to Makefile to speed up rebuilds added incremental build option to Makefile to speed up rebuilds Dec 21, 2018
@jkowalski
added incremental build to Makefile to speed up rebuilds
5f307e7 
(used everywhere by default except in do-release target)

Before:
=======
$ time make -C build -j4 build-images
real    2m33.772s
user    0m1.302s
sys     0m0.889s

Incremental re-builds:
=======
$ time make -C build -j4 build-controller-image
real    0m1.837s
user    0m0.241s
sys     0m0.085s

$ time make -C build -j4 build-agones-sdk-image
real    0m5.352s
user    0m0.447s
sys     0m0.244s

$ time make -C build -j4 build-ping-image
real    0m1.318s
user    0m0.198s
sys     0m0.063s

$ time make -C build -j4 build-images
real    0m5.506s
user    0m1.177s
sys     0m0.617s
@agones-bot
Copy link
Collaborator

Build Failed 😱

Build Id: 9560ca9e-4cb1-4de6-8e86-d9d4e2c2d25a

Build Logs 
starting build "9560ca9e-4cb1-4de6-8e86-d9d4e2c2d25a"

FETCHSOURCE
Initialized empty Git repository in /workspace/.git/
fatal: remote error: want 4d2c8bcc9422cc89089976d18fb676e1a21c6be6 not valid
fatal: the remote end hung up unexpectedly
Reinitialized existing Git repository in /workspace/.git/
fatal: remote origin already exists.
fatal: remote error: want 4d2c8bcc9422cc89089976d18fb676e1a21c6be6 not valid
fatal: the remote end hung up unexpectedly
Reinitialized existing Git repository in /workspace/.git/
fatal: remote origin already exists.
fatal: remote error: want 4d2c8bcc9422cc89089976d18fb676e1a21c6be6 not valid
fatal: the remote end hung up unexpectedly
ERROR
ERROR: fetching git source: Failure fetching git repo: exit status 128

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 31b4fd7a-1d5f-4cff-9548-548d2f6479a9

The following development artifacts have been built, and will exist for the next 30 days:

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/454/head:pr_454 && git checkout pr_454
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-f1627a3

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 2bc5c718-7a3f-4da3-87b9-464a3a2b6ba6

The following development artifacts have been built, and will exist for the next 30 days:

To install this version:

  • git fetch https://github.com/GoogleCloudPlatform/agones.git pull/454/head:pr_454 && git checkout pr_454
  • helm install install/helm/agones --namespace agones-system --name agones --set agones.image.tag=0.7.0-5f307e7

@markmandel
Copy link
Member

🔥 WOW THAT IS FAST NOW 🔥

markmandel
markmandel approved these changes Dec 24, 2018
View reviewed changes
Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

we should have done this eons ago!

@markmandel markmandel merged commit 3ebf6c1 into googleforgames:master Dec 24, 2018
@markmandel markmandel added kind/feature New features for Agones area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. labels Dec 24, 2018
@markmandel markmandel added this to the 0.7.0 milestone Dec 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markmandel markmandel markmandel approved these changes

Assignees
No one assigned
Labels
area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. kind/feature New features for Agones
Projects
None yet
Milestone
0.7.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jkowalski @agones-bot @markmandel