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

Make Bazel's RAM estimate container aware #20435

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Dec 5, 2023

As of JDK 14, OperatingSystemMXBean provides information about system memory that is container-aware. Outside containers, it uses the same mechanisms as Bazel to determine available RAM (/proc/meminfo on Linux, hw.memsize on macOS) and can thus be used as a drop-in replacement for the custom implementation.

A small caveat is that Bazel's macOS RAM estimate was based on converting bytes to "MB" via a divisor of 1000^2 instead of 1024^2, resulting in a consistent overestimate compared to an identical Linux machine that is now corrected.

This opportunity was missed in #16512 since OperatingSystemMXBean is based on a complete Java implementation of cgroups handling and doesn't go through the os::total_memory or os::physical_memory Hotspot functions.

RELNOTES[INC]:

  • On Linux, Bazel's RAM estimate for the host machine is now aware of container resource limits.
  • On macOS, Bazel no longer consistently overestimates the total RAM by ~5% (1024^2/1000^2).
  • On Windows, Bazel's RAM estimate is now generally more accurate as it is no longer influenced by JVM heuristics.

Fixes #3886

@fmeum
Copy link
Collaborator Author

fmeum commented Dec 5, 2023

@meisterT Could you review this follow-up to #16512? It's another case of a change that is very hard to guard behind an incompatible flag.

@fmeum fmeum marked this pull request as ready for review December 5, 2023 18:13
@github-actions github-actions bot added team-Performance Issues for Performance teams awaiting-review PR is awaiting review from an assigned reviewer labels Dec 5, 2023
@meisterT meisterT added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Dec 6, 2023
@brentleyjones
Copy link
Contributor

@bazel-io flag

@bazel-io bazel-io added the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 6, 2023
@iancha1992
Copy link
Member

@bazel-io fork 7.1.0

@bazel-io bazel-io removed the potential release blocker Flagged by community members using "@bazel-io flag". Should be added to a release blocker milestone label Dec 6, 2023
@copybara-service copybara-service bot closed this in 2f3cdc5 Dec 7, 2023
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Dec 7, 2023
@fmeum fmeum deleted the container-aware-ram branch December 7, 2023 15:31
bazel-io pushed a commit to bazel-io/bazel that referenced this pull request Dec 21, 2023
As of JDK 14, `OperatingSystemMXBean` provides information about system memory that is container-aware. Outside containers, it uses the same mechanisms as Bazel to determine available RAM (`/proc/meminfo` on Linux, `hw.memsize` on macOS) and can thus be used as a drop-in replacement for the custom implementation.

A small caveat is that Bazel's macOS RAM estimate was based on converting bytes to "MB" via a divisor of `1000^2` instead of `1024^2`, resulting in a consistent overestimate compared to an identical Linux machine that is now corrected.

This opportunity was missed in bazelbuild#16512 since `OperatingSystemMXBean` is based on a complete Java implementation of cgroups handling and doesn't go through the `os::total_memory` or `os::physical_memory` Hotspot functions.

RELNOTES[INC]:
* On Linux, Bazel's RAM estimate for the host machine is now aware of container resource limits.
* On macOS, Bazel no longer consistently overestimates the total RAM by ~5% (`1024^2/1000^2`).
* On Windows, Bazel's RAM estimate is now generally more accurate as it is no longer influenced by JVM heuristics.

Fixes bazelbuild#3886

Closes bazelbuild#20435.

PiperOrigin-RevId: 588718034
Change-Id: I2daafa0567740a1b149ca8756ec27f102129283c
github-merge-queue bot pushed a commit that referenced this pull request Jan 11, 2024
As of JDK 14, `OperatingSystemMXBean` provides information about system
memory that is container-aware. Outside containers, it uses the same
mechanisms as Bazel to determine available RAM (`/proc/meminfo` on
Linux, `hw.memsize` on macOS) and can thus be used as a drop-in
replacement for the custom implementation.

A small caveat is that Bazel's macOS RAM estimate was based on
converting bytes to "MB" via a divisor of `1000^2` instead of `1024^2`,
resulting in a consistent overestimate compared to an identical Linux
machine that is now corrected.

This opportunity was missed in
#16512 since
`OperatingSystemMXBean` is based on a complete Java implementation of
cgroups handling and doesn't go through the `os::total_memory` or
`os::physical_memory` Hotspot functions.

RELNOTES[INC]:
* On Linux, Bazel's RAM estimate for the host machine is now aware of
container resource limits.
* On macOS, Bazel no longer consistently overestimates the total RAM by
~5% (`1024^2/1000^2`).
* On Windows, Bazel's RAM estimate is now generally more accurate as it
is no longer influenced by JVM heuristics.

Fixes #3886

Closes #20435.

Commit
2f3cdc5

PiperOrigin-RevId: 588718034
Change-Id: I2daafa0567740a1b149ca8756ec27f102129283c

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Performance Issues for Performance teams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default local resources should respect cgroup limits on Linux
5 participants