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

feature request: better build @com_googlesource_chromium_v8:build #15145

Closed
lambdai opened this issue Feb 23, 2021 · 4 comments · Fixed by #19275
Closed

feature request: better build @com_googlesource_chromium_v8:build #15145

lambdai opened this issue Feb 23, 2021 · 4 comments · Fixed by #19275
Assignees

Comments

@lambdai
Copy link
Contributor

lambdai commented Feb 23, 2021

This target is internal spawn lots of tasks within. However, this target is considered as a single-core task from the view of bazel scheduler.

Generally speaking, when you run bazel build //source/exe:envoy with N cores, N tasks will be scheduled and at most N clang is running. However, if the running task contains @com_googlesource_chromium_v8:build, ~2N clang is running, and the peak ram usage could be huge.

A workaround: you may want to run bazel build @com_googlesource_chromium_v8//:build first, exhausting the cpu and ram.
When the above task is completed, you can run bazel build YOUR_REAL_TARGET using the build cache of the above v8 build.

@lambdai lambdai added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Feb 23, 2021
@PiotrSikora
Copy link
Contributor

The official Bazel support has been requested from the V8 team (https://bugs.chromium.org/p/v8/issues/detail?id=11234), and I don't think we can do too much until that's ready, since Envoy doesn't want to maintain custom BUILD files.

To address the issue at hand, perhaps we could provide an upper bound to limit the number of CPUs that ninja can spawn when building V8? Note that this will slow-down the whole process if V8 is scheduled to be build towards the end of the build, so the workaround you suggested might actually be the best thing to do.

@lambdai
Copy link
Contributor Author

lambdai commented Feb 23, 2021

Thank you for the confirmation and for sharing the update at chromium! I don't think Envoy should invest too much.

@zuercher zuercher added area/build and removed triage Issue requires triage labels Feb 24, 2021
@github-actions
Copy link

github-actions bot commented Apr 3, 2021

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Apr 3, 2021
@mattklein123 mattklein123 added help wanted Needs help! area/wasm and removed enhancement Feature requests. Not bugs or questions. stale stalebot believes this issue/PR has not been touched recently labels Apr 5, 2021
@PiotrSikora PiotrSikora self-assigned this Jun 15, 2021
@PiotrSikora
Copy link
Contributor

V8 team is working on this right now, and V8 v9.3 (branch cut on 7/15) should have native Bazel support.

lizan pushed a commit that referenced this issue Feb 9, 2022
Fixes #15145.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
joshperry pushed a commit to joshperry/envoy that referenced this issue Feb 13, 2022
Fixes envoyproxy#15145.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Josh Perry <josh.perry@mx.com>
phlax pushed a commit to phlax/envoy that referenced this issue Nov 15, 2022
Fixes envoyproxy#15145.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
phlax pushed a commit that referenced this issue Nov 18, 2022
Fixes #15145.

Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Signed-off-by: Ryan Northey <ryan@synca.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants