From: Jan Vorlicek Date: Fri, 29 Mar 2019 11:46:55 +0000 (+0100) Subject: Reflect PR feedback X-Git-Tag: accepted/tizen/unified/20190813.215958~53^2~8^2~3 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=94359a5208206be08ff44a1fc14e0334e2af2071;p=platform%2Fupstream%2Fcoreclr.git Reflect PR feedback --- diff --git a/src/gc/env/gcenv.base.h b/src/gc/env/gcenv.base.h index 35a8a0b..614b84a 100644 --- 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 --- 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 --- 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(GCConfig::GetHeapCount()); diff --git a/src/gc/gcconfig.cpp b/src/gc/gcconfig.cpp index d84a5a5..1e9dbe1 100644 --- 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 --- 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 --- 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 --- 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 --- 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 --- a/src/inc/corerror.xml +++ b/src/inc/corerror.xml @@ -2237,10 +2237,16 @@ CLR_E_GC_BAD_AFFINITY_CONFIG - "GC affinity mask or set doesn't contain any CPUs the current process is affinitized to." + "GCHeapAffinitizeMask or GCHeapAffinitizeRanges didn't specify any CPUs the current process is affinitized to." During a GC initialization, the affinity mask specified via GCHeapAffinitizeMask or GCHeapAffinitizeRanges didn't contain any CPUs the current process is affinitized to. + + CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT + "GCHeapAffinitizeRanges configuration string has invalid format." + During a GC initialization, the GCHeapAffinitizeRanges config couldn't be parsed due to its invalid format. + + COR_E_UNAUTHORIZEDACCESS 0x80070005 // Access is denied. diff --git a/src/pal/prebuilt/corerror/mscorurt.rc b/src/pal/prebuilt/corerror/mscorurt.rc index 87d3f0b..5b0a9e2 100644 --- 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 --- 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 --- 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;