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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[currencyservice] Create multiple build jobs to optimize build time #569

Merged
merged 3 commits into from
Nov 11, 2022

Conversation

mviitane
Copy link
Member

@mviitane mviitane commented Nov 11, 2022

Changes

Create multiple build jobs to run the currency service build in parallel and optimize build time.

Different commands are needed to find the number of available CPU cores per OS.
make -j$(nproc || sysctl -n hw.ncpu || echo 1)

  • Uses nproc on Linux
  • Uses sysctl -n hw.ncpu on macOS
  • Uses 1 core for others

Background

Currency service had the longest build time among the Demo App services.

Rough build time improvements with this change:

  • Linux, docker (Ryzen 5 5600G): 1000 s -> 400 s.
  • macOS, docker desktop (M1 Pro): 750 s -> 450 s.

Measured with:
docker compose build currencyservice --no-cache

  • Appropriate CHANGELOG.md updated for non-trivial changes

Create multiple build jobs to run the build in parallel and optimize build time. Different commands are needed to find the number of available CPU cores per OS.

make -j$(nproc || sysctl -n hw.ncpu || echo 1)
- Uses nproc on Linux
- Uses sysctl -n hw.ncpu on macOS
- Uses 1 core for others

Currency service had the longest build times among the Demo App services.

Rough build time improvements with this change:
Linux, docker (Ryzen 5 5600G): 1000 s -> 400 s.
macOS, docker desktop (M1 Pro): 750 s -> 450 s.
@mviitane mviitane requested a review from a team November 11, 2022 09:49
Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

Awesome!

docker compose build currencyservice ||| Building 553.7s (14/14) FINISHED

@cartersocha
Copy link
Contributor

No real change on my personal machine. But it's a basic macbook air m2 sku

@cartersocha
Copy link
Contributor

it did improve build time by ~36 seconds and any improvement is welcome

@cartersocha cartersocha merged commit d7481db into open-telemetry:main Nov 11, 2022
@mviitane
Copy link
Member Author

No real change on my personal machine. But it's a basic macbook air m2 sku

@cartersocha, it can be that Docker Desktop allocates only 2 CPUs by default (for MacBook Air). In that case, this PR makes no difference. You can check this from Docker Desktop Preferences/Resources and allocate more. You should get faster build times when you give more than 2 CPUs.

@cartersocha
Copy link
Contributor

Ahh of course thanks. I overlooked that.

Should we document a cpu recommendation in the docker or contributor file? Bumping my cpus up to 5 + your updates dropped my build time to ~830 seconds from ~1150.

Ideally we still make perf improvements to currency, feature flag, and shipping services regardless

@mviitane mviitane deleted the currencyservice_build_time branch February 2, 2024 13:20
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants