Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Streamlined MinOpts GC ref tracking. #9869

Merged
merged 1 commit into from
Mar 1, 2017
Merged

Streamlined MinOpts GC ref tracking. #9869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/jit/codegenarm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1918,12 +1918,14 @@ void CodeGen::genCreateAndStoreGCInfo(unsigned codeSize,
// Follow the code pattern of the x86 gc info encoder (genCreateAndStoreGCInfoJIT32).
gcInfo.gcInfoBlockHdrSave(gcInfoEncoder, codeSize, prologSize);

// We keep the call count for the second call to gcMakeRegPtrTable() below.
unsigned callCnt = 0;
// First we figure out the encoder ID's for the stack slots and registers.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS, &callCnt);
// Now we've requested all the slots we'll need; "finalize" these (make more compact data structures for them).
gcInfoEncoder->FinalizeSlotIds();
// Now we can actually use those slot ID's to declare live ranges.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK, &callCnt);

gcInfoEncoder->Build();

Expand Down
7 changes: 5 additions & 2 deletions src/jit/codegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6061,14 +6061,17 @@ void CodeGen::genCreateAndStoreGCInfoX64(unsigned codeSize, unsigned prologSize
// Follow the code pattern of the x86 gc info encoder (genCreateAndStoreGCInfoJIT32).
gcInfo.gcInfoBlockHdrSave(gcInfoEncoder, codeSize, prologSize);

// We keep the call count for the second call to gcMakeRegPtrTable() below.
unsigned callCnt = 0;

// First we figure out the encoder ID's for the stack slots and registers.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS, &callCnt);

// Now we've requested all the slots we'll need; "finalize" these (make more compact data structures for them).
gcInfoEncoder->FinalizeSlotIds();

// Now we can actually use those slot ID's to declare live ranges.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK, &callCnt);

if (compiler->opts.compDbgEnC)
{
Expand Down
6 changes: 4 additions & 2 deletions src/jit/codegenlegacy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20164,12 +20164,14 @@ void CodeGen::genCreateAndStoreGCInfoX64(unsigned codeSize, unsigned prologSize
// Follow the code pattern of the x86 gc info encoder (genCreateAndStoreGCInfoJIT32).
gcInfo.gcInfoBlockHdrSave(gcInfoEncoder, codeSize, prologSize);

// We keep the call count for the second call to gcMakeRegPtrTable() below.
unsigned callCnt = 0;
// First we figure out the encoder ID's for the stack slots and registers.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS, &callCnt);
// Now we've requested all the slots we'll need; "finalize" these (make more compact data structures for them).
gcInfoEncoder->FinalizeSlotIds();
// Now we can actually use those slot ID's to declare live ranges.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK, &callCnt);

gcInfoEncoder->Build();

Expand Down
6 changes: 4 additions & 2 deletions src/jit/codegenxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8361,12 +8361,14 @@ void CodeGen::genCreateAndStoreGCInfoX64(unsigned codeSize, unsigned prologSize
// Follow the code pattern of the x86 gc info encoder (genCreateAndStoreGCInfoJIT32).
gcInfo.gcInfoBlockHdrSave(gcInfoEncoder, codeSize, prologSize);

// We keep the call count for the second call to gcMakeRegPtrTable() below.
unsigned callCnt = 0;
// First we figure out the encoder ID's for the stack slots and registers.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS, &callCnt);
// Now we've requested all the slots we'll need; "finalize" these (make more compact data structures for them).
gcInfoEncoder->FinalizeSlotIds();
// Now we can actually use those slot ID's to declare live ranges.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK);
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_DO_WORK, &callCnt);

if (compiler->opts.compDbgEnC)
{
Expand Down
97 changes: 69 additions & 28 deletions src/jit/gcencode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3844,13 +3844,15 @@ struct InterruptibleRangeReporter
}
};

void GCInfo::gcMakeRegPtrTable(GcInfoEncoder* gcInfoEncoder,
unsigned codeSize,
unsigned prologSize,
MakeRegPtrMode mode)
void GCInfo::gcMakeRegPtrTable(
GcInfoEncoder* gcInfoEncoder, unsigned codeSize, unsigned prologSize, MakeRegPtrMode mode, unsigned* callCntRef)
{
GCENCODER_WITH_LOGGING(gcInfoEncoderWithLog, gcInfoEncoder);

const bool noTrackedGCSlots =
(compiler->opts.MinOpts() && !compiler->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_PREJIT) &&
!JitConfig.JitMinOptsTrackGCrefs());

if (mode == MAKE_REG_PTR_MODE_ASSIGN_SLOTS)
{
m_regSlotMap = new (compiler->getAllocator()) RegSlotMap(compiler->getAllocator());
Expand Down Expand Up @@ -3961,14 +3963,25 @@ void GCInfo::gcMakeRegPtrTable(GcInfoEncoder* gcInfoEncoder,
{
stackSlotBase = GC_FRAMEREG_REL;
}
StackSlotIdKey sskey(varDsc->lvStkOffs, (stackSlotBase == GC_FRAMEREG_REL), flags);
GcSlotId varSlotId;
if (mode == MAKE_REG_PTR_MODE_ASSIGN_SLOTS)
if (noTrackedGCSlots)
{
if (!m_stackSlotMap->Lookup(sskey, &varSlotId))
// No need to hash/lookup untracked GC refs; just grab a new Slot Id.
if (mode == MAKE_REG_PTR_MODE_ASSIGN_SLOTS)
{
varSlotId = gcInfoEncoderWithLog->GetStackSlotId(varDsc->lvStkOffs, flags, stackSlotBase);
m_stackSlotMap->Set(sskey, varSlotId);
gcInfoEncoderWithLog->GetStackSlotId(varDsc->lvStkOffs, flags, stackSlotBase);
}
}
else
{
StackSlotIdKey sskey(varDsc->lvStkOffs, (stackSlotBase == GC_FRAMEREG_REL), flags);
GcSlotId varSlotId;
if (mode == MAKE_REG_PTR_MODE_ASSIGN_SLOTS)
{
if (!m_stackSlotMap->Lookup(sskey, &varSlotId))
{
varSlotId = gcInfoEncoderWithLog->GetStackSlotId(varDsc->lvStkOffs, flags, stackSlotBase);
m_stackSlotMap->Set(sskey, varSlotId);
}
}
}
}
Expand Down Expand Up @@ -4204,9 +4217,24 @@ void GCInfo::gcMakeRegPtrTable(GcInfoEncoder* gcInfoEncoder,
{
if (gcCallDescList != nullptr)
{
for (CallDsc* call = gcCallDescList; call != nullptr; call = call->cdNext)
if (noTrackedGCSlots)
{
numCallSites++;
// We have the call count from the previous run.
numCallSites = *callCntRef;

// If there are no calls, tell the world and bail.
if (numCallSites == 0)
{
gcInfoEncoderWithLog->DefineCallSites(nullptr, nullptr, 0);
return;
}
}
else
{
for (CallDsc* call = gcCallDescList; call != nullptr; call = call->cdNext)
{
numCallSites++;
}
}
pCallSites = new (compiler, CMK_GC) unsigned[numCallSites];
pCallSiteSizes = new (compiler, CMK_GC) BYTE[numCallSites];
Expand All @@ -4216,17 +4244,8 @@ void GCInfo::gcMakeRegPtrTable(GcInfoEncoder* gcInfoEncoder,
// Now consider every call.
for (CallDsc* call = gcCallDescList; call != nullptr; call = call->cdNext)
{
if (mode == MAKE_REG_PTR_MODE_DO_WORK)
{
pCallSites[callSiteNum] = call->cdOffs - call->cdCallInstrSize;
pCallSiteSizes[callSiteNum] = call->cdCallInstrSize;
callSiteNum++;
}

unsigned nextOffset;

// Figure out the code offset of this entry.
nextOffset = call->cdOffs;
unsigned nextOffset = call->cdOffs;

// As far as I (DLD, 2010) can determine by asking around, the "call->u1.cdArgMask"
// and "cdArgCnt" cases are to handle x86 situations in which a call expression is nested as an
Expand All @@ -4251,13 +4270,35 @@ void GCInfo::gcMakeRegPtrTable(GcInfoEncoder* gcInfoEncoder,
assert(call->cdOffs >= call->cdCallInstrSize);
// call->cdOffs is actually the offset of the instruction *following* the call, so subtract
// the call instruction size to get the offset of the actual call instruction...
unsigned callOffset = call->cdOffs - call->cdCallInstrSize;
// Record that these registers are live before the call...
gcInfoRecordGCRegStateChange(gcInfoEncoder, mode, callOffset, regMask, GC_SLOT_LIVE, byrefRegMask, nullptr);
// ...and dead after.
gcInfoRecordGCRegStateChange(gcInfoEncoder, mode, call->cdOffs, regMask, GC_SLOT_DEAD, byrefRegMask,
nullptr);
unsigned callOffset = nextOffset - call->cdCallInstrSize;

if (noTrackedGCSlots && regMask == 0)
{
// No live GC refs in regs at the call -> don't record the call.
}
else
{
// Append an entry for the call if doing the real thing.
if (mode == MAKE_REG_PTR_MODE_DO_WORK)
{
pCallSites[callSiteNum] = callOffset;
pCallSiteSizes[callSiteNum] = call->cdCallInstrSize;
}
callSiteNum++;

// Record that these registers are live before the call...
gcInfoRecordGCRegStateChange(gcInfoEncoder, mode, callOffset, regMask, GC_SLOT_LIVE, byrefRegMask,
nullptr);
// ...and dead after.
gcInfoRecordGCRegStateChange(gcInfoEncoder, mode, nextOffset, regMask, GC_SLOT_DEAD, byrefRegMask,
nullptr);
}
}
// Make sure we've recorded the expected number of calls
assert(mode != MAKE_REG_PTR_MODE_DO_WORK || numCallSites == callSiteNum);
// Return the actual recorded call count to the caller
*callCntRef = callSiteNum;

// OK, define the call sites.
if (mode == MAKE_REG_PTR_MODE_DO_WORK)
{
Expand Down
9 changes: 9 additions & 0 deletions src/jit/jitconfigvalues.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,15 @@ CONFIG_INTEGER(JitEnableNoWayAssert, W("JitEnableNoWayAssert"), 0)
CONFIG_INTEGER(JitEnableNoWayAssert, W("JitEnableNoWayAssert"), 1)
#endif // !defined(DEBUG) && !defined(_DEBUG)

#if !defined(JIT32_GCENCODER)
#if defined(_TARGET_AMD64_) && defined(FEATURE_CORECLR)
#define JitMinOptsTrackGCrefs_Default 0 // Not tracking GC refs in MinOpts is new behavior
#else
#define JitMinOptsTrackGCrefs_Default 1
#endif
CONFIG_INTEGER(JitMinOptsTrackGCrefs, W("JitMinOptsTrackGCrefs"), JitMinOptsTrackGCrefs_Default) // Track GC roots
#endif // !defined(JIT32_GCENCODER)

// The following should be wrapped inside "#if MEASURE_MEM_ALLOC / #endif", but
// some files include this one without bringing in the definitions from "jit.h"
// so we don't always know what the "true" value of that flag should be. For now
Expand Down
6 changes: 5 additions & 1 deletion src/jit/jitgcinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,11 @@ class GCInfo
// references, building up mappings from tuples of <reg/offset X byref/pinning> to the corresponding
// slot id (in the two member fields declared above). In the "do work" mode, we use these slot ids to
// actually declare live ranges to the encoder.
void gcMakeRegPtrTable(GcInfoEncoder* gcInfoEncoder, unsigned codeSize, unsigned prologSize, MakeRegPtrMode mode);
void gcMakeRegPtrTable(GcInfoEncoder* gcInfoEncoder,
unsigned codeSize,
unsigned prologSize,
MakeRegPtrMode mode,
unsigned* callCntRef);
#endif

#ifdef JIT32_GCENCODER
Expand Down
4 changes: 4 additions & 0 deletions src/jit/lclvars.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3023,6 +3023,10 @@ void Compiler::lvaSortByRefCount()
lvaSetVarDoNotEnregister(lclNum DEBUGARG(DNER_PinningRef));
#endif
}
else if (opts.MinOpts() && !JitConfig.JitMinOptsTrackGCrefs() && varTypeIsGC(varDsc->TypeGet()))
{
varDsc->lvTracked = 0;
}

// Are we not optimizing and we have exception handlers?
// if so mark all args and locals "do not enregister".
Expand Down
60 changes: 33 additions & 27 deletions src/vm/gcinfodecoder.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,6 +628,8 @@ bool GcInfoDecoder::EnumerateLiveSlots(


#ifdef PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED
bool noTrackedRefs = false;

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CodegenMirror branch only mirrors the JIT directory not the VM (or inc) directories.
So the VM changes don't come over and won't automatically be part of any new Desktop releases.

If the VM changes rae required for correctness then they should be checked in to the CodegenMirror branch shortly after your PR goes through the mirror.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the heads up, @briansull - hopefully we'll be able to complete this thing quick this time around (it's almost exactly the same as last time, with one simplification that also fixes a weird problem that cropped up in one CoreFX test, BTW - @BruceForstall PTAL when you get a chance) and then I'll jump on the CGmirror. By default it should stay off for the desktop build so it shouldn't break anything.

if(m_SafePointIndex < m_NumSafePoints && !executionAborted)
{
// Skip interruptibility information
Expand All @@ -648,33 +650,40 @@ bool GcInfoDecoder::EnumerateLiveSlots(
//
if(!executionAborted)
{
_ASSERTE(m_NumInterruptibleRanges);
if(m_NumInterruptibleRanges == 0)
{
// No ranges and no explicit safepoint - must be MinOpts with untracked refs.
noTrackedRefs = true;
}
}

int countIntersections = 0;
UINT32 lastNormStop = 0;
for(UINT32 i=0; i<m_NumInterruptibleRanges; i++)
if(m_NumInterruptibleRanges != 0)
{
UINT32 normStartDelta = (UINT32) m_Reader.DecodeVarLengthUnsigned( INTERRUPTIBLE_RANGE_DELTA1_ENCBASE );
UINT32 normStopDelta = (UINT32) m_Reader.DecodeVarLengthUnsigned( INTERRUPTIBLE_RANGE_DELTA2_ENCBASE ) + 1;
int countIntersections = 0;
UINT32 lastNormStop = 0;
for(UINT32 i=0; i<m_NumInterruptibleRanges; i++)
{
UINT32 normStartDelta = (UINT32) m_Reader.DecodeVarLengthUnsigned( INTERRUPTIBLE_RANGE_DELTA1_ENCBASE );
UINT32 normStopDelta = (UINT32) m_Reader.DecodeVarLengthUnsigned( INTERRUPTIBLE_RANGE_DELTA2_ENCBASE ) + 1;

UINT32 normStart = lastNormStop + normStartDelta;
UINT32 normStop = normStart + normStopDelta;
if(normBreakOffset >= normStart && normBreakOffset < normStop)
UINT32 normStart = lastNormStop + normStartDelta;
UINT32 normStop = normStart + normStopDelta;
if(normBreakOffset >= normStart && normBreakOffset < normStop)
{
_ASSERTE(pseudoBreakOffset == 0);
countIntersections++;
pseudoBreakOffset = numInterruptibleLength + normBreakOffset - normStart;
}
numInterruptibleLength += normStopDelta;
lastNormStop = normStop;
}
_ASSERTE(countIntersections <= 1);
if(countIntersections == 0)
{
_ASSERTE(pseudoBreakOffset == 0);
countIntersections++;
pseudoBreakOffset = numInterruptibleLength + normBreakOffset - normStart;
_ASSERTE(executionAborted);
LOG((LF_GCROOTS, LL_INFO100000, "Not reporting this frame because it is aborted and not fully interruptible.\n"));
goto ExitSuccess;
}
numInterruptibleLength += normStopDelta;
lastNormStop = normStop;
}
_ASSERTE(countIntersections <= 1);
if(countIntersections == 0)
{
_ASSERTE(executionAborted);
LOG((LF_GCROOTS, LL_INFO100000, "Not reporting this frame because it is aborted and not fully interruptible.\n"));
goto ExitSuccess;
}
}
#else // !PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED
Expand Down Expand Up @@ -716,12 +725,7 @@ bool GcInfoDecoder::EnumerateLiveSlots(
// Try partially interruptible first
//------------------------------------------------------------------------------

if(executionAborted)
{
_ASSERTE(m_NumSafePoints == 0);
m_Reader.Skip(m_NumSafePoints * numSlots);
}
else if( m_SafePointIndex != m_NumSafePoints )
if( !executionAborted && m_SafePointIndex != m_NumSafePoints )
{
if (numBitsPerOffset)
{
Expand Down Expand Up @@ -787,6 +791,8 @@ bool GcInfoDecoder::EnumerateLiveSlots(
else
{
m_Reader.Skip(m_NumSafePoints * numSlots);
if(m_NumInterruptibleRanges == 0)
goto ReportUntracked;
}
#endif // PARTIALLY_INTERRUPTIBLE_GC_SUPPORTED

Expand Down
11 changes: 9 additions & 2 deletions tests/src/GC/API/GC/KeepAlive.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,23 @@ public class Dummy2
}


public static void RunTest2()
{
Dummy2 obj2 = new Dummy2();
obj2 = null;
}


public static void RunTest()
{
Dummy obj = new Dummy();
Dummy2 obj2 = new Dummy2();

RunTest2();

// *uncomment the for loop to make test fail with complus_jitminops set
// by design as per briansul

//for (int i=0; i<5; i++) {
obj2 = null;
GC.Collect();
GC.WaitForPendingFinalizers();
//}
Expand Down
10 changes: 4 additions & 6 deletions tests/src/GC/Scenarios/LeakGen/leakgen.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ public bool runTest(int iRep, int iObj)
MakeLeak(iObj);
}

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();

Console.WriteLine("~LeakObject() was called {0} times.", LeakObject.icFinal);
return (LeakObject.icFinal == iObj*iRep);
}
Expand All @@ -80,12 +84,6 @@ public void MakeLeak(int iObj)
mem[mem.Length-1] = 1;
}

Mv_Obj = null;

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();

}


Expand Down
Loading