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 14 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
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<LinkerFlavor Condition="'$(LinkerFlavor)' == '' and '$(_targetOS)' == 'freebsd'">lld</LinkerFlavor>
<LinkerFlavor Condition="'$(LinkerFlavor)' == '' and '$(_linuxLibcFlavor)' == 'bionic'">lld</LinkerFlavor>
<LinkerFlavor Condition="'$(LinkerFlavor)' == '' and '$(_targetOS)' == 'linux'">bfd</LinkerFlavor>
<IlcDefaultStackSize Condition="'$(_linuxLibcFlavor)' == 'musl'">1572864</IlcDefaultStackSize>
<IlcDefaultStackSize Condition="'$(IlcDefaultStackSize)' == '' and '$(_linuxLibcFlavor)' == 'musl'">1572864</IlcDefaultStackSize>
</PropertyGroup>

<Target Name="SetupOSSpecificProps" DependsOnTargets="$(IlcDynamicBuildPropertyDependencies)">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<LinkerSubsystem Condition="'$(OutputType)' == 'Exe' and '$(LinkerSubsystem)' == ''">CONSOLE</LinkerSubsystem>
<EventPipeName>eventpipe-disabled</EventPipeName>
<EventPipeName Condition="'$(EventSourceSupport)' == 'true'">eventpipe-enabled</EventPipeName>
<IlcDefaultStackSize Condition="'$(IlcDefaultStackSize)' == ''">1572864</IlcDefaultStackSize>
</PropertyGroup>

<!-- Ensure that runtime-specific paths have already been set -->
Expand Down Expand Up @@ -94,6 +95,7 @@ The .NET Foundation licenses this file to you under the MIT license.
<LinkerArg Condition="'$(OutputType)' == 'WinExe' or '$(OutputType)' == 'Exe'" Include="/ENTRY:$(EntryPointSymbol) /NOEXP /NOIMPLIB" />
<LinkerArg Include="/NATVIS:&quot;$(MSBuildThisFileDirectory)NativeAOT.natvis&quot;" />
<LinkerArg Condition="'$(ControlFlowGuard)' == 'Guard'" Include="/guard:cf" />
<LinkerArg Condition="'$(OutputType)' == 'WinExe' or '$(OutputType)' == 'Exe'" Include="/STACK:$(IlcDefaultStackSize)" />
</ItemGroup>

<ItemGroup Condition="!Exists('$(IlcSdkPath)debugucrt.txt')">
Expand Down
8 changes: 8 additions & 0 deletions src/coreclr/pal/src/init/pal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,14 @@ InitializeDefaultStackSize()
}
}

#if defined(HOST_OSX) || defined(HOST_MACCATALYST)
// Match Windows stack size
if (g_defaultStackSize == 0)
{
g_defaultStackSize = 1536 * 1024;
}
#endif

#ifdef ENSURE_PRIMARY_STACK_SIZE
if (g_defaultStackSize == 0)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4475,7 +4475,6 @@ public static void DCS_TypeWithPrimitiveKnownTypes()
Assert.NotNull(actual);
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/1417", TestPlatforms.OSX)]
[SkipOnPlatform(TestPlatforms.Browser, "Causes a stack overflow")]
[Fact]
public static void DCS_DeeplyLinkedData()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1223,7 +1223,7 @@ public static void RunLazyCancellationTests_Negative()
}

[Fact]
[ActiveIssue("https://github.com/dotnet/runtime/issues/2084", TestRuntimes.Mono)]
[SkipOnPlatform(TestPlatforms.Browser, "Causes a stack overflow")]
public static void LongContinuationChain_ContinueWith_DoesNotStackOverflow()
{
const int DiveDepth = 12_000;
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);

#if defined(HOST_OSX) || defined(HOST_MACCATALYST)
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
// Match Windows stack size
if (stackSize == 0)
{
stackSize = 1536 * 1024;
}
#endif

if (stackSize > 0)
{
if (stackSize < (uintptr_t)PTHREAD_STACK_MIN)
Expand Down
4 changes: 0 additions & 4 deletions src/tests/JIT/jit64/opt/cse/HugeArray1.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,6 @@
<!-- Needed for GCStressIncompatible -->
<RequiresProcessIsolation>true</RequiresProcessIsolation>
<GCStressIncompatible>true</GCStressIncompatible>

<!-- The test is too complex to compile on macOS where secondary threads have small stack size by default
and that is not enough for Roslyn, see https://github.com/dotnet/runtime/issues/87879 -->
<DisableProjectBuild Condition="'$(HostOS)' == 'osx'">true</DisableProjectBuild>
</PropertyGroup>
<PropertyGroup>
<DebugType>Full</DebugType>
Expand Down
4 changes: 0 additions & 4 deletions src/tests/JIT/jit64/opt/cse/hugeSimpleExpr1.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@
<RequiresProcessIsolation>true</RequiresProcessIsolation>
<!-- Timeout on Arm64 -->
<GCStressIncompatible>true</GCStressIncompatible>

<!-- The test is too complex to compile on macOS where secondary threads have small stack size by default
and that is not enough for Roslyn, see https://github.com/dotnet/runtime/issues/87879 -->
<DisableProjectBuild Condition="'$(HostOS)' == 'osx'">true</DisableProjectBuild>
</PropertyGroup>
<PropertyGroup>
<DebugType>Full</DebugType>
Expand Down
4 changes: 0 additions & 4 deletions src/tests/JIT/jit64/opt/rngchk/RngchkStress2_o.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,6 @@
<PropertyGroup>
<DebugType>PdbOnly</DebugType>
<Optimize>True</Optimize>

<!-- The test is too complex to compile on macOS where secondary threads have small stack size by default
and that is not enough for Roslyn, see https://github.com/dotnet/runtime/issues/87879 -->
<DisableProjectBuild Condition="'$(HostOS)' == 'osx'">true</DisableProjectBuild>
</PropertyGroup>
<ItemGroup>
<Compile Include="RngchkStress2.cs" />
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;
using Xunit;

namespace test87879;

public class test87879
{
[Fact, SkipLocalsInit]
public static void TestEntryPoint()
{
//determine the expected available stack size 1.5MB, minus a little bit (384kB) for overhead.
var expectedSize = 0x180000 - 0x60000;

//allocate on the stack as specified above
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.");
}

[MethodImpl(MethodImplOptions.NoInlining)]
static void Consume(Span<byte> bytes)
{
}
}
12 changes: 12 additions & 0 deletions src/tests/Regressions/coreclr/GitHub_87879/test87879.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
<RequiresProcessIsolation>true</RequiresProcessIsolation>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<CLRTestTargetUnsupported>true</CLRTestTargetUnsupported>
<CLRTestTargetUnsupported Condition="'$(TargetsWindows)' == 'true' OR '$(TargetsMacCatalyst)' == 'true' OR ('$(TargetsUnix)' == 'true' AND '$(TargetsMobile)' != 'true')">false</CLRTestTargetUnsupported>
</PropertyGroup>
<ItemGroup>
<Compile Include="test87879.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<Project Sdk="Microsoft.NET.Sdk">
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
<PropertyGroup>
<!-- Needed for mechanical merging of all remaining tests, this particular project may not actually need process isolation -->
<RequiresProcessIsolation>true</RequiresProcessIsolation>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<UseAppHost>true</UseAppHost>
<CLRTestTargetUnsupported>true</CLRTestTargetUnsupported>
<CLRTestTargetUnsupported Condition="'$(TargetsWindows)' == 'true'">false</CLRTestTargetUnsupported>
hamarb123 marked this conversation as resolved.
Show resolved Hide resolved
<DisableProjectBuild>$(CLRTestTargetUnsupported)</DisableProjectBuild>
</PropertyGroup>
<ItemGroup>
<Compile Include="..\GitHub_87879\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 1.5MB, minus a little bit (384kB) for overhead.
var expectedSize = 0x180000 - 0x60000;

//allocate on the stack as specified above
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 '$(TargetsMacCatalyst)' == 'true' OR ('$(TargetsUnix)' == 'true' AND '$(TargetsMobile)' != 'true')">false</CLRTestTargetUnsupported>
</PropertyGroup>

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