[GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image. #7277

graalvmbot · 2023-08-25T07:31:33Z

No description provided.

jerboaa

Some food for thought. I'm not sure this patch will be helping. Take with a grain of salt :-)

jerboaa · 2023-08-25T12:30:59Z

substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MemoryUtil.java

+        var containerMetrics = jdk.internal.platform.Container.metrics();
+        if (containerMetrics != null) {
+            long memoryLimit = containerMetrics.getMemoryLimit();
+            if (memoryLimit >= 0) {
+                /*
+                 * In containerized environments with a memory limit, do not use any headroom to
+                 * avoid OutOfMemoryErrors.
+                 */
+                return (long) (memoryLimit * MAX_ALLOWED_CONTAINER_MEMORY_RATIO);
+            }


Aside: Note that this isn't really available memory in the container case. getAvailableMemorySizeLinux() will return 80% of the set container memory limit. For awareness.

Right, I guess I shifted the meaning of available memory in this file from "available system memory" to "memory available to the builder". Maybe I should rename the methods...

jerboaa · 2023-08-25T12:54:40Z

substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/MemoryUtil.java

+                /*
+                 * Based on total swap, set headroom somewhere between zero (no swap) and
+                 * MAX_HEADROOM_RATIO of available memory (total swap exceeds available memory).
+                 */
+                long reasonableHeadroomKiB = (long) (Math.min(swapTotalKiB, memAvailableKiB) * MAX_HEADROOM_RATIO);
+                return (memAvailableKiB + reasonableHeadroomKiB) * KiB_TO_BYTES;


I wonder what the headroom is supposed to achieve. It will likely not help systems with sufficient available memory (swap memory won't be used). I'd argue it will likely not help systems with very limited available memory, either. If this should help cases like GHA, where we'd have a virtualized system (or physical machine), not containers, where available memory - A - will be small, this extra headroom will not help much. The generator will likely get about A * 1.2 of memory (instead of A amount of memory prior this patch).

Considering a GHA runner with, say 8 GB of total RAM, it's not totally inconceivable with a maven build to only have say, 2GB, available at an inconvenient time. Let that runner have 4GB of total swap, that particular runner will grant the native image generator 2.4 GB of memory to use. Will that bump from 2 GB to 2.4GB really make a significant difference? I doubt it.

Considering a patch which used 50% of total swap space as the extra headroom, that would bump the memory to use for the image generator from 2 to 4 GB, which doesn't seem to be too unreasonable. Some food for thought.

I wonder what the headroom is supposed to achieve.

It's supposed to compensate for the cases when available memory at startup is lower than it could be. Also, there's no easy way to determine swap space on macOS and Windows, so this helps there too.

Considering a patch which used 50% of total swap space as the extra headroom.

How about using 80% of free swap space, or 20% of available memory, whichever is larger? If we use 50% of total swap space but the swap if full, aren't we going to put the system in trouble?

I wonder what the headroom is supposed to achieve.

It's supposed to compensate for the cases when available memory at startup is lower than it could be.

Got it. It makes me think if the current patch will help (on Linux), though. I welcome the container simplification.

Considering a patch which used 50% of total swap space as the extra headroom.

How about using 80% of free swap space, or 20% of available memory, whichever is larger?

I think that would be better than the current approach.

If we use 50% of total swap space but the swap if full, aren't we going to put the system in trouble?

Note that the previous heuristic before considering available memory (only) - current master - didn't consider whether or not swap was available. It used 80% of physical memory, AFAIK. Considering a system with 64GB of memory and 32 gb of swap, it would use up to ~51gb of memory. On systems with little available memory at the time, it's conceivable that would cause swapping, possibly up to 100% of total swap. Now we are on the other extreme of the pendulum since only available memory (estimate before using swap) is being used. Given that, I find 50% of total swap isn't unreasonable as an upper bound to land somewhere between where we were and current master. That's my thinking.

Using the available swap metric has it's own set of problems as it's again a snapshot value at a specific point in time.

Anyway, I think the higher of '80% of free swap and 20% of available memory' will do the trick too.

Now we are on the other extreme of the pendulum since only available memory (estimate before using swap) is being used. Given that, I find 50% of total swap isn't unreasonable as an upper bound to land somewhere between where we were and current master. That's my thinking.

Yes, you are right and I agree. I'm just not sure if we want to go straight to the middle of the two approaches. My thinking was to try a 20% headroom first before going to 50% of swap.

Anyway, I think the higher of '80% of free swap and 20% of available memory' will do the trick too.

Ok, let's go with this for now. Keep in mind that we still make sure that the MaxRAMPercentage is about 100%, and that we use more than 32GB of memory to avoid compressed pointers. So we should be fine if the headroom exceeds any of the two upper limits.

jerboaa

LGTM. Thanks for taking my considerations into account.

fniephaus · 2023-08-28T08:50:11Z

LGTM. Thanks for taking my considerations into account.

Of course! Thanks for your feedback and taking the time to discuss different approaches.

jerboaa · 2023-08-31T08:54:56Z

substratevm/mx.substratevm/mx_substratevm.py

@@ -1019,6 +1019,7 @@ def _native_image_launcher_extra_jvm_args():
 ] + svm_experimental_options([
    '-H:IncludeResources=com/oracle/svm/driver/launcher/.*',
    '-H:-ParseRuntimeOptions',
+    f'-R:MaxHeapSize={256 * 1024 * 1024}',


Shouldn't this go into the bash launcher as well? My understanding is this would only affect the AOT compiled native-image binary at runtime, right?

That's correct, we don't have any good datapoints on how large this should be on HotSpot. Does Mandrel set a max heap already?

No, we don't set anything specifically.

The native driver uses the Serial GC which uses 80% of total memory as max heap by default, the HotSpot GC use a much smaller share. So unless we have a problem with bash launchers and a good understanding how much memory the driver needs on HotSpot, let's set a max heap for the native driver for now? (we'd notice memory leaks that way, too, unlikely they are HotSpot only)

This option is used, for example, by Native Build Tools to inject metadata generated with the agent or from the reachability metadata repository.

See oracle#7277 (comment) for motivation. We have been using this as the default in Mandrel since the 23.1 release.

graalvmbot assigned fniephaus Aug 25, 2023

oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 25, 2023

graalvmbot changed the title ~~[GR-48232] Improve max heap settings of Native Image builder.~~ [GR-48232] Improve GC settings of Native Image builder. Aug 25, 2023

graalvmbot force-pushed the fniephaus/GR-48232/memory-util branch from 93d8885 to 9deec4a Compare August 25, 2023 11:27

graalvmbot changed the title ~~[GR-48232] Improve GC settings of Native Image builder.~~ [GR-46399] [GR-48232] Improve GC settings of Native Image builder. Aug 25, 2023

fniephaus requested a review from jerboaa August 25, 2023 11:38

fniephaus added this to the GraalVM for JDK 21 (September 19, 2023) milestone Aug 25, 2023

jerboaa reviewed Aug 25, 2023

View reviewed changes

graalvmbot force-pushed the fniephaus/GR-48232/memory-util branch from 9deec4a to dd8b035 Compare August 28, 2023 07:53

jerboaa approved these changes Aug 28, 2023

View reviewed changes

graalvmbot force-pushed the fniephaus/GR-48232/memory-util branch 2 times, most recently from ea0f7c4 to b218f21 Compare August 29, 2023 12:24

graalvmbot changed the title ~~[GR-46399] [GR-48232] Improve GC settings of Native Image builder.~~ [GR-46399] [GR-48232] [GR-48207] Improve GC settings of Native Image builder. Aug 29, 2023

graalvmbot changed the title ~~[GR-46399] [GR-48232] [GR-48207] Improve GC settings of Native Image builder.~~ [GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image. Aug 31, 2023

graalvmbot force-pushed the fniephaus/GR-48232/memory-util branch from b218f21 to 1b717f2 Compare August 31, 2023 07:14

No longer avoid init of metrics in OperatingSystemImpl.

1f5a05e

graalvmbot force-pushed the fniephaus/GR-48232/memory-util branch 3 times, most recently from f10a363 to 3f7a965 Compare August 31, 2023 08:06

jerboaa reviewed Aug 31, 2023

View reviewed changes

fniephaus added 7 commits August 31, 2023 12:45

Use free memory to simplify MemoryUtil.

40e779a

Increase GC goal from 5% to 10%.

5160c49

Do not print null chars as part of APIOption.

6f153f3

Inject driver origin when falling back.

47c826e

Define a 256MiB max heap for the SVM driver.

dd21ccb

Record activity when generating debuginfo.

34d8885

Mark ConfigurationFileDirectories as stable.

d28d39b

This option is used, for example, by Native Build Tools to inject metadata generated with the agent or from the reachability metadata repository.

graalvmbot force-pushed the fniephaus/GR-48232/memory-util branch from 3f7a965 to d28d39b Compare August 31, 2023 10:50

graalvmbot changed the title ~~[GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image.~~ [GR-46399] [GR-48232] [GR-48207] [GR-48180] [GR-48349] Various important bug fixes for Native Image. Aug 31, 2023

jerboaa mentioned this pull request Aug 31, 2023

Add support for MaxRAM restriction for the native image driver process graalvm/mandrel#557

Closed

graalvmbot changed the title ~~[GR-46399] [GR-48232] [GR-48207] [GR-48180] [GR-48349] Various important bug fixes for Native Image.~~ [GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image. Aug 31, 2023

Update TruffleRuby import.

f664277

graalvmbot force-pushed the fniephaus/GR-48232/memory-util branch from bac6f47 to f664277 Compare August 31, 2023 18:54

graalvmbot merged commit 46de604 into master Sep 1, 2023
12 checks passed

graalvmbot deleted the fniephaus/GR-48232/memory-util branch September 1, 2023 09:16

fniephaus mentioned this pull request Sep 5, 2023

Native build uses a lot of memory graalvm/native-build-tools#494

Closed

linghengqian mentioned this pull request Sep 5, 2023

Make ShardingSphere Proxy in GraalVM Native Image form available apache/shardingsphere#21347

Closed

7 tasks

zakkak added a commit to zakkak/mandrel that referenced this pull request Feb 13, 2024

Set MaxRAM and MaxRAMPercentage in launcher for native-image driver

67a3be2

See oracle#7277 (comment) for motivation. We have been using this as the default in Mandrel since the 23.1 release.

zakkak mentioned this pull request Feb 13, 2024

Set MaxRAM and MaxRAMPercentage in launcher for native-image driver #8366

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image. #7277

[GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image. #7277

graalvmbot commented Aug 25, 2023 •

edited by fniephaus

Loading

jerboaa left a comment

jerboaa Aug 25, 2023

fniephaus Aug 25, 2023

jerboaa Aug 25, 2023

fniephaus Aug 25, 2023 •

edited

Loading

jerboaa Aug 25, 2023

fniephaus Aug 28, 2023

jerboaa left a comment

fniephaus commented Aug 28, 2023

jerboaa Aug 31, 2023

fniephaus Aug 31, 2023 •

edited

Loading

jerboaa Aug 31, 2023

fniephaus Aug 31, 2023 •

edited

Loading

jerboaa Aug 31, 2023

[GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image. #7277

[GR-46399] [GR-48232] [GR-48207] [GR-48180] Various important bug fixes for Native Image. #7277

Conversation

graalvmbot commented Aug 25, 2023 • edited by fniephaus Loading

jerboaa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fniephaus Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerboaa left a comment

Choose a reason for hiding this comment

fniephaus commented Aug 28, 2023

Choose a reason for hiding this comment

fniephaus Aug 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fniephaus Aug 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

graalvmbot commented Aug 25, 2023 •

edited by fniephaus

Loading

fniephaus Aug 25, 2023 •

edited

Loading

fniephaus Aug 31, 2023 •

edited

Loading

fniephaus Aug 31, 2023 •

edited

Loading