Allow stack size to be configured

[keegan-caruso]

EDIT: this has been extended to also work with CoreCLR-VM so that we can resolve #113344 with it.

If a native AOT compiled application can lower its allocated memory, more scenarios that are memory sensitive can use it.

The default stack size can be excessive for simple applications and can significantly contribute to the RSS.

For linux, there is the ulimit command, and this is respected, but it can be too broad.

With this trivial code:

int s = 0;

await Task.Run(() => Interlocked.Increment(ref s));

Console.ReadLine();

On linux with the default stack size of 8mb, RSS measured at 5916.

With setting the stack size to 1mb, the RSS measured at 3428.

Roughly a 40% decrease.

[jkotas]

We have (undocumented) IlcDefaultStackSize build property to set the default stack size in Native AOT binaries. Would it be sufficient for what you are trying to do?

[keegan-caruso]

We have (undocumented) IlcDefaultStackSize build property to set the default stack size in Native AOT binaries. Would it be sufficient for what you are trying to do?

Thanks. This works on Windows, it is not working for me on Linux.

On windows I checked against kernel32 GetCurrentThreadStackLimits. On linux I checked against strace.

[jkotas]

Thanks. This works on Windows, it is not working for me on Linux.

It works for musl libc (musl libc is used e.g. by Alpine Linux). glibc that is used by the most popular Linux distros like Ubuntu does not respect value set by -z,stack-size linker switch for some reason. That's unfortunate.

[jkotas]

Github status is stuck in "Checking for ability to merge automatically…".

Could you please rebase against main and force push? Hopefully, it is going to reset the status.

[MichalStrehovsky]

I think we should look at this in the context of #113344:

Here's a couple questions I have:

• Are we happy with the name of the runtimeconfig.json name of this and DOTNET_ environment variable? (They look good to me.)
• Are we happy with the radix? (I assume we want to keep it as base-16.)

If so, I can add the CoreCLR version so that we can also resolve #113344 and we can doc it.

Cc @kouvel @AaronRobinsonMSFT for opinion

[am11]

The naming looks good as it's consistent with:

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.cs:            AppContextConfigHelper.GetInt16ComPlusOrDotNetConfig("System.Threading.ThreadPool.MinThreads", "ThreadPool_ForceMinWorkerThreads", 0, false);
src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.cs:            AppContextConfigHelper.GetInt16ComPlusOrDotNetConfig("System.Threading.ThreadPool.MaxThreads", "ThreadPool_ForceMaxWorkerThreads", 0, false);
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Unix.cs:            AppContextConfigHelper.GetBooleanConfig("System.Threading.ThreadPool.EnableWorkerTracking", "DOTNET_ThreadPool_EnableWorkerTracking");
src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Windows.cs:            AppContextConfigHelper.GetBooleanConfig("System.Threading.ThreadPool.UseWindowsThreadPool", "DOTNET_ThreadPool_UseWindowsThreadPool");

[AaronRobinsonMSFT]

@MichalStrehovsky See #113344 also.

Are we happy with the name of the runtimeconfig.json name of this and DOTNET_ environment variable? (They look good to me.)

I didn't see the DOTNET_ environment variable. Can you point me to it?

Are we happy with the radix? (I assume we want to keep it as base-16.)

This was a source of confusion on a partner team who had been assuming it was radix 10 for about 6 years. I think having it as radix 16 is fine, but we need to document this properly at https://learn.microsoft.com/dotnet/core/runtime-config/threading.

[AaronRobinsonMSFT]

DOTNET_DefaultStackSize

Seems reasonable to me.

[am11]

I didn't see the DOTNET_ environment variable. Can you point me to it?

It's DOTNET_Threading_DefaultStackSize. Prefix is added later. It's consistent with existing convention DOTNET_ThreadPool_UseWindowsThreadPool, DOTNET_ThreadPool_EnableWorkerTracking etc.

[kouvel]

It's DOTNET_Threading_DefaultStackSize. Prefix is added later. It's consistent with existing convention DOTNET_ThreadPool_UseWindowsThreadPool, DOTNET_ThreadPool_EnableWorkerTracking etc.

CoreCLR already has a DOTNET_DefaultStackSize config setting. There are other thread-specific settings with DOTNET_Thread_*. Should we use the DOTNET_Thread_ prefix for consistency and add the new one to CoreCLR as well?

[MichalStrehovsky]

CoreCLR already has a DOTNET_DefaultStackSize config setting. There are other thread-specific settings with DOTNET_Thread_*. Should we use the DOTNET_Thread_ prefix for consistency and add the new one to CoreCLR as well?

Made this change and standardized on DOTNET_Thread. Note there are some external hits for COMPlus_DefaultStackSize but I guess we should be fine with breaking them, otherwise there's no point in the INTERNAL and EXTERNAL prefixes for CLRConfigs.

I left DEFAULT_STACK_SIZE as is. We could probably delete it in favor of the new System.Threading.DefaultStackSize.

[MichalStrehovsky]

@VSadov is this something you'd be able to review? Could you please have a look?

[AaronRobinsonMSFT]

LGTM.

[jkotas]

We still read DOTNET_DefaultStackSize here: https://github.com/dotnet/runtime/blob/main/src/coreclr/pal/src/init/pal.cpp#L251 . Do we need to change that as well?

[jkotas]

My point is that we have changed DefaultStackSize to Thread_DefaultStackSize in one place, but not in the other place.

[rhuijben]

Why || and not &&. I assume it is always true now?

[MichalStrehovsky]

Good catch! #118077

[MichalStrehovsky]

We still read DOTNET_DefaultStackSize here: https://github.com/dotnet/runtime/blob/main/src/coreclr/pal/src/init/pal.cpp#L251 . Do we need to change that as well?

Good catch! #118076