[clr-interp] Fix performance of compiling huge methods

src/coreclr/interpreter/compiler.cpp

@@ -838,10 +838,16 @@ int32_t* InterpCompiler::EmitCodeIns(int32_t *ip, InterpInst *ins, TArray<Reloc*
             // This code assumes that instructions for the same IL offset are emitted in a single run without
             // any other IL offsets in between and that they don't repeat again after the run ends.
 #ifdef DEBUG
-            for (int i = 0; i < m_ILToNativeMapSize; i++)
+            // Use a side table for fast checking of whether or not we've emitted this IL offset before.
+            // Walking the m_pILToNativeMap can be slow for excessively large functions
+            if (m_pNativeMapIndexToILOffset == NULL)
             {
-                assert(m_pILToNativeMap[i].ilOffset != ilOffset);
+                m_pNativeMapIndexToILOffset = new (this) int32_t[m_ILCodeSize];
+                memset(m_pNativeMapIndexToILOffset, 0, sizeof(int32_t) * m_ILCodeSize);
             }
+
+            assert(m_pNativeMapIndexToILOffset[ilOffset] == 0);
+            m_pNativeMapIndexToILOffset[ilOffset] = m_ILToNativeMapSize;
 #endif // DEBUG
 
             // Since we can have at most one entry per IL offset,
@@ -909,6 +915,7 @@ void InterpCompiler::EmitCode()
 
     // These will eventually be freed by the VM, and they use the delete [] operator for the deletion.
     m_pILToNativeMap = new ICorDebugInfo::OffsetMapping[m_ILCodeSize];
+
     ICorDebugInfo::NativeVarInfo* eeVars = NULL;
     if (m_numILVars > 0)
     {
@@ -998,6 +1005,13 @@ class InterpGcSlotAllocator
         uint32_t startOffset;
         uint32_t endOffset;
 
+        bool OverlapsOrAdjacentTo(uint32_t startOther, uint32_t endOther) const
+        {
+            assert (startOther < endOther);
+
+            return !(endOffset < startOther || startOffset > endOther);
+        }
+
         void MergeWith(uint32_t startOther, uint32_t endOther)
         {
             // This can only be called with overlapping or adjacent ranges
@@ -1014,38 +1028,77 @@ class InterpGcSlotAllocator
         }
     };
 
-    ConservativeRange** m_conservativeRanges = nullptr;
-
-    ConservativeRange* FindAndRemoveOverlappingOrAdjacentRange(uint32_t offsetBytes, uint32_t start, uint32_t end)
+    struct ConservativeRanges
     {
-        uint32_t slotIndex = offsetBytes / sizeof(void *);
-        ConservativeRange* prev = nullptr;
-        ConservativeRange* range = m_conservativeRanges[slotIndex];
-        while (range)
+        TArray<ConservativeRange> m_liveRanges;
+
+        void InsertRange(InterpCompiler* pCompiler, uint32_t start, uint32_t end)
         {
-            if (!(end < range->startOffset || start > range->endOffset))
+#ifdef DEBUG
+            bool m_verbose = pCompiler->m_verbose;
+#endif
+
+            ConservativeRange newRange(start, end);
+
+            if (m_liveRanges.GetSize() != 0)
             {
-                // Overlapping or adjacent ranges
-                if (prev)
-                    prev->next = range->next;
-                else
-                    m_conservativeRanges[slotIndex] = range->next;
-                range->next = nullptr;
-                return range;
+                // Find the first range which has a start offset greater or equal to the new ranges start offset
+
+                int32_t hiBound = m_liveRanges.GetSize();
+                int32_t loBound = 0;
+                while (loBound < hiBound)
+                {
+                    int32_t mid = (loBound + hiBound) / 2;
+                    ConservativeRange existingRange = m_liveRanges.Get(mid);
+                    if (existingRange.startOffset >= start)
+                    {
+                        hiBound = mid;
+                    }
+                    else
+                    {
+                        loBound = mid + 1;
+                    }
+                }
+                int32_t iFirstRangeWithGreaterStartOffset = hiBound;
+
+                // The range before the first range which is greater, may overlap with the new range, so subtract 1 if possible
+                int32_t iFirstInterestingRange = iFirstRangeWithGreaterStartOffset > 0 ? iFirstRangeWithGreaterStartOffset - 1: 0;
+
+                for (int32_t i = iFirstInterestingRange; i < m_liveRanges.GetSize(); i++)
+                {
+                    ConservativeRange existingRange = m_liveRanges.Get(i);
+                    if (existingRange.OverlapsOrAdjacentTo(start, end))
+                    {
+                        ConservativeRange updatedRange = existingRange;
+                        INTERP_DUMP("Merging with existing range [%u - %u]\n", existingRange.startOffset, existingRange.endOffset);
+                        updatedRange.MergeWith(start, end);
+                        while ((i + 1 < m_liveRanges.GetSize()) && m_liveRanges.Get(i + 1).OverlapsOrAdjacentTo(start, end))
+                        {
+                            ConservativeRange otherExistingRange = m_liveRanges.Get(i + 1);
+                            INTERP_DUMP("Merging with existing range [%u - %u]\n", otherExistingRange.startOffset, otherExistingRange.endOffset);
+                            updatedRange.MergeWith(otherExistingRange.startOffset, otherExistingRange.endOffset);
+                            m_liveRanges.RemoveAt(i + 1);
+                        }
+                        INTERP_DUMP("Final merged range [%u - %u]\n", updatedRange.startOffset, updatedRange.endOffset);
+                        m_liveRanges.Set(i, updatedRange);
+                        return;
+                    }
+
+                    if (existingRange.startOffset > start)
+                    {
+                        // If we reach here, the new range is disjoint from all existing ranges, and we've found the right place to insert
+                        m_liveRanges.InsertAt(i, newRange);
+                        return;
+                    }
+                }
+
             }
-            prev = range;
-            range = range->next;
+            // If we reach here, the new range is disjoint from all existing ranges, and should be added at the end of the list
+            m_liveRanges.Add(newRange);
         }
-        return nullptr;
-    }
-
-    void InsertConservativeRange(uint32_t offsetBytes, ConservativeRange* range)
-    {
-        uint32_t slotIndex = offsetBytes / sizeof(void *);
+    };
 
-        range->next = m_conservativeRanges[slotIndex];
-        m_conservativeRanges[slotIndex] = range;
-    }
+    ConservativeRanges* m_conservativeRanges = nullptr;
 
     unsigned m_slotTableSize;
 
@@ -1076,8 +1129,12 @@ class InterpGcSlotAllocator
             // 0 is a valid slot id so default-initialize all the slots to 0xFFFFFFFF
             memset(m_slotTables[i], 0xFF, sizeof(GcSlotId) * m_slotTableSize);
         }
-        m_conservativeRanges = new (m_compiler) ConservativeRange *[m_slotTableSize];
-        memset(m_conservativeRanges, 0, sizeof(ConservativeRange *) * m_slotTableSize);
+        m_conservativeRanges = new (m_compiler) ConservativeRanges[m_slotTableSize];
+    }
+
+    ~InterpGcSlotAllocator()
+    {
+        delete[] m_conservativeRanges;
     }
 
     void AllocateOrReuseGcSlot(uint32_t offsetBytes, GcSlotFlags flags)
@@ -1128,51 +1185,33 @@ class InterpGcSlotAllocator
             startOffset, endOffset
         );
 
-
-        ConservativeRange* pExistingRange = FindAndRemoveOverlappingOrAdjacentRange(offsetBytes, startOffset, endOffset);
-        if (pExistingRange != nullptr)
-        {
-            INTERP_DUMP("Merging with existing range [%u - %u]\n", pExistingRange->startOffset, pExistingRange->endOffset);
-            pExistingRange->MergeWith(startOffset, endOffset);
-            ConservativeRange* pOtherExistingRange = FindAndRemoveOverlappingOrAdjacentRange(offsetBytes, pExistingRange->startOffset, pExistingRange->endOffset);
-            while (pOtherExistingRange != nullptr)
-            {
-                INTERP_DUMP("Merging with existing range [%u - %u]\n", pOtherExistingRange->startOffset, pOtherExistingRange->endOffset);
-                pExistingRange->MergeWith(pOtherExistingRange->startOffset, pOtherExistingRange->endOffset);
-                pOtherExistingRange = FindAndRemoveOverlappingOrAdjacentRange(offsetBytes, pExistingRange->startOffset, pExistingRange->endOffset);
-            };
-            InsertConservativeRange(offsetBytes, pExistingRange);
-        }
-        else
-        {
-            ConservativeRange* pNewRange = new (m_compiler) ConservativeRange(startOffset, endOffset);
-            InsertConservativeRange(offsetBytes, pNewRange);
-        }
+        uint32_t slotIndex = offsetBytes / sizeof(void *);
+        m_conservativeRanges[slotIndex].InsertRange(m_compiler, startOffset, endOffset);
     }
 
     void ReportConservativeRangesToGCEncoder()
     {
         uint32_t maxEndOffset = m_compiler->ConvertOffset(m_compiler->m_methodCodeSize);
         for (uint32_t i = 0; i < m_slotTableSize; i++)
         {
-            ConservativeRange* range = m_conservativeRanges[i];
-
-            if (range == nullptr)
+            if (m_conservativeRanges[i].m_liveRanges.GetSize() == 0)
                 continue;
 
             GcSlotId* pSlot = LocateGcSlotTableEntry(i * sizeof(void *), GC_SLOT_INTERIOR);
             assert(pSlot != nullptr && *pSlot != (GcSlotId)-1);
 
-            while (range)
+            for(int32_t i = 0; i < m_conservativeRanges[i].m_liveRanges.GetSize(); i++)
             {
+                ConservativeRange range = m_conservativeRanges[i].m_liveRanges.Get(i);
+
                 INTERP_DUMP(
                     "Conservative range for slot %u at %u [%u - %u]\n",
                     *pSlot,
                     (unsigned)(i * sizeof(void *)),
-                    range->startOffset,
-                    range->endOffset + 1
+                    range.startOffset,
+                    range.endOffset + 1
                 );
-                m_encoder->SetSlotState(range->startOffset, *pSlot, GC_SLOT_LIVE);
+                m_encoder->SetSlotState(range.startOffset, *pSlot, GC_SLOT_LIVE);
 
                 // Conservatively assume that the lifetime of a slot extends to just past the end of what
                 // the lifetime analysis has determined. This is due to an inaccuracy of reporting in the
@@ -1187,16 +1226,15 @@ class InterpGcSlotAllocator
                 // in the caller's stack. This would likely reduce the conservative reporting being done by
                 // functions up the stack in the interpreter case, such that we don't have excessive conservative
                 // reporting causing substantial pinning overhead.
-                uint32_t endOffset = range->endOffset + 1;
+                uint32_t endOffset = range.endOffset + 1;
 
                 if (endOffset > maxEndOffset)
                 {
                     // The GcInfoEncoder should not report slots beyond the method's end
                     endOffset = maxEndOffset;
-                    assert(endOffset == range->endOffset);
+                    assert(endOffset == range.endOffset);
                 }
                 m_encoder->SetSlotState(endOffset, *pSlot, GC_SLOT_DEAD);
-                range = range->next;
             }
         }
     }

src/coreclr/interpreter/compiler.h

@@ -649,6 +649,9 @@ class InterpCompiler
     InterpBasicBlock**  m_ppOffsetToBB;
 
     ICorDebugInfo::OffsetMapping* m_pILToNativeMap = NULL;
+#ifdef DEBUG
+    int32_t* m_pNativeMapIndexToILOffset = NULL;
+#endif
     int32_t m_ILToNativeMapSize = 0;
 
     InterpBasicBlock*   AllocBB(int32_t ilOffset);
@@ -781,15 +784,25 @@ class InterpCompiler
  *  Uses the compiler's AllocMemPool0, which will eventually free automatically at the end of compilation (doesn't yet).
  */
 
- inline void* operator new(size_t sz, InterpCompiler* compiler)
- {
+inline void* operator new(size_t sz, InterpCompiler* compiler)
+{
+    return compiler->AllocMemPool0(sz);
+}
+
+inline void* operator new[](size_t sz, InterpCompiler* compiler)
+{
     return compiler->AllocMemPool0(sz);
 }
 
- inline void* operator new[](size_t sz, InterpCompiler* compiler)
- {
-     return compiler->AllocMemPool0(sz);
- }
+inline void operator delete(void* ptr, InterpCompiler* compiler)
+{
+    // Nothing to do, memory will be freed when the compiler is destroyed
+}
+
+inline void operator delete[](void* ptr, InterpCompiler* compiler)
+{
+    // Nothing to do, memory will be freed when the compiler is destroyed
+}
 
 template<typename T>
 int32_t InterpDataItemIndexMap::GetDataItemIndexForT(const T& lookup)

src/coreclr/interpreter/compileropt.cpp

@@ -137,7 +137,7 @@ void InterpCompiler::InitializeGlobalVars()
 void InterpCompiler::EndActiveCall(InterpInst *call)
 {
     // Remove call from array
-    m_pActiveCalls->Remove(call);
+    m_pActiveCalls->Remove_ArrayIsBag(call);
 
     // Push active call that should be resolved onto the stack
     if (m_pActiveCalls->GetSize())
@@ -211,7 +211,7 @@ void InterpCompiler::CompactActiveVars(int32_t *pCurrentOffset)
         if (m_pVars[var].alive)
             return;
         *pCurrentOffset = m_pVars[var].offset;
-        m_pActiveVars->RemoveAt(i);
+        m_pActiveVars->RemoveAt_ArrayIsBag(i);
         i--;
     }
 }

src/coreclr/interpreter/datastructs.h

@@ -130,6 +130,12 @@ class TArray
         return m_array[index];
     }
 
+    void Set(int32_t index, T value)
+    {
+        assert(index < m_size);
+        m_array[index] = value;
+    }
+
     int32_t Find(T element)
     {
         for (int i = 0; i < m_size; i++)
@@ -140,8 +146,32 @@ class TArray
         return -1;
     }
 
-    // Assumes elements are unique
     void RemoveAt(int32_t index)
+    {
+        assert(index < m_size);
+        for (int32_t iCopy = index + 1; iCopy < m_size; iCopy++)
+        {
+            m_array[iCopy - 1] = m_array[iCopy];
+        }
+        m_size--;
+    }
+
+    void InsertAt(int32_t index, T newValue)
+    {
+        assert(index < m_size);
+        if (m_size == m_capacity)
+            Grow();
+
+        for (int32_t iCopy = index; iCopy < m_size; iCopy++)
+        {
+            m_array[iCopy + 1] = m_array[iCopy];
+        }
+        m_array[index] = newValue;
+        m_size++;
+    }
+
+    // Assumes elements are unique, and order of items in the array is unimportant
+    void RemoveAt_ArrayIsBag(int32_t index)
     {
         assert(index < m_size);
         m_size--;
@@ -150,14 +180,14 @@ class TArray
             m_array[index] = m_array[m_size];
     } 
 
-    // Assumes elements are unique
-    void Remove(T element)
+    // Assumes elements are unique, and order of items in the array is unimportant
+    void Remove_ArrayIsBag(T element)
     {
         for (int32_t i = 0; i < m_size; i++)
         {
             if (element == m_array[i])
             {
-                RemoveAt(i);
+                RemoveAt_ArrayIsBag(i);
                 break;
             }
         }