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

Change stack size on all desktop platforms to at least 1.5MB #98007

Merged
merged 21 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c02fb4d
Change stack size
hamarb123 Feb 5, 2024
b39177d
Merge branch 'main' into main8
hamarb123 Feb 6, 2024
2dbbfc9
Fix test
hamarb123 Feb 6, 2024
78e7135
Revert changes to PEHeaderBuilder
hamarb123 Feb 6, 2024
bc404d8
Revert the part of the reversion that wasn't a reversion?
hamarb123 Feb 6, 2024
a473b2a
Change tests and fix for NAOT
hamarb123 Feb 6, 2024
8c235d9
Fix tests
hamarb123 Feb 6, 2024
48b0a2a
Move UseAppHost=false test to another folder
hamarb123 Feb 6, 2024
ace1720
Change stack size to 1.5MB on all desktop platforms & re-enable some …
hamarb123 Feb 6, 2024
8ca1c96
Re-enable some tests, and respect `IlcDefaultStackSize` on Windows
hamarb123 Feb 6, 2024
2abc0cd
Fix comments
hamarb123 Feb 6, 2024
7bc3c70
Apply feedback, up stack size on Mac Catalyst, disable (expected) fai…
hamarb123 Feb 7, 2024
499ff8c
Update AppHost test to only run on Windows
hamarb123 Feb 7, 2024
08ceb1b
Disable building AppHost test on unsupported platforms
hamarb123 Feb 7, 2024
2773a79
Increase stack size for all apple platforms, remove unnecessary comment
hamarb123 Feb 7, 2024
99f07de
Test if AppHost variant is running on Windows
hamarb123 Feb 7, 2024
9bd87cf
Try running GitHub_87879_AppHost test on same platforms as GitHub_87879
hamarb123 Feb 8, 2024
9a249f0
Rename test87879_AppHost.csproj
hamarb123 Feb 8, 2024
c07d228
Merge remote-tracking branch 'upstream/main' into main8
hamarb123 Feb 8, 2024
63c6108
Remove AppHost test, and update test projects to run on all Apple pla…
hamarb123 Feb 9, 2024
3cf73dc
Apply feedback
hamarb123 Feb 10, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/native/configurecompiler.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ if (MSVC)
if (CLR_CMAKE_ENABLE_SANITIZERS)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:0x800000")
else()
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:0x180000")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} /STACK:0x400000")
endif()

if(EXISTS ${CLR_SOURCELINK_FILE_PATH})
Expand Down
10 changes: 9 additions & 1 deletion src/coreclr/pal/src/init/pal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,20 @@ InitializeDefaultStackSize()
}
}

#ifdef HOST_OSX
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
// Match Windows stack size
if (g_defaultStackSize == 0)
{
g_defaultStackSize = 4096 * 1024;
}
#endif

#ifdef ENSURE_PRIMARY_STACK_SIZE
if (g_defaultStackSize == 0)
{
// Set the default minimum stack size for MUSL to the same value as we
// use on Windows.
g_defaultStackSize = 1536 * 1024;
g_defaultStackSize = 4096 * 1024;
}
#endif // ENSURE_PRIMARY_STACK_SIZE
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3400,7 +3400,7 @@ internal PEHeader() { }
}
public sealed partial class PEHeaderBuilder
{
public PEHeaderBuilder(System.Reflection.PortableExecutable.Machine machine = System.Reflection.PortableExecutable.Machine.Unknown, int sectionAlignment = 8192, int fileAlignment = 512, ulong imageBase = (ulong)4194304, byte majorLinkerVersion = (byte)48, byte minorLinkerVersion = (byte)0, ushort majorOperatingSystemVersion = (ushort)4, ushort minorOperatingSystemVersion = (ushort)0, ushort majorImageVersion = (ushort)0, ushort minorImageVersion = (ushort)0, ushort majorSubsystemVersion = (ushort)4, ushort minorSubsystemVersion = (ushort)0, System.Reflection.PortableExecutable.Subsystem subsystem = System.Reflection.PortableExecutable.Subsystem.WindowsCui, System.Reflection.PortableExecutable.DllCharacteristics dllCharacteristics = System.Reflection.PortableExecutable.DllCharacteristics.DynamicBase | System.Reflection.PortableExecutable.DllCharacteristics.NoSeh | System.Reflection.PortableExecutable.DllCharacteristics.NxCompatible | System.Reflection.PortableExecutable.DllCharacteristics.TerminalServerAware, System.Reflection.PortableExecutable.Characteristics imageCharacteristics = System.Reflection.PortableExecutable.Characteristics.Dll, ulong sizeOfStackReserve = (ulong)1048576, ulong sizeOfStackCommit = (ulong)4096, ulong sizeOfHeapReserve = (ulong)1048576, ulong sizeOfHeapCommit = (ulong)4096) { }
public PEHeaderBuilder(System.Reflection.PortableExecutable.Machine machine = System.Reflection.PortableExecutable.Machine.Unknown, int sectionAlignment = 8192, int fileAlignment = 512, ulong imageBase = (ulong)4194304, byte majorLinkerVersion = (byte)48, byte minorLinkerVersion = (byte)0, ushort majorOperatingSystemVersion = (ushort)4, ushort minorOperatingSystemVersion = (ushort)0, ushort majorImageVersion = (ushort)0, ushort minorImageVersion = (ushort)0, ushort majorSubsystemVersion = (ushort)4, ushort minorSubsystemVersion = (ushort)0, System.Reflection.PortableExecutable.Subsystem subsystem = System.Reflection.PortableExecutable.Subsystem.WindowsCui, System.Reflection.PortableExecutable.DllCharacteristics dllCharacteristics = System.Reflection.PortableExecutable.DllCharacteristics.DynamicBase | System.Reflection.PortableExecutable.DllCharacteristics.NoSeh | System.Reflection.PortableExecutable.DllCharacteristics.NxCompatible | System.Reflection.PortableExecutable.DllCharacteristics.TerminalServerAware, System.Reflection.PortableExecutable.Characteristics imageCharacteristics = System.Reflection.PortableExecutable.Characteristics.Dll, ulong sizeOfStackReserve = (ulong)0, ulong sizeOfStackCommit = (ulong)0, ulong sizeOfHeapReserve = (ulong)0, ulong sizeOfHeapCommit = (ulong)0) { }
public System.Reflection.PortableExecutable.DllCharacteristics DllCharacteristics { get { throw null; } }
public int FileAlignment { get { throw null; } }
public ulong ImageBase { get { throw null; } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ public PEHeaderBuilder(
Subsystem subsystem = Subsystem.WindowsCui,
DllCharacteristics dllCharacteristics = DllCharacteristics.DynamicBase | DllCharacteristics.NxCompatible | DllCharacteristics.NoSeh | DllCharacteristics.TerminalServerAware,
Characteristics imageCharacteristics = Characteristics.Dll,
ulong sizeOfStackReserve = 0x00100000,
ulong sizeOfStackCommit = 0x1000,
ulong sizeOfHeapReserve = 0x00100000,
ulong sizeOfHeapCommit = 0x1000)
ulong sizeOfStackReserve = 0,
ulong sizeOfStackCommit = 0,
ulong sizeOfHeapReserve = 0,
ulong sizeOfHeapCommit = 0)
{
if (fileAlignment < 512 || fileAlignment > 64 * 1024 || BitArithmetic.CountBits(fileAlignment) != 1)
{
Expand All @@ -89,10 +89,10 @@ public PEHeaderBuilder(
Subsystem = subsystem;
DllCharacteristics = dllCharacteristics;
ImageCharacteristics = imageCharacteristics;
SizeOfStackReserve = sizeOfStackReserve;
SizeOfStackCommit = sizeOfStackCommit;
SizeOfHeapReserve = sizeOfHeapReserve;
SizeOfHeapCommit = sizeOfHeapCommit;
SizeOfStackReserve = sizeOfStackReserve == 0 ? (Is32Bit ? 0x1000u : 0x4000u) : sizeOfStackReserve;
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
SizeOfStackCommit = sizeOfStackCommit == 0 ? (Is32Bit ? 0x1000000u : 0x4000000u) : sizeOfStackCommit;
SizeOfHeapReserve = sizeOfHeapReserve == 0 ? 0x100000u : sizeOfHeapReserve;
SizeOfHeapCommit = sizeOfHeapCommit == 0 ? (Is32Bit ? 0x1000u : 0x2000u) : sizeOfHeapCommit;
}

public static PEHeaderBuilder CreateExecutableHeader()
Expand Down
8 changes: 8 additions & 0 deletions src/native/libs/System.Native/pal_threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,14 @@ int32_t SystemNative_CreateThread(uintptr_t stackSize, void *(*startAddress)(voi
error = pthread_attr_setdetachstate(&attrs, PTHREAD_CREATE_DETACHED);
assert(error == 0);

#ifdef HOST_OSX
// Match Windows stack size
if (stackSize == 0)
{
stackSize = 4096 * 1024;
}
#endif

if (stackSize > 0)
{
if (stackSize < (uintptr_t)PTHREAD_STACK_MIN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ extern "C" DLL_EXPORT void InvokeCallbackOnNewThread(PFNACTION1 callback)
int st = pthread_attr_init(&attr);
AbortIfFail(st);

// set stack size to 1.5MB
st = pthread_attr_setstacksize(&attr, 0x180000);
// set stack size to 4.0MB
st = pthread_attr_setstacksize(&attr, 0x400000);
AbortIfFail(st);

pthread_t t;
Expand Down
39 changes: 39 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_87879/test87879.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Threading;
using System.Runtime.CompilerServices;

class Program
{
[SkipLocalsInit]
static int Main()

Check failure on line 11 in src/tests/Regressions/coreclr/GitHub_87879/test87879.cs

View check run for this annotation

Azure Pipelines / runtime (Build coreclr Common Pri0 Test Build AnyOS AnyCPU checked)

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs#L11

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs(11,16): error XUW1001: Projects in merged tests group should not have entry points. Convert to Facts or Theories. [/__w/1/s/src/tests/Regressions/coreclr/GitHub_87879/test87879.csproj]

Check failure on line 11 in src/tests/Regressions/coreclr/GitHub_87879/test87879.cs

View check run for this annotation

Azure Pipelines / runtime (Build mono Common Pri0 Test Build AnyOS AnyCPU release)

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs#L11

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs(11,16): error XUW1001: Projects in merged tests group should not have entry points. Convert to Facts or Theories. [/__w/1/s/src/tests/Regressions/coreclr/GitHub_87879/test87879.csproj]

Check failure on line 11 in src/tests/Regressions/coreclr/GitHub_87879/test87879.cs

View check run for this annotation

Azure Pipelines / runtime (Build linux-x64 Release AllSubsets_Mono_LLVMAot_RuntimeTests llvmaot)

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs#L11

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs(11,16): error XUW1001: Projects in merged tests group should not have entry points. Convert to Facts or Theories. [/__w/1/s/src/tests/Regressions/coreclr/GitHub_87879/test87879.csproj]

Check failure on line 11 in src/tests/Regressions/coreclr/GitHub_87879/test87879.cs

View check run for this annotation

Azure Pipelines / runtime (Build linux-arm64 Release AllSubsets_Mono_Minijit_RuntimeTests minijit)

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs#L11

src/tests/Regressions/coreclr/GitHub_87879/test87879.cs(11,16): error XUW1001: Projects in merged tests group should not have entry points. Convert to Facts or Theories. [/__w/1/s/src/tests/Regressions/coreclr/GitHub_87879/test87879.csproj]
{
//determine the expected available stack size (4MB or 1MB), minus a little bit (384kB) for overhead.
var expectedSize = (IntPtr.Size == 8 ? 0x4_00000 : 0x1_00000) - 0x60000;

//allocate 4MB, minus a little bit (512kB) for overhead
Span<byte> bytes = stackalloc byte[expectedSize];
Consume(bytes);
Console.WriteLine("Main thread succeeded.");

//repeat on a secondary thread
Thread t = new Thread([SkipLocalsInit] () =>
{
Span<byte> bytes = stackalloc byte[expectedSize];
Consume(bytes);
});
t.Start();
t.Join();
Console.WriteLine("Secondary thread succeeded.");

//success
return 100;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Consume(Span<byte> bytes)
{
}
}
11 changes: 11 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_87879/test87879.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestTargetUnsupported>true</CLRTestTargetUnsupported>
<CLRTestTargetUnsupported Condition="'$(TargetsWindows)' == 'true' OR ('$(TargetsUnix)' == 'true' AND '$(TargetsMobile)' != 'true')">false</CLRTestTargetUnsupported>
</PropertyGroup>
<ItemGroup>
<Compile Include="test87879.cs" />
</ItemGroup>
</Project>
42 changes: 42 additions & 0 deletions src/tests/nativeaot/DesktopStackSize/DesktopStackSize.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Licensed to the .NET Foundation under one or more agreements.
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Threading;
using System.Runtime.CompilerServices;

namespace DesktopStackSize
{
class Program
{
[SkipLocalsInit]
static int Main()
{
//determine the expected available stack size (4MB or 1MB), minus a little bit (384kB) for overhead.
var expectedSize = (IntPtr.Size == 8 ? 0x4_00000 : 0x1_00000) - 0x60000;

//allocate 4MB, minus a little bit (512kB) for overhead
Span<byte> bytes = stackalloc byte[expectedSize];
Consume(bytes);
Console.WriteLine("Main thread succeeded.");

//repeat on a secondary thread
Thread t = new Thread([SkipLocalsInit] () =>
{
Span<byte> bytes = stackalloc byte[expectedSize];
Consume(bytes);
});
t.Start();
t.Join();
Console.WriteLine("Secondary thread succeeded.");

//success
return 100;
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Consume(Span<byte> bytes)
{
}
}
}
12 changes: 12 additions & 0 deletions src/tests/nativeaot/DesktopStackSize/DesktopStackSize.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestTargetUnsupported>true</CLRTestTargetUnsupported>
<CLRTestTargetUnsupported Condition="'$(TargetsWindows)' == 'true' OR ('$(TargetsUnix)' == 'true' AND '$(TargetsMobile)' != 'true')">false</CLRTestTargetUnsupported>
</PropertyGroup>

<ItemGroup>
<Compile Include="DesktopStackSize.cs" />
</ItemGroup>
</Project>
Loading