Feedback:

Use different keys from reading from env vs config

Author: Keegan Caruso

src/coreclr/nativeaot/Runtime/RhConfig.cpp

@@ -139,6 +139,27 @@ bool RhConfig::ReadConfigValue(_In_z_ const char *name, uint64_t* pValue, bool d
     return false;
 }
 
+bool RhConfig::ReadConfigValue(_In_z_ const char* envName, _In_z_ const char* configName, uint64_t* pValue, bool decimal)
+{
+    uint64_t uiValue;
+    if (g_pRhConfig->ReadConfigValue(envName, &uiValue))
+    {
+        *pValue = uiValue;
+        return true;
+    }
+
+    if (configName)
+    {
+        if (g_pRhConfig->ReadKnobUInt64Value(configName, &uiValue))
+        {
+            *pValue = uiValue;
+            return true;
+        }
+    }
+
+    return false;
+}
+
 bool RhConfig::ReadKnobUInt64Value(_In_z_ const char *name, uint64_t* pValue)
 {
     const char *embeddedValue = nullptr;

src/coreclr/nativeaot/Runtime/RhConfig.h

@@ -45,6 +45,7 @@ class RhConfig
     };
 
     bool ReadConfigValue(_In_z_ const char* wszName, uint64_t* pValue, bool decimal = false);
+    bool ReadConfigValue(_In_z_ const char* wszEnvName, _In_z_ const char* wszConfigName, uint64_t* pValue, bool decimal = false);
     bool ReadKnobUInt64Value(_In_z_ const char* wszName, uint64_t* pValue);
     bool ReadKnobBooleanValue(_In_z_ const char* wszName, bool* pValue);
 

src/coreclr/nativeaot/Runtime/RhConfigValues.h

@@ -14,7 +14,6 @@ RETAIL_CONFIG_VALUE(StressLogLevel)
 RETAIL_CONFIG_VALUE(TotalStressLogSize)
 RETAIL_CONFIG_VALUE(gcServer)
 RETAIL_CONFIG_VALUE(gcConservative)         // Enables conservative stack reporting
-RETAIL_CONFIG_VALUE(DefaultStackSize)        // Enables setting the default stack size
 DEBUG_CONFIG_VALUE(GcStressThrottleMode)    // gcstm_TriggerAlways / gcstm_TriggerOnFirstHit / gcstm_TriggerRandom
 DEBUG_CONFIG_VALUE(GcStressFreqCallsite)    // Number of times to force GC out of GcStressFreqDenom (for GCSTM_RANDOM)
 DEBUG_CONFIG_VALUE(GcStressFreqLoop)        // Number of times to force GC out of GcStressFreqDenom (for GCSTM_RANDOM)

src/coreclr/nativeaot/Runtime/gcenv.ee.cpp

@@ -768,21 +768,12 @@ bool GCToEEInterface::GetIntConfigValue(const char* privateKey, const char* publ
     }
 
     uint64_t uiValue;
-    if (g_pRhConfig->ReadConfigValue(privateKey, &uiValue))
+    if (g_pRhConfig->ReadConfigValue(privateKey, publicKey, &uiValue))
     {
         *value = uiValue;
         return true;
     }
 
-    if (publicKey)
-    {
-        if (g_pRhConfig->ReadKnobUInt64Value(publicKey, &uiValue))
-        {
-            *value = uiValue;
-            return true;
-        }
-    }
-
     return false;
 }
 

src/coreclr/nativeaot/Runtime/unix/PalRedhawkUnix.cpp

@@ -689,12 +689,14 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalStartBackgroundWork(_In_ BackgroundCall
     const size_t minStack = 0x10000;     // 64K
     const size_t maxStack = 0x80000000;  //  2G
 
-    uint64_t stacksize = g_pRhConfig->GetDefaultStackSize();
-
-    if (stacksize < maxStack && stacksize >= minStack)
+    uint64_t stackSize;
+    if (g_pRhConfig->ReadConfigValue("Threading_DefaultStackSize", "System.Threading.DefaultStackSize", &stackSize))
     {
-        st = pthread_attr_setstacksize(&attrs, (size_t)stacksize);
-        ASSERT(st == 0);
+        if (stacksize < maxStack && stacksize >= minStack)
+        {
+            st = pthread_attr_setstacksize(&attrs, (size_t)stacksize);
+            ASSERT(st == 0);
+        }
     }
 
     static const int NormalPriority = 0;

src/coreclr/nativeaot/Runtime/windows/PalRedhawkMinWin.cpp

@@ -897,11 +897,15 @@ REDHAWK_PALEXPORT bool REDHAWK_PALAPI PalStartBackgroundWork(_In_ BackgroundCall
     const size_t minStack = 0x10000;     // 64K
     const size_t maxStack = 0x80000000;  //  2G
 
-    uint64_t stacksize = g_pRhConfig->GetDefaultStackSize();
+    uint64_t stacksize = 0;
 
-    if (stacksize >= maxStack || stacksize < minStack)
+    uint64_t uiStacksize;
+    if (g_pRhConfig->ReadConfigValue("Threading_DefaultStackSize", "System.Threading.DefaultStackSize", &uiStacksize))
     {
-        stacksize = 0;
+        if (uiStacksize < maxStack || uiStacksize >= minStack)
+        {
+            stacksize = uiStacksize;
+        }
     }
 
     HANDLE hThread = CreateThread(

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Unix.cs

@@ -98,7 +98,7 @@ private unsafe bool CreateThread(GCHandle thisThreadHandle)
 
             if (stackSize <= 0)
             {
-                stackSize = StackSizeFromConfig;
+                stackSize = _stackSizeFromConfig;
             }
 
             if (!Interop.Sys.CreateThread((IntPtr)stackSize, &ThreadEntryPoint, (IntPtr)thisThreadHandle))

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.Windows.cs

@@ -182,7 +182,7 @@ private unsafe bool CreateThread(GCHandle thisThreadHandle)
 
             if (stackSize <= 0)
             {
-                stackSize = StackSizeFromConfig;
+                stackSize = _stackSizeFromConfig;
             }
 
             if ((0 < stackSize) && (stackSize < AllocationGranularity))

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/Thread.NativeAot.cs

@@ -483,36 +483,24 @@ private static void StopThread(Thread thread)
             }
         }
 
-
-        private static int StackSizeFromConfig
-        {
-            get => _stackSizeFromConfig ??= GetDefaultStackSize();
-        }
-
-        private static int? _stackSizeFromConfig;
+        private static int _stackSizeFromConfig = GetDefaultStackSize();
 
         internal static int GetDefaultStackSize()
         {
             // Keep the same arbitrary minimum and maximum from the coreclr\vm layer.
+            // The max was 0x80000000 (2G).
+            // This is checked by the value being parsed into an int32.
             const uint minStack = 0x10000;     // 64K
-            const uint maxStack = 0x80000000;  //  2G
-
-            const string stackSizeVar = "DefaultStackSize";
-            const string stackSizeEnvVar = "DOTNET_DefaultStackSize";
-
-            // Values in RuntimeHostConfigurationOptions get mapped as knobs, knobs get exposed through
-            // the AppContext class.
-            // To have similar behavior as RhConfig, also read from environment variables, which have priority.
 
-            string? valueFromConfig = Environment.GetEnvironmentVariable(stackSizeEnvVar) ?? AppContext.GetData(stackSizeVar)?.ToString();
+            int sizeFromConfig = AppContextConfigHelper.GetInt32Config(
+                "System.Threading.DefaultStackSize",
+                "DOTNET_Threading_DefaultStackSize",
+                0,
+                false);
 
-            if (!string.IsNullOrEmpty(valueFromConfig) &&
-                uint.TryParse(valueFromConfig, NumberStyles.HexNumber, CultureInfo.InvariantCulture, out uint sizeFromConfig) &&
-                sizeFromConfig >= minStack &&
-                sizeFromConfig < maxStack)
+            if (sizeFromConfig >= minStack)
             {
-                // Less than the maxStack size means it will fit in a signed int32.
-                return (int)sizeFromConfig;
+                return sizeFromConfig;
             }
 
             return 0;