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

[GR-52400] Revise heuristic for memory usage of Native Image build process. #10441

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

graalvmbot
Copy link
Collaborator

@graalvmbot graalvmbot commented Jan 9, 2025

This PR mainly revises the heuristic that is used, unless overridden by flags, to determine the memory limits of the Native Image build process. The heuristic selects one for two modes:

  1. Dedicated Mode: use a fixed percentage (85% of system memory) to maximize resource usage for fast builds
  2. Shared Mode: use available memory to avoid slowing down the build machine

Unless overridden with Xmx or other memory flags, the heuristic will use dedicated mode in containers and CI environments. Otherwise, it will try to use available memory except if there is not enough memory available (in that case it falls back to the dedicated mode).

The PR also adjusts the build resource section of the build output so that it shows a reason for selected memory limit.

In addition, the PR also addresses a number of smaller issues (see individual commits).

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jan 9, 2025
@graalvmbot graalvmbot force-pushed the fniephaus/GR-52400/memory-usage branch 2 times, most recently from 019ab36 to ee2c4d1 Compare January 10, 2025 10:15
@graalvmbot graalvmbot changed the title [GR-52400] Attempt to improve memory usage of builder. WIP [GR-52400] Revise heuristic for memory usage of Native Image build process. Jan 10, 2025
@graalvmbot graalvmbot force-pushed the fniephaus/GR-52400/memory-usage branch from ee2c4d1 to 9ef987f Compare January 10, 2025 10:46
@fniephaus fniephaus requested review from jerboaa, zakkak and olpaw January 10, 2025 10:52
@graalvmbot graalvmbot force-pushed the fniephaus/GR-52400/memory-usage branch from 9ef987f to e424fb5 Compare January 10, 2025 10:56
Comment on lines 125 to 127
memoryUsageReason = "enough available";
} else { // fall back to dedicated mode
memoryUsageReason = "not enough available";
Copy link
Collaborator

Choose a reason for hiding this comment

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

"(not) enough available" seems confusing to me.

  • "enough available" makes me think I should not tune anything even if my build fails due to out of memory or I get a lot of time spent on GC. I would replace this with something like "available free memory used".
  • "not enough available" makes me feel something is wrong and I need to tune things. I would replace this with something like "less than 8GB ram is available, setting mac to 85% of system memory".

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1. Mentioning the threshold value of 8G would be helpful to the user.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we can do that. I also tried to keep the text short so that the lines fit in 80 characters. I'll see if I can come up with something better.

Copy link
Member

Choose a reason for hiding this comment

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

Changed it to:

 - 13.24GB of memory (85.0% of system memory, less than 8GB of memory available)

for now.

docs/reference-manual/native-image/BuildOutput.md Outdated Show resolved Hide resolved
Comment on lines 125 to 127
memoryUsageReason = "enough available";
} else { // fall back to dedicated mode
memoryUsageReason = "not enough available";
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1. Mentioning the threshold value of 8G would be helpful to the user.

*/
reasonableMaxMemorySize = totalMemorySize * DEDICATED_MODE_TOTAL_MEMORY_RATIO;
String memoryUsageReason = "unknown";
final boolean isDedicatedMemoryUsage;
Copy link
Collaborator

@jerboaa jerboaa Jan 10, 2025

Choose a reason for hiding this comment

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

It looks like there is no way for a developer to chose shared mode explicitly (only happens if $CI != true and not containerized). dedicated can be forced with CI=true, but not so for shared. Maybe that would be worth doing?

Copy link
Member

Choose a reason for hiding this comment

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

You are essentially asking for #8468. I thought about this some more and it didn't seem worth the additional complexity (needs special option treatment because it happens very early in the process). Why would anyone force available memory when that is a moving target? Also, you can easily override the default behavior with Xmx and other memory flags.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure. No strong feelings about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants