diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index 5dc7e5d34b20bd..525af1eefcd9f9 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -289,7 +289,7 @@ RETAIL_CONFIG_STRING_INFO(EXTERNAL_GCName, W("GCName"), "") RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_ConvertIbcData, W("ConvertIbcData"), 1, "Converts between v1 and v2 IBC data") RETAIL_CONFIG_DWORD_INFO(UNSUPPORTED_DisableIBC, W("DisableIBC"), 0, "Disables the use of IBC data") RETAIL_CONFIG_DWORD_INFO(EXTERNAL_UseIBCFile, W("UseIBCFile"), 0, "") - +RETAIL_CONFIG_STRING_INFO(UNSUPPORTED_ProfileValidationPath, W("ProfileValidationPath"), "Path to save list of methods with polluted profiles to.") /// /// JIT diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index c2fde45f84fc52..be202266f5ee02 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -608,6 +608,8 @@ enum CorInfoHelpFunc CORINFO_HELP_CLASSPROFILE32, // Update 32-bit class profile for a call site CORINFO_HELP_CLASSPROFILE64, // Update 64-bit class profile for a call site + CORINFO_HELP_PROFILE_VALIDATOR, // Notify runtime if a cold block is called + CORINFO_HELP_COUNT, }; diff --git a/src/coreclr/inc/corjitflags.h b/src/coreclr/inc/corjitflags.h index c749e876d2e0fa..f1dfd4353998d6 100644 --- a/src/coreclr/inc/corjitflags.h +++ b/src/coreclr/inc/corjitflags.h @@ -103,7 +103,7 @@ class CORJIT_FLAGS CORJIT_FLAG_UNUSED16 = 43, #endif // !defined(TARGET_ARM) - CORJIT_FLAG_UNUSED17 = 44, + CORJIT_FLAG_VALIDATE_PROFILE = 44, CORJIT_FLAG_UNUSED18 = 45, CORJIT_FLAG_UNUSED19 = 46, CORJIT_FLAG_UNUSED20 = 47, diff --git a/src/coreclr/inc/crsttypes.h b/src/coreclr/inc/crsttypes.h index a1bab2ecb906cf..9ea240b8ee4962 100644 --- a/src/coreclr/inc/crsttypes.h +++ b/src/coreclr/inc/crsttypes.h @@ -134,7 +134,8 @@ enum CrstType CrstUnwindInfoTableLock = 116, CrstVSDIndirectionCellLock = 117, CrstWrapperTemplate = 118, - kNumberOfCrstTypes = 119 + CrstMethodsWithPollutedProfiles = 119, + kNumberOfCrstTypes = 120 }; #endif // __CRST_TYPES_INCLUDED diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 25746b45992942..807fef5635685f 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* 1052f490-cad7-4610-99bb-6f2bd91a1d19 */ - 0x1052f490, - 0xcad7, - 0x4610, - {0x99, 0xbb, 0x6f, 0x2b, 0xd9, 0x1a, 0x1d, 0x19} +constexpr GUID JITEEVersionIdentifier = { /* 64303781-0337-40D2-BFE2-698DF0D110F1 */ + 0x64303781, + 0x337, + 0x40d2, + {0xbf, 0xe2, 0x69, 0x8d, 0xf0, 0xd1, 0x10, 0xf1} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/inc/jithelpers.h b/src/coreclr/inc/jithelpers.h index fb65ea9fa613c7..2da81eb7e904fc 100644 --- a/src/coreclr/inc/jithelpers.h +++ b/src/coreclr/inc/jithelpers.h @@ -359,6 +359,7 @@ JITHELPER(CORINFO_HELP_PATCHPOINT, JIT_Patchpoint, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_CLASSPROFILE32, JIT_ClassProfile32, CORINFO_HELP_SIG_REG_ONLY) JITHELPER(CORINFO_HELP_CLASSPROFILE64, JIT_ClassProfile64, CORINFO_HELP_SIG_REG_ONLY) + JITHELPER(CORINFO_HELP_PROFILE_VALIDATOR, JIT_ProfileValidator, CORINFO_HELP_SIG_REG_ONLY) #undef JITHELPER #undef DYNAMICJITHELPER diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 5f086e1b8c5050..e7eaf2348956bc 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5095,6 +5095,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // Insert GC Polls DoPhase(this, PHASE_INSERT_GC_POLLS, &Compiler::fgInsertGCPolls); + // Instrument cold blocks in order to validate profiles + if (!compileFlags->IsSet(JitFlags::JIT_FLAG_BBINSTR) && compileFlags->IsSet(JitFlags::JIT_FLAG_VALIDATE_PROFILE) && + opts.OptimizationEnabled() && fgFirstBB->hasProfileWeight() && !opts.IsReadyToRun()) + { + DoPhase(this, PHASE_IBCINSTR, &Compiler::fgInsertProfileValidators); + } + // Determine start of cold region if we are hot/cold splitting // DoPhase(this, PHASE_DETERMINE_FIRST_COLD_BLOCK, &Compiler::fgDetermineFirstColdBlock); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 58340e00922036..aed9738612d171 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5728,6 +5728,7 @@ class Compiler PhaseStatus fgPrepareToInstrumentMethod(); PhaseStatus fgInstrumentMethod(); + PhaseStatus fgInsertProfileValidators(); PhaseStatus fgIncorporateProfileData(); void fgIncorporateBlockCounts(); void fgIncorporateEdgeCounts(); diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 1039d303a0d2e3..abe64af228bf26 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -1560,6 +1560,36 @@ PhaseStatus Compiler::fgPrepareToInstrumentMethod() return PhaseStatus::MODIFIED_NOTHING; } +PhaseStatus Compiler::fgInsertProfileValidators() +{ + bool modified = false; + for (BasicBlock* block = fgFirstBB; (block != nullptr); block = block->bbNext) + { + if (block->hasProfileWeight() && (block->getBBWeight(this) == BB_ZERO_WEIGHT)) + { + // TODO: don't emit validator if the block is dominated by other cold blocks + + // Pass CORINFO_METHOD_HANDLE + GenTreeCall::Use* const args = + gtNewCallArgs(gtNewIconNode(reinterpret_cast(info.compMethodHnd), TYP_I_IMPL), + gtNewIconNode(block->bbNum, TYP_INT)); + + GenTree* call = fgMorphCall(gtNewHelperCallNode(CORINFO_HELP_PROFILE_VALIDATOR, TYP_VOID, args)); + gtSetEvalOrder(call); + block->bbFlags |= BBF_HAS_CALL; + + Statement* newStmt = fgNewStmtAtBeg(block, call); + if (fgStmtListThreaded) + { + gtSetStmtInfo(newStmt); + fgSetStmtSeq(newStmt); + } + modified = true; + } + } + return modified ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; +} + //------------------------------------------------------------------------ // fgInstrumentMethod: add instrumentation probes to the method // diff --git a/src/coreclr/jit/jitee.h b/src/coreclr/jit/jitee.h index 27149c824348aa..39653b5e9e27fa 100644 --- a/src/coreclr/jit/jitee.h +++ b/src/coreclr/jit/jitee.h @@ -87,7 +87,7 @@ class JitFlags JIT_FLAG_UNUSED16 = 43, #endif // !defined(TARGET_ARM) - JIT_FLAG_UNUSED17 = 44, + JIT_FLAG_VALIDATE_PROFILE = 44, JIT_FLAG_UNUSED18 = 45, JIT_FLAG_UNUSED19 = 46, JIT_FLAG_UNUSED20 = 47, @@ -212,6 +212,7 @@ class JitFlags FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_REVERSE_PINVOKE, JIT_FLAG_REVERSE_PINVOKE); FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_TIER0, JIT_FLAG_TIER0); FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_TIER1, JIT_FLAG_TIER1); + FLAGS_EQUAL(CORJIT_FLAGS::CORJIT_FLAG_VALIDATE_PROFILE, JIT_FLAG_VALIDATE_PROFILE); #if defined(TARGET_ARM) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs index 6bc8a1999877aa..8fc747464d3e14 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs @@ -297,6 +297,7 @@ which is the right helper to use to allocate an object of a given type. */ CORINFO_HELP_STACK_PROBE, // Probes each page of the allocated stack frame + CORINFO_HELP_PROFILE_VALIDATOR, CORINFO_HELP_COUNT, } } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs index 91114435396e75..f300cdc7e5b669 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs @@ -1378,6 +1378,7 @@ public enum CorJitFlag : uint CORJIT_FLAG_RELATIVE_CODE_RELOCS = 41, // JIT should generate PC-relative address computations instead of EE relocation records CORJIT_FLAG_NO_INLINING = 42, // JIT should not inline any called method into this method CORJIT_FLAG_SOFTFP_ABI = 43, // On ARM should enable armel calling convention + CORJIT_FLAG_VALIDATE_PROFILE = 44, // Instrument cold blocks with counters to validate profiles } public struct CORJIT_FLAGS diff --git a/src/coreclr/vm/appdomain.cpp b/src/coreclr/vm/appdomain.cpp index 8192392cd08f6c..dfc5e0ddaf564c 100644 --- a/src/coreclr/vm/appdomain.cpp +++ b/src/coreclr/vm/appdomain.cpp @@ -2133,6 +2133,7 @@ AppDomain::AppDomain() #endif m_pRefClassFactHash = NULL; + s_methodsWithPollutedProfiles = NULL; m_ForceTrivialWaitOperations = false; m_Stage=STAGE_CREATING; @@ -2230,6 +2231,7 @@ void AppDomain::Init() m_ReflectionCrst.Init(CrstReflection, CRST_UNSAFE_ANYMODE); m_RefClassFactCrst.Init(CrstClassFactInfoHash); + m_MethodsWithPollutedProfileCrst.Init(CrstMethodsWithPollutedProfiles, CRST_UNSAFE_ANYMODE); SetStage(STAGE_READYFORMANAGEDCODE); @@ -2392,6 +2394,82 @@ EEClassFactoryInfoHashTable* AppDomain::SetupClassFactHash() return m_pRefClassFactHash; } +void AppDomain::LogMethodWithPollutedProfile(CORINFO_METHOD_HANDLE method, unsigned bbNum) +{ + CrstHolder ch(&m_MethodsWithPollutedProfileCrst); + + if (s_methodsWithPollutedProfiles == nullptr) + { + NewHolder pCache(new EEPtrHashTable()); + if (!pCache->Init(20, nullptr, nullptr, false)) + return; + s_methodsWithPollutedProfiles = pCache.Extract(); + } + + PTR_VOID key = method; + HashDatum value; + if (s_methodsWithPollutedProfiles->GetValue(key, &value)) + { + // Increment the counter + value = reinterpret_cast(reinterpret_cast(value) + 1); + s_methodsWithPollutedProfiles->ReplaceValue(key, value); + } + else + { + s_methodsWithPollutedProfiles->InsertValue(key, reinterpret_cast(1)); + } +} + +void AppDomain::ListMethodsWithPollutedProfile() +{ + if (s_methodsWithPollutedProfiles == nullptr) + { + return; + } + + LPWSTR filePath = CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_ProfileValidationPath); + if (filePath == nullptr) + { + return; + } + + FILE* file = _wfopen(filePath, W("a")); + if (file == nullptr) + { + return; + } + + CrstHolder ch(&m_MethodsWithPollutedProfileCrst); + + // CSV Format + fprintf(file, "Method name, Cold block hits\n"); + + EEHashTableIteration iter; + s_methodsWithPollutedProfiles->IterateStart(&iter); + BOOL keepGoing = s_methodsWithPollutedProfiles->IterateNext(&iter); + while (keepGoing) + { + auto key = static_cast(s_methodsWithPollutedProfiles->IterateGetKey(&iter)); + if (key != nullptr) + { + MethodDesc* method = GetMethod(key); + if (method != nullptr) + { + HashDatum value = s_methodsWithPollutedProfiles->IterateGetValue(&iter); + const UINT64 counter = reinterpret_cast(value); + + LPCUTF8 nameSpace; + LPCUTF8 scope = method->GetMethodTable()->GetFullyQualifiedNameInfo(&nameSpace); + fprintf(file, "%s::%s, %llu\n", scope, method->GetName(), counter); + } + } + keepGoing = s_methodsWithPollutedProfiles->IterateNext(&iter); + } + delete s_methodsWithPollutedProfiles; + s_methodsWithPollutedProfiles = nullptr; + fclose(file); +} + #ifdef FEATURE_COMINTEROP DispIDCache* AppDomain::SetupRefDispIDCache() { diff --git a/src/coreclr/vm/appdomain.hpp b/src/coreclr/vm/appdomain.hpp index 0487299c5f26c9..8a1e597aa67cf4 100644 --- a/src/coreclr/vm/appdomain.hpp +++ b/src/coreclr/vm/appdomain.hpp @@ -2074,8 +2074,9 @@ class AppDomain : public BaseDomain private: CrstExplicitInit m_ReflectionCrst; CrstExplicitInit m_RefClassFactCrst; + CrstExplicitInit m_MethodsWithPollutedProfileCrst; - + EEPtrHashTable *s_methodsWithPollutedProfiles; EEClassFactoryInfoHashTable *m_pRefClassFactHash; // Hash table that maps a class factory info to a COM comp. #ifdef FEATURE_COMINTEROP DispIDCache *m_pRefDispIDCache; @@ -2091,6 +2092,9 @@ class AppDomain : public BaseDomain return &m_RefClassFactCrst; } + void LogMethodWithPollutedProfile(CORINFO_METHOD_HANDLE method, unsigned bbNum); + void ListMethodsWithPollutedProfile(); + #ifndef DACCESS_COMPILE EEClassFactoryInfoHashTable* GetClassFactHash() { diff --git a/src/coreclr/vm/ceemain.cpp b/src/coreclr/vm/ceemain.cpp index eb31d0f928de5e..e3fb3d41dbc9e5 100644 --- a/src/coreclr/vm/ceemain.cpp +++ b/src/coreclr/vm/ceemain.cpp @@ -1295,6 +1295,8 @@ void STDMETHODCALLTYPE EEShutDownHelper(BOOL fIsDllUnloading) { ClrFlsSetThreadType(ThreadType_Shutdown); + AppDomain::GetCurrentDomain()->ListMethodsWithPollutedProfile(); + if (fIsDllUnloading && g_fEEShutDown) { // I'm in the final shutdown and the first part has already been run. diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index b7f92254c7083b..812a7ddb24a20a 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -5322,6 +5322,15 @@ HCIMPL2(void, JIT_ClassProfile32, Object *obj, void* tableAddress) } HCIMPLEND +HCIMPL2(void, JIT_ProfileValidator, CORINFO_METHOD_HANDLE methodHnd, unsigned bbNum) +{ + FCALL_CONTRACT; + FC_GC_POLL_NOT_NEEDED(); + + AppDomain::GetCurrentDomain()->LogMethodWithPollutedProfile(methodHnd, bbNum); +} +HCIMPLEND + // Version of helper above used when the count is 64-bit HCIMPL2(void, JIT_ClassProfile64, Object *obj, void* tableAddress) { diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index c5363172f79b38..492bdf688c8366 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -12947,6 +12947,12 @@ CORJIT_FLAGS GetCompileFlags(MethodDesc * ftn, CORJIT_FLAGS flags, CORINFO_METHO flags.Set(CORJIT_FLAGS::CORJIT_FLAG_BBOPT); } + // Instrument cold blocks in order to validate profiles. + if (CLRConfig::GetConfigValue(CLRConfig::UNSUPPORTED_ProfileValidationPath) != nullptr) + { + flags.Set(CORJIT_FLAGS::CORJIT_FLAG_VALIDATE_PROFILE); + } + #endif return flags;