Skip to content

Conversation

@janvorli
Copy link
Member

@janvorli janvorli commented Jan 6, 2023

The coreclr runtime currently doesn't honor virtual memory size limit specified by the rlimit APIs (or the ulimit -v shell command).

There were couple of cases in the past where people have hit this problem when trying to run .NET on systems where the administrators have applied virtual memory limit for all users as a simple mean to limit the physical memory usage.

This change enables honoring that limit in the GC and PAL preallocator of the initial executable memory, as those two are the factors preventing coreclr execution on such systems.

@janvorli janvorli added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) area-Meta labels Jan 6, 2023
@janvorli janvorli added this to the 8.0.0 milestone Jan 6, 2023
@janvorli janvorli requested a review from cshung January 6, 2023 15:02
@janvorli janvorli self-assigned this Jan 6, 2023
@ghost
Copy link

ghost commented Jan 6, 2023

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

The coreclr runtime currently doesn't honor virtual memory size limit specified by the rlimit APIs (or the ulimit -v shell command).

There were couple of cases in the past where people have hit this problem when trying to run .NET on systems where the administrators have applied virtual memory limit for all users as a simple mean to limit the physical memory usage.

This change enables honoring that limit in the GC and PAL preallocator of the initial executable memory, as those two are the factors preventing coreclr execution on such systems.

Author: janvorli
Assignees: janvorli
Labels:

NO-MERGE, area-Meta

Milestone: 8.0.0

@janvorli janvorli force-pushed the honor-virtual-memory-limit branch from 608b0c8 to 993ea84 Compare January 6, 2023 15:04
Copy link
Member Author

Choose a reason for hiding this comment

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

I am not happy with this COMPlus env variable name. Any suggestions for better name are welcome.

Copy link
Member Author

Choose a reason for hiding this comment

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

It feels a bit strange to specify the percentage in hex, but it is the default we use for all COMPlus / DOTNET variables, including percentual settings in the GC.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if it is architecturally feasible, but it would be nice if we don't need to proliferate the environment variable parsing rules.

@jeffhandley
Copy link
Member

Hey @janvorli -- I wanted to check in to see if this was still in progress, if it should be converted to draft, if it should be closed for now, or if you want to leave it as-is. Thanks!

@janvorli
Copy link
Member Author

@jeffhandley it is marked as [WIP] and NO-MERGE, I have assumed it is the same as marking it as draft. I still want to finish this, I just got distracted by other important stuff.

@jeffhandley
Copy link
Member

Cool; thanks for the update, @janvorli!

@richlander
Copy link
Member

Should we add a section to this doc on this topic (and generalize the title)?

https://github.com/dotnet/designs/blob/main/accepted/2019/support-for-memory-limits.md

@janvorli
Copy link
Member Author

Should we add a section to this doc on this topic

That makes sense.

@ghost
Copy link

ghost commented Jul 5, 2023

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

The coreclr runtime currently doesn't honor virtual memory size limit specified by the rlimit APIs (or the ulimit -v shell command).

There were couple of cases in the past where people have hit this problem when trying to run .NET on systems where the administrators have applied virtual memory limit for all users as a simple mean to limit the physical memory usage.

This change enables honoring that limit in the GC and PAL preallocator of the initial executable memory, as those two are the factors preventing coreclr execution on such systems.

Author: janvorli
Assignees: janvorli
Labels:

NO-MERGE, area-GC-coreclr

Milestone: 8.0.0

@janvorli janvorli force-pushed the honor-virtual-memory-limit branch 2 times, most recently from 22f1144 to 8dc8055 Compare October 20, 2023 21:58
The coreclr runtime currently doesn't honor virtual memory size limit
specified by the rlimit APIs (or the `ulimit -v` shell command).

There were couple of cases in the past where people have hit this
problem when trying to run .NET on systems where the administrators
have applied virtual memory limit for all users as a simple mean
to limit the physical memory usage.

This change enables honoring that limit in the GC and PAL preallocator
of the initial executable memory, as those two are the factors preventing
coreclr execution on such systems.
@janvorli janvorli force-pushed the honor-virtual-memory-limit branch from 8dc8055 to 86db568 Compare October 23, 2023 09:09
@janvorli janvorli changed the title [WIP] Honor virtual memory limit Honor virtual memory limit Oct 23, 2023
@janvorli janvorli removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 23, 2023
@janvorli janvorli modified the milestones: 8.0.0, 9.0.0 Oct 23, 2023
@janvorli
Copy link
Member Author

@cshung, @Maoni0, the PR is now ready for review

Copy link
Contributor

@cshung cshung left a comment

Choose a reason for hiding this comment

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

LGTM

@janvorli janvorli closed this Nov 15, 2023
@janvorli janvorli reopened this Nov 15, 2023
Comment on lines +187 to +202
set(CMAKE_REQUIRED_LIBRARIES)
check_cxx_source_runs("
#include <fcntl.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
int main(void) {
int fd;
fd = open(\"/proc/self/statm\", O_RDONLY);
if (fd == -1) {
exit(1);
}
exit(0);
}" HAVE_PROCFS_STATM)
Copy link
Contributor

@MichalPetryka MichalPetryka Nov 16, 2023

Choose a reason for hiding this comment

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

What's the point of checking this at build time, shouldn't the runtime not depend on whether the build machine exposes statm?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is to detect whether the OS supports that at all. We don't actually support /proc file system optionally on Linux. There is no other way to get the information we need there. We have similar checks in the PAL configure.cmake.

@janvorli janvorli merged commit eb64819 into dotnet:main Nov 28, 2023
@janvorli janvorli deleted the honor-virtual-memory-limit branch November 28, 2023 16:41
akoeplinger added a commit to akoeplinger/runtime that referenced this pull request Nov 29, 2023
dotnet#80295 added a new entry to tryrun.cmake but iOS/tvOS use a separate tryrun_ios_tvos.cmake which was missing the new entry.
akoeplinger added a commit that referenced this pull request Nov 29, 2023
#80295 added a new entry to tryrun.cmake but iOS/tvOS use a separate tryrun_ios_tvos.cmake which was missing the new entry.
t-mustafin added a commit to t-mustafin/runtime that referenced this pull request Dec 1, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants