Skip to content

Commit

Permalink
Fixes for text-based PGO (#56986)
Browse files Browse the repository at this point in the history
Fix missing indirection when reading in text-based PGO data.

Prioritize reading text-based PGO over dynamic or static PGO data, so that we
can use text to provide a fixed set of PGO data when trying to isolate bugs.

In the jit, give text-based PGO data the same trust level we give to dynamic
PGO data.
  • Loading branch information
AndyAyersMS committed Aug 7, 2021
1 parent f26a143 commit 4803ce2
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 20 deletions.
1 change: 1 addition & 0 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -5849,6 +5849,7 @@ class Compiler
void fgSetProfileWeight(BasicBlock* block, BasicBlock::weight_t weight);
void fgApplyProfileScale();
bool fgHaveSufficientProfileData();
bool fgHaveTrustedProfileData();

// fgIsUsingProfileWeights - returns true if we have real profile data for this method
// or if we have some fake profile data for the stress mode
Expand Down
34 changes: 34 additions & 0 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,37 @@ bool Compiler::fgHaveSufficientProfileData()
return true;
}

//------------------------------------------------------------------------
// fgHaveTrustedProfileData: check if profile data source is one
// that can be trusted to faithfully represent the current program
// behavior.
//
// Returns:
// true if so
//
// Note:
// See notes for fgHaveProfileData.
//
bool Compiler::fgHaveTrustedProfileData()
{
if (!fgHaveProfileData())
{
return false;
}

// We allow Text to be trusted so we can use it to stand in
// for Dynamic results.
//
switch (fgPgoSource)
{
case ICorJitInfo::PgoSource::Dynamic:
case ICorJitInfo::PgoSource::Text:
return true;
default:
return false;
}
}

//------------------------------------------------------------------------
// fgApplyProfileScale: scale inlinee counts by appropriate scale factor
//
Expand Down Expand Up @@ -1796,6 +1827,9 @@ PhaseStatus Compiler::fgIncorporateProfileData()
break;

default:
JITDUMP("Unknown PGO record type 0x%x in schema entry %u (offset 0x%x count 0x%x other 0x%x)\n",
fgPgoSchema[iSchema].InstrumentationKind, iSchema, fgPgoSchema[iSchema].ILOffset,
fgPgoSchema[iSchema].Count, fgPgoSchema[iSchema].Other);
otherRecords++;
break;
}
Expand Down
5 changes: 2 additions & 3 deletions src/coreclr/jit/inlinepolicy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1338,7 +1338,7 @@ void ExtendedDefaultPolicy::NoteInt(InlineObservation obs, int value)
unsigned maxCodeSize = static_cast<unsigned>(JitConfig.JitExtDefaultPolicyMaxIL());

// TODO: Enable for PgoSource::Static as well if it's not the generic profile we bundle.
if (m_HasProfile && (m_RootCompiler->fgPgoSource == ICorJitInfo::PgoSource::Dynamic))
if (m_HasProfile && (m_RootCompiler->fgHaveTrustedProfileData()))
{
maxCodeSize = static_cast<unsigned>(JitConfig.JitExtDefaultPolicyMaxILProf());
}
Expand Down Expand Up @@ -1684,9 +1684,8 @@ double ExtendedDefaultPolicy::DetermineMultiplier()
const double profileTrustCoef = (double)JitConfig.JitExtDefaultPolicyProfTrust() / 10.0;
const double profileScale = (double)JitConfig.JitExtDefaultPolicyProfScale() / 10.0;

if (m_RootCompiler->fgPgoSource == ICorJitInfo::PgoSource::Dynamic)
if (m_RootCompiler->fgHaveTrustedProfileData())
{
// For now we only "trust" dynamic profiles.
multiplier *= (1.0 - profileTrustCoef) + min(m_ProfileFrequency, 1.0) * profileScale;
}
else
Expand Down
42 changes: 25 additions & 17 deletions src/coreclr/vm/pgo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -718,24 +718,11 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca
*pCountSchemaItems = 0;
*pPgoSource = ICorJitInfo::PgoSource::Unknown;

PgoManager *mgr;
if (!pMD->IsDynamicMethod())
{
mgr = pMD->GetLoaderAllocator()->GetPgoManager();
}
else
{
mgr = pMD->AsDynamicMethodDesc()->GetResolver()->GetDynamicPgoManager();
}

HRESULT hr = E_NOTIMPL;
if (mgr != NULL)
{
hr = mgr->getPgoInstrumentationResultsInstance(pMD, pAllocatedData, ppSchema, pCountSchemaItems, pInstrumentationData, pPgoSource);
}

// If not found in the data from the current run, look in the data from the text file
if (FAILED(hr) && s_textFormatPgoData.GetCount() > 0)
// If there is text format PGO data, prefer that over any dynamic or static data.
//
if (s_textFormatPgoData.GetCount() > 0)
{
COUNT_T methodhash = pMD->GetStableHash();
int codehash;
Expand Down Expand Up @@ -796,11 +783,12 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca
*pAllocatedData = new BYTE[schemaArray.GetCount() * sizeof(ICorJitInfo::PgoInstrumentationSchema)];
memcpy(*pAllocatedData, schemaArray.OpenRawBuffer(), schemaArray.GetCount() * sizeof(ICorJitInfo::PgoInstrumentationSchema));
schemaArray.CloseRawBuffer();
*ppSchema = (ICorJitInfo::PgoInstrumentationSchema*)pAllocatedData;
*ppSchema = (ICorJitInfo::PgoInstrumentationSchema*)*pAllocatedData;

*pCountSchemaItems = schemaArray.GetCount();
*pInstrumentationData = found->GetData();
*pPgoSource = ICorJitInfo::PgoSource::Text;

hr = S_OK;
}
EX_CATCH
Expand All @@ -818,6 +806,26 @@ HRESULT PgoManager::getPgoInstrumentationResults(MethodDesc* pMD, BYTE** pAlloca
}
}

// If we didn't find any text format data, look for dynamic or static data.
//
if (FAILED(hr))
{
PgoManager *mgr;
if (!pMD->IsDynamicMethod())
{
mgr = pMD->GetLoaderAllocator()->GetPgoManager();
}
else
{
mgr = pMD->AsDynamicMethodDesc()->GetResolver()->GetDynamicPgoManager();
}

if (mgr != NULL)
{
hr = mgr->getPgoInstrumentationResultsInstance(pMD, pAllocatedData, ppSchema, pCountSchemaItems, pInstrumentationData, pPgoSource);
}
}

return hr;
}

Expand Down

0 comments on commit 4803ce2

Please sign in to comment.