Reflect PR feedback

diff --git a/src/gc/env/gcenv.base.h b/src/gc/env/gcenv.base.h
index 35a8a0b..614b84a 100644 (file)
--- a/src/gc/env/gcenv.base.h
+++ b/src/gc/env/gcenv.base.h
@@ -72,6 +72,7 @@ inline HRESULT HRESULT_FROM_WIN32(unsigned long x)
 #define E_OUTOFMEMORY           0x8007000E
 #define COR_E_EXECUTIONENGINE   0x80131506
 #define CLR_E_GC_BAD_AFFINITY_CONFIG 0x8013200A
+#define CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT 0x8013200B
 
 #define NOERROR                 0x0
 #define ERROR_TIMEOUT           1460

diff --git a/src/gc/env/gcenv.os.h b/src/gc/env/gcenv.os.h
index f4dddb8..323e193 100644 (file)
--- a/src/gc/env/gcenv.os.h
+++ b/src/gc/env/gcenv.os.h
@@ -336,13 +336,13 @@ public:
     // Check if the OS supports getting current processor number
     static bool CanGetCurrentProcessorNumber();
 
-    // Add ideal processor for the current thread
+    // Set ideal processor for the current thread
     // Parameters:
     //  srcProcNo - processor number the thread currently runs on
     //  dstProcNo - processor number the thread should be migrated to
     // Return:
     //  true if it has succeeded, false if it has failed
-    static bool MigrateThread(uint16_t srcProcNo, uint16_t dstProcNo);
+    static bool SetCurrentThreadIdealAffinity(uint16_t srcProcNo, uint16_t dstProcNo);
 
     // Get numeric id of the current thread if possible on the
     // current platform. It is indended for logging purposes only.

diff --git a/src/gc/gc.cpp b/src/gc/gc.cpp
index 222f144..910ff07 100644 (file)
--- a/src/gc/gc.cpp
+++ b/src/gc/gc.cpp
@@ -13540,7 +13540,7 @@ try_again:
                         uint16_t src_proc_no = heap_select::find_proc_no_from_heap_no(org_hp->heap_number);
                         uint16_t dst_proc_no = heap_select::find_proc_no_from_heap_no(max_hp->heap_number);
 
-                        if (!GCToOSInterface::MigrateThread(src_proc_no, dst_proc_no))
+                        if (!GCToOSInterface::SetCurrentThreadIdealAffinity(src_proc_no, dst_proc_no))
                         {
                             dprintf (3, ("Failed to set the ideal processor for heap %d.",
                                         org_hp->heap_number));
@@ -34086,66 +34086,9 @@ HRESULT GCHeap::Initialize()
 
 #ifdef MULTIPLE_HEAPS
     AffinitySet config_affinity_set;
-
-    // Get the affinity set configured by the user
-    uintptr_t heap_affinity_mask = GCConfig::GetGCHeapAffinitizeMask();
-    if (heap_affinity_mask != 0)
+    if (!ParseGCHeapAffinitySettings(&config_affinity_set))
     {
-        for (size_t i = 0; i < 8 * sizeof(uintptr_t); i++)
-        {
-            if (heap_affinity_mask & ((uintptr_t)1 << i))
-            {
-                config_affinity_set.Add(i);
-            }
-        }
-    }
-    else
-    {
-        GCConfigStringHolder cpu_index_ranges_holder(GCConfig::GetGCHeapAffinitizeRanges());
-        const char* cpu_index_ranges = cpu_index_ranges_holder.Get();
-
-        // The cpu index ranges is a comma separated list of indices or ranges of indices (e.g. 1-5).
-        // Example 1,3,5,7-9,12
-
-        if (cpu_index_ranges != NULL)
-        {
-            char* number_end;
-
-            do
-            {
-                size_t start_index = strtoul(cpu_index_ranges, &number_end, 10);
-
-                if (number_end == cpu_index_ranges)
-                {
-                    // No number found, invalid format
-                    break;
-                }
-
-                size_t end_index = start_index;
-
-                if (*number_end == '-')
-                {
-                    char* range_end_start = number_end + 1;
-                    end_index = strtoul(range_end_start, &number_end, 10);
-                    if (number_end == range_end_start)
-                    {
-                        // No number found, invalid format
-                        break;
-                    }
-                }
-
-                if ((start_index < MAX_SUPPORTED_CPUS) && end_index < (MAX_SUPPORTED_CPUS))
-                {
-                    for (size_t i = start_index; i <= end_index; i++)
-                    {
-                        config_affinity_set.Add(i);
-                    }
-                }
-
-                cpu_index_ranges = number_end + 1;
-            }
-            while (*number_end == ',');
-        }
+        return CLR_E_GC_BAD_AFFINITY_CONFIG;
     }
 
     AffinitySet* process_affinity_set = GCToOSInterface::GetCurrentProcessAffinitySet();
@@ -34164,7 +34107,7 @@ HRESULT GCHeap::Initialize()
 
     if (process_affinity_set->IsEmpty())
     {
-        return CLR_E_GC_BAD_AFFINITY_CONFIG;
+        return CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT;
     }
 
     nhp_from_config = static_cast<uint32_t>(GCConfig::GetHeapCount());

diff --git a/src/gc/gcconfig.cpp b/src/gc/gcconfig.cpp
index d84a5a5..1e9dbe1 100644 (file)
--- a/src/gc/gcconfig.cpp
+++ b/src/gc/gcconfig.cpp
@@ -46,3 +46,73 @@ GC_CONFIGURATION_KEYS
 #undef BOOL_CONFIG
 #undef INT_CONFIG
 }
+
+bool ParseGCHeapAffinitySettings(AffinitySet* config_affinity_set)
+{
+    bool success = true;
+
+    // Get the affinity set configured by the user
+    uintptr_t heap_affinity_mask = GCConfig::GetGCHeapAffinitizeMask();
+    if (heap_affinity_mask != 0)
+    {
+        for (size_t i = 0; i < 8 * sizeof(uintptr_t); i++)
+        {
+            if (heap_affinity_mask & ((uintptr_t)1 << i))
+            {
+                config_affinity_set->Add(i);
+            }
+        }
+    }
+    else
+    {
+        GCConfigStringHolder cpu_index_ranges_holder(GCConfig::GetGCHeapAffinitizeRanges());
+        const char* cpu_index_ranges = cpu_index_ranges_holder.Get();
+
+        // The cpu index ranges is a comma separated list of indices or ranges of indices (e.g. 1-5).
+        // Example 1,3,5,7-9,12
+
+        if (cpu_index_ranges != NULL)
+        {
+            char* number_end;
+
+            do
+            {
+                size_t start_index = strtoul(cpu_index_ranges, &number_end, 10);
+
+                if (number_end == cpu_index_ranges)
+                {
+                    // No number found, invalid format
+                    break;
+                }
+
+                size_t end_index = start_index;
+
+                if (*number_end == '-')
+                {
+                    char* range_end_start = number_end + 1;
+                    end_index = strtoul(range_end_start, &number_end, 10);
+                    if (number_end == range_end_start)
+                    {
+                        // No number found, invalid format
+                        break;
+                    }
+                }
+
+                if ((start_index < MAX_SUPPORTED_CPUS) && end_index < (MAX_SUPPORTED_CPUS))
+                {
+                    for (size_t i = start_index; i <= end_index; i++)
+                    {
+                        config_affinity_set->Add(i);
+                    }
+                }
+
+                cpu_index_ranges = number_end + 1;
+            }
+            while (*number_end == ',');
+
+            success = (*number_end == '\0');
+        }
+    }
+
+    return success;
+}

diff --git a/src/gc/gcconfig.h b/src/gc/gcconfig.h
index c3c0d5d..f292a12 100644 (file)
--- a/src/gc/gcconfig.h
+++ b/src/gc/gcconfig.h
@@ -96,7 +96,8 @@ public:
   INT_CONFIG(GCHeapAffinitizeMask, "GCHeapAffinitizeMask", 0,                                  \
       "Specifies processor mask for Server GC threads")                                        \
   STRING_CONFIG(GCHeapAffinitizeRanges, "GCHeapAffinitizeRanges",                              \
-      "Specifies list of processors for Server GC threads")                                    \
+      "Specifies list of processors for Server GC threads. The format is a comma separated "   \
+      "list of processor numbers or ranges of processor numbers. Example: 1,3,5,7-9,12")       \
   INT_CONFIG(GCHighMemPercent, "GCHighMemPercent", 0,                                          \
       "The percent for GC to consider as high memory")                                         \
   INT_CONFIG(GCProvModeStress, "GCProvModeStress", 0,                                          \
@@ -155,4 +156,6 @@ static void Initialize();
 
 };
 
+bool ParseGCHeapAffinitySettings(AffinitySet* config_affinity_set);
+
 #endif // __GCCONFIG_H__

diff --git a/src/gc/unix/gcenv.unix.cpp b/src/gc/unix/gcenv.unix.cpp
index 4ab9e09..140c546 100644 (file)
--- a/src/gc/unix/gcenv.unix.cpp
+++ b/src/gc/unix/gcenv.unix.cpp
@@ -210,7 +210,7 @@ uint32_t GCToOSInterface::GetCurrentProcessId()
 //  dstProcNo - processor number the thread should be migrated to
 // Return:
 //  true if it has succeeded, false if it has failed
-bool GCToOSInterface::MigrateThread(uint16_t srcProcNo, uint16_t dstProcNo)
+bool GCToOSInterface::SetCurrentThreadIdealAffinity(uint16_t srcProcNo, uint16_t dstProcNo)
 {
     return GCToOSInterface::SetThreadAffinity(dstProcNo);
 }

diff --git a/src/gc/windows/gcenv.windows.cpp b/src/gc/windows/gcenv.windows.cpp
index 100b218..738fdec 100644 (file)
--- a/src/gc/windows/gcenv.windows.cpp
+++ b/src/gc/windows/gcenv.windows.cpp
@@ -562,7 +562,7 @@ uint32_t GCToOSInterface::GetCurrentProcessId()
 //  dstProcNo - processor number the thread should be migrated to
 // Return:
 //  true if it has succeeded, false if it has failed
-bool GCToOSInterface::MigrateThread(uint16_t srcProcNo, uint16_t dstProcNo)
+bool GCToOSInterface::SetCurrentThreadIdealAffinity(uint16_t srcProcNo, uint16_t dstProcNo)
 {
     LIMITED_METHOD_CONTRACT;
 

diff --git a/src/inc/clrconfigvalues.h b/src/inc/clrconfigvalues.h
index ef55dde..c8f7461 100644 (file)
--- a/src/inc/clrconfigvalues.h
+++ b/src/inc/clrconfigvalues.h
@@ -329,6 +329,7 @@ RETAIL_CONFIG_DWORD_INFO(EXTERNAL_GCHighMemPercent, W("GCHighMemPercent"), 0, "S
 RETAIL_CONFIG_STRING_INFO(EXTERNAL_GCName, W("GCName"), "")
 RETAIL_CONFIG_DWORD_INFO_DIRECT_ACCESS(EXTERNAL_GCHeapHardLimit, W("GCHeapHardLimit"), "Specifies the maximum commit size for the GC heap")
 RETAIL_CONFIG_DWORD_INFO_DIRECT_ACCESS(EXTERNAL_GCHeapHardLimitPercent, W("GCHeapHardLimitPercent"), "Specifies the GC heap usage as a percentage of the total memory")
+RETAIL_CONFIG_STRING_INFO(EXTERNAL_GCHeapAffinitizeRanges, W("GCHeapAffinitizeRanges"), "Specifies list of processors for Server GC threads. The format is a comma separated list of processor numbers or ranges of processor numbers. Example: 1,3,5,7-9,12")
 
 ///
 /// IBC

diff --git a/src/inc/corerror.xml b/src/inc/corerror.xml
index bd8b626..3b7551b 100644 (file)
--- a/src/inc/corerror.xml
+++ b/src/inc/corerror.xml
@@ -2237,10 +2237,16 @@
 
 <HRESULT NumericValue="0x8013200A">
        <SymbolicName>CLR_E_GC_BAD_AFFINITY_CONFIG</SymbolicName>
-       <Message>"GC affinity mask or set doesn't contain any CPUs the current process is affinitized to."</Message>
+       <Message>"GCHeapAffinitizeMask or GCHeapAffinitizeRanges didn't specify any CPUs the current process is affinitized to."</Message>
        <Comment>During a GC initialization, the affinity mask specified via GCHeapAffinitizeMask or GCHeapAffinitizeRanges didn't contain any CPUs the current process is affinitized to.</Comment>
 </HRESULT>
 
+<HRESULT NumericValue="0x8013200B">
+       <SymbolicName>CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT</SymbolicName>
+       <Message>"GCHeapAffinitizeRanges configuration string has invalid format."</Message>
+       <Comment>During a GC initialization, the GCHeapAffinitizeRanges config couldn't be parsed due to its invalid format.</Comment>
+</HRESULT>
+
 <HRESULT NumericValue="E_ACCESSDENIED">
        <SymbolicName>COR_E_UNAUTHORIZEDACCESS</SymbolicName>
        <Comment> 0x80070005 // Access is denied.</Comment>

diff --git a/src/pal/prebuilt/corerror/mscorurt.rc b/src/pal/prebuilt/corerror/mscorurt.rc
index 87d3f0b..5b0a9e2 100644 (file)
--- a/src/pal/prebuilt/corerror/mscorurt.rc
+++ b/src/pal/prebuilt/corerror/mscorurt.rc
@@ -325,6 +325,7 @@ BEGIN
        MSG_FOR_URT_HR(CLR_E_BIND_NI_SECURITY_FAILURE) "Native image was generated in a different trust level than present at runtime"
        MSG_FOR_URT_HR(CLR_E_BIND_NI_DEP_IDENTITY_MISMATCH) "Native image identity mismatch with respect to its dependencies"
        MSG_FOR_URT_HR(CLR_E_GC_OOM) "Failfast due to an OOM during a GC"
-       MSG_FOR_URT_HR(CLR_E_GC_BAD_AFFINITY_CONFIG) "GC affinity mask or set doesn't contain any CPUs the current process is affinitized to."
+       MSG_FOR_URT_HR(CLR_E_GC_BAD_AFFINITY_CONFIG) "GCHeapAffinitizeMask or GCHeapAffinitizeRanges didn't specify any CPUs the current process is affinitized to."
+       MSG_FOR_URT_HR(CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT) "GCHeapAffinitizeRanges configuration string has invalid format."
        MSG_FOR_URT_HR(COR_E_BADIMAGEFORMAT) "The format of a DLL or executable being loaded is invalid."
 END

diff --git a/src/pal/prebuilt/inc/corerror.h b/src/pal/prebuilt/inc/corerror.h
index 547ac53..be2d5fd 100644 (file)
--- a/src/pal/prebuilt/inc/corerror.h
+++ b/src/pal/prebuilt/inc/corerror.h
@@ -392,6 +392,7 @@
 #define CLR_E_BIND_NI_DEP_IDENTITY_MISMATCH EMAKEHR(0x2008)
 #define CLR_E_GC_OOM EMAKEHR(0x2009)
 #define CLR_E_GC_BAD_AFFINITY_CONFIG EMAKEHR(0x200a)
+#define CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT EMAKEHR(0x200b)
 #define COR_E_UNAUTHORIZEDACCESS E_ACCESSDENIED
 #define COR_E_ARGUMENT E_INVALIDARG
 #define COR_E_INVALIDCAST E_NOINTERFACE

diff --git a/src/vm/gcenv.os.cpp b/src/vm/gcenv.os.cpp
index 1c9cb04..fcf716f 100644 (file)
--- a/src/vm/gcenv.os.cpp
+++ b/src/vm/gcenv.os.cpp
@@ -114,7 +114,7 @@ uint32_t GCToOSInterface::GetCurrentProcessId()
 //  dstProcNo - processor number the thread should be migrated to
 // Return:
 //  true if it has succeeded, false if it has failed
-bool GCToOSInterface::MigrateThread(uint16_t srcProcNo, uint16_t dstProcNo)
+bool GCToOSInterface::SetCurrentThreadIdealAffinity(uint16_t srcProcNo, uint16_t dstProcNo)
 {
     LIMITED_METHOD_CONTRACT;