Improve Makefile #389

Open
opened 2023-05-25 09:38:17 +00:00 by fyrchik · 3 comments
Owner

Our current Makefile should be optimized for parallel execution. This means that building our project can take longer than necessary. In order to improve our build times, we need to update the Makefile to support parallel execution using the -j flag.

The updated Makefile should meet the following requirements:

  • All targets and their dependencies should be clearly defined and documented in the Makefile
  • All object and resulting binary files should be placed in a separate build directory
  • The Makefile should be able to handle changes in dependencies, and only rebuild the necessary targets
Our current Makefile should be optimized for parallel execution. This means that building our project can take longer than necessary. In order to improve our build times, we need to update the Makefile to support parallel execution using the -j flag. The updated Makefile should meet the following requirements: - All targets and their dependencies should be clearly defined and documented in the Makefile - All object and resulting binary files should be placed in a separate build directory - The Makefile should be able to handle changes in dependencies, and only rebuild the necessary targets
fyrchik added the
triage
label 2023-05-25 09:38:17 +00:00
fyrchik added this to the v0.38.0 milestone 2023-05-29 17:29:44 +00:00
fyrchik added the
good first issue
label 2023-05-29 17:30:47 +00:00
fyrchik modified the milestone from v0.38.0 to v0.39.0 2023-10-02 13:22:31 +00:00
fyrchik modified the milestone from v0.39.0 to v0.40.0 2024-05-14 14:15:58 +00:00
fyrchik modified the milestone from v0.40.0 to v0.41.0 2024-06-01 09:19:50 +00:00
fyrchik modified the milestone from v0.41.0 to v0.42.0 2024-06-14 07:07:51 +00:00
fyrchik modified the milestone from v0.42.0 to v0.43.0 2024-07-23 06:34:46 +00:00
Owner

TrueCloudLab/frostfs-dev-env#76 introduced new tag for images that uses git.frostfs.info/TrueCloudLab/frostfs as HUB_IMAGE, see packages.

However this repo does not contain corresponding changes in Makefile:

Line 7 in a4fb7f0
HUB_IMAGE ?= truecloudlab/frostfs

/cc @achuprov

https://git.frostfs.info/TrueCloudLab/frostfs-dev-env/pulls/76 introduced new tag for images that uses `git.frostfs.info/TrueCloudLab/frostfs` as `HUB_IMAGE`, see [packages](https://git.frostfs.info/TrueCloudLab/-/packages/container/frostfs-storage/v0.42.9). However this repo does not contain corresponding changes in Makefile: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/a4fb7f085b4cd2c5d7bb2ec91e6e626238dead54/Makefile#L7 /cc @achuprov
achuprov was assigned by realloc 2024-09-09 09:10:00 +00:00
potyarkin was assigned by realloc 2024-09-09 09:10:01 +00:00
achuprov removed their assignment 2024-09-10 14:59:21 +00:00
potyarkin was unassigned by achuprov 2024-09-10 14:59:22 +00:00
fyrchik modified the milestone from v0.43.0 to v0.44.0 2024-09-30 11:51:38 +00:00
potyarkin self-assigned this 2024-10-07 08:24:02 +00:00
Member

What are our goals here?

I see several possible avenues for improvement, but it does not look like current Makefile is causing a significant time waste:

  • With "cold" cache (clean cloud VM where no Go builds were ever executed; 2 vCPU, 8GB RAM) make all in this repo took 1m18s
  • Subsequent rebuild without any file modifications took 1.3s and was essentially a no-op
  • Rerunning make all after adding a couple of newlines to each *.go file took 18s: caches provided by go tool seem to work as expected
  • Rebuilding all binaries after rm bin/* took 6s, again thanks to go tool
  • Using make -j would not improve our explicitly sequential CI builds

So I don't think that parallelizing the build should be our top priority.

Instead I think we should focus on:

Improving CI run times

  • Caching $GOPATH/pkg/mod should shave off 10s+ of each build, maybe more
  • Dropping "Set up Go" steps and using official golang Docker images.
    Numbers don't look impressive (this step is attributed to only 3-5s of CI time in logs), but my intuition says this may shave off much more than that from the "Set up job" stage. Need to confirm this intuition with tests, of course.

Improving Makefile maintainability

  • Splitting do-it-all Makefile into reusable chunks (as suggested by @fyrchik).
    This would allow us to sync common *.mk files between TrueCloudLab repos and to distribute Makefile updates to all repos in a sane manner
  • Fixing Makefile target discoverability and docstrings while we're at it
  • I would drop .ONESHELL or at least introduce .SHELLFLAGS = -euo pipefail -c to mitigate the worst of ONESHELL side effects. Without this we can never be sure that exit code 0 from make means that there were no errors

I would also like to ask for clarification regarding the point from OP:

All object and resulting binary files should be placed in a separate build directory

Do you mean separate as in separate from source code, like we have bin/$EXE now? Or separate as in separate from all previous builds, like bin/${GIT_REVISION}+${HASH_OF_WORKTREE_CHANGES}/$EXE?

What are our goals here? I see several possible avenues for improvement, but it does not look like current Makefile is causing a significant time waste: - With "cold" cache (clean cloud VM where no Go builds were ever executed; 2 vCPU, 8GB RAM) `make all` in this repo took 1m18s - Subsequent rebuild without any file modifications took 1.3s and was essentially a no-op - Rerunning `make all` after adding a couple of newlines to each `*.go` file took 18s: caches provided by go tool seem to work as expected - Rebuilding all binaries after `rm bin/*` took 6s, again thanks to go tool - Using `make -j` would not improve our explicitly sequential [CI builds][sequential] [sequential]: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/dfb00083d07499a0e3d89076cc3f08729c00cb71/.forgejo/workflows/build.yml#L25-L41 So I don't think that parallelizing the build should be our top priority. Instead I think we should focus on: #### Improving CI run times - [ ] Caching `$GOPATH/pkg/mod` should shave off 10s+ of each build, maybe more - [ ] Dropping "Set up Go" steps and using official [golang Docker images][DockerHub]. Numbers don't look impressive (this step is attributed to only 3-5s of CI time in logs), but my intuition says this may shave off much more than that from the "Set up job" stage. Need to confirm this intuition with tests, of course. [DockerHub]: https://hub.docker.com/_/golang #### Improving Makefile maintainability - [ ] Splitting do-it-all Makefile into reusable chunks (as suggested by @fyrchik). This would allow us to sync common `*.mk` files between TrueCloudLab repos and to distribute Makefile updates to all repos in a sane manner - [ ] Fixing Makefile target discoverability and docstrings while we're at it - [ ] I would drop `.ONESHELL` or at least introduce `.SHELLFLAGS = -euo pipefail -c` to mitigate the worst of ONESHELL side effects. Without this we can never be sure that exit code 0 from make means that there were no errors --- I would also like to ask for clarification regarding the point from OP: > All object and resulting binary files should be placed in a separate build directory Do you mean separate as in separate from source code, like we have `bin/$EXE` now? Or separate as in separate from all previous builds, like `bin/${GIT_REVISION}+${HASH_OF_WORKTREE_CHANGES}/$EXE`?
Author
Owner

My main goal is pure aesthetics and conforming to community standards (needs to be defined).
The second one is readability and maintainability.

Regarding CI -- optimisations are always welcome.

My main goal is pure aesthetics and conforming to community standards (needs to be defined). The second one is readability and maintainability. Regarding CI -- optimisations are always welcome.
fyrchik modified the milestone from v0.44.0 to v0.45.0 2024-11-25 10:46:51 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#389
No description provided.