Fix a few profiler related issues associated with frozen objects

src/coreclr/gc/gc.cpp

@@ -33324,22 +33324,20 @@ void gc_heap::walk_relocation (void* profiling_context, record_surv_fn fn)
     int condemned_gen_number = settings.condemned_generation;
     int stop_gen_idx = get_stop_generation_index (condemned_gen_number);
 
+    reset_pinned_queue_bos();
+    update_oldest_pinned_plug();
+
     for (int i = condemned_gen_number; i >= stop_gen_idx; i--)
     {
         generation* condemned_gen = generation_of (i);
-        heap_segment*  current_heap_segment = heap_segment_rw (generation_start_segment (condemned_gen));
-#ifdef USE_REGIONS
-        current_heap_segment = walk_relocation_sip (current_heap_segment, profiling_context, fn);
+        heap_segment*  current_heap_segment = generation_start_segment (condemned_gen);
+        current_heap_segment = walk_relocation_special_segments (current_heap_segment, profiling_context, fn);
         if (!current_heap_segment)
             continue;
-#endif // USE_REGIONS
         uint8_t*  start_address = get_soh_start_object (current_heap_segment, condemned_gen);
         size_t  current_brick = brick_of (start_address);
 
         PREFIX_ASSUME(current_heap_segment != NULL);
-
-        reset_pinned_queue_bos();
-        update_oldest_pinned_plug();
         size_t end_brick = brick_of (heap_segment_allocated (current_heap_segment)-1);
         walk_relocate_args args;
         args.is_shortened = FALSE;
@@ -33360,10 +33358,8 @@ void gc_heap::walk_relocation (void* profiling_context, record_surv_fn fn)
                             &args);
                     args.last_plug = 0;
                 }
-                current_heap_segment = heap_segment_next_rw (current_heap_segment);
-#ifdef USE_REGIONS
-                current_heap_segment = walk_relocation_sip (current_heap_segment, profiling_context, fn);
-#endif // USE_REGIONS
+                current_heap_segment = heap_segment_next (current_heap_segment);
+                current_heap_segment = walk_relocation_special_segments (current_heap_segment, profiling_context, fn);
                 if (current_heap_segment)
                 {
                     current_brick = brick_of (heap_segment_mem (current_heap_segment));
@@ -33389,44 +33385,58 @@ void gc_heap::walk_relocation (void* profiling_context, record_surv_fn fn)
     }
 }
 
-#ifdef USE_REGIONS
-heap_segment* gc_heap::walk_relocation_sip (heap_segment* current_heap_segment, void* profiling_context, record_surv_fn fn)
+heap_segment* gc_heap::walk_relocation_special_segments (heap_segment* current_heap_segment, void* profiling_context, record_surv_fn fn)
 {
-    while (current_heap_segment && heap_segment_swept_in_plan (current_heap_segment))
+    while (current_heap_segment)
     {
-        uint8_t* start = heap_segment_mem (current_heap_segment);
-        uint8_t* end = heap_segment_allocated (current_heap_segment);
-        uint8_t* obj = start;
-        uint8_t* plug_start = nullptr;
-        while (obj < end)
+        if (heap_segment_read_only_p (current_heap_segment))
         {
-            if (((CObjectHeader*)obj)->IsFree())
+            uint8_t* start = heap_segment_mem (current_heap_segment);
+            uint8_t* end = heap_segment_allocated (current_heap_segment);
+            fn (start, end, 0, profiling_context, !!settings.compaction, false);
+            current_heap_segment = heap_segment_next (current_heap_segment);
+        }
+#ifdef USE_REGIONS
+        else if (heap_segment_swept_in_plan (current_heap_segment))
+        {
+            uint8_t* start = heap_segment_mem (current_heap_segment);
+            uint8_t* end = heap_segment_allocated (current_heap_segment);
+            uint8_t* obj = start;
+            uint8_t* plug_start = nullptr;
+            while (obj < end)
             {
-                if (plug_start)
+                if (((CObjectHeader*)obj)->IsFree())
+                {
+                    if (plug_start)
+                    {
+                        fn (plug_start, obj, 0, profiling_context, !!settings.compaction, false);
+                        plug_start = nullptr;
+                    }
+                }
+                else
                 {
-                    fn (plug_start, obj, 0, profiling_context, false, false);
-                    plug_start = nullptr;
+                    if (!plug_start)
+                    {
+                        plug_start = obj;
+                    }
                 }
+
+                obj += Align (size (obj));
             }
-            else
+            if (plug_start)
             {
-                if (!plug_start)
-                {
-                    plug_start = obj;
-                }
+                fn (plug_start, end, 0, profiling_context, false, false);
             }
-
-            obj += Align (size (obj));
+            current_heap_segment = heap_segment_next (current_heap_segment);
         }
-        if (plug_start)
+#endif //USE_REGIONS
+        else
         {
-            fn (plug_start, end, 0, profiling_context, false, false);
+            break;
         }
-        current_heap_segment = heap_segment_next_rw (current_heap_segment);
     }
     return current_heap_segment;
 }
-#endif // USE_REGIONS
 
 void gc_heap::walk_survivors (record_surv_fn fn, void* context, walk_surv_type type)
 {

src/coreclr/gc/gcee.cpp

@@ -450,6 +450,9 @@ segment_handle GCHeap::RegisterFrozenSegment(segment_info *pseginfo)
     heap_segment_next(seg) = 0;
     heap_segment_used(seg) = heap_segment_allocated(seg);
     heap_segment_plan_allocated(seg) = 0;
+#ifdef USE_REGIONS
+    heap_segment_gen_num(seg) = 2;
+#endif //USE_REGIONS
     seg->flags = heap_segment_flags_readonly;
 
 #ifdef MULTIPLE_HEAPS

src/coreclr/gc/gcpriv.h

@@ -1697,10 +1697,8 @@ class gc_heap
 
     PER_HEAP
     void walk_relocation (void* profiling_context, record_surv_fn fn);
-#ifdef USE_REGIONS
     PER_HEAP
-    heap_segment* walk_relocation_sip (heap_segment* current_heap_segment, void* profiling_context, record_surv_fn fn);
-#endif // USE_REGIONS
+    heap_segment* walk_relocation_special_segments (heap_segment* current_heap_segment, void* profiling_context, record_surv_fn fn);
     PER_HEAP
     void walk_relocation_in_brick (uint8_t* tree, walk_relocate_args* args);
 

src/coreclr/vm/frozenobjectheap.cpp

@@ -18,7 +18,7 @@ FrozenObjectHeapManager::FrozenObjectHeapManager():
 // Allocates an object of the give size (including header) on a frozen segment.
 // May return nullptr if object is too large (larger than FOH_COMMIT_SIZE)
 // in such cases caller is responsible to find a more appropriate heap to allocate it
-Object* FrozenObjectHeapManager::TryAllocateObject(PTR_MethodTable type, size_t objectSize)
+Object* FrozenObjectHeapManager::TryAllocateObject(PTR_MethodTable type, size_t objectSize, bool publish)
 {
     CONTRACTL
     {
@@ -32,51 +32,59 @@ Object* FrozenObjectHeapManager::TryAllocateObject(PTR_MethodTable type, size_t
     return nullptr;
 #else // FEATURE_BASICFREEZE
 
-    CrstHolder ch(&m_Crst);
+    Object* obj = nullptr;
+    {
+        CrstHolder ch(&m_Crst);
 
-    _ASSERT(type != nullptr);
-    _ASSERT(FOH_COMMIT_SIZE >= MIN_OBJECT_SIZE);
+        _ASSERT(type != nullptr);
+        _ASSERT(FOH_COMMIT_SIZE >= MIN_OBJECT_SIZE);
 
-#ifdef FEATURE_64BIT_ALIGNMENT
-    _ASSERT(!type->RequiresAlign8());
-#endif
+    #ifdef FEATURE_64BIT_ALIGNMENT
+        _ASSERT(!type->RequiresAlign8());
+    #endif
 
-    // NOTE: objectSize is expected be the full size including header
-    _ASSERT(objectSize >= MIN_OBJECT_SIZE);
+        // NOTE: objectSize is expected be the full size including header
+        _ASSERT(objectSize >= MIN_OBJECT_SIZE);
 
-    if (objectSize > FOH_COMMIT_SIZE)
-    {
-        // The current design doesn't allow objects larger than FOH_COMMIT_SIZE and
-        // since FrozenObjectHeap is just an optimization, let's not fill it with huge objects.
-        return nullptr;
-    }
+        if (objectSize > FOH_COMMIT_SIZE)
+        {
+            // The current design doesn't allow objects larger than FOH_COMMIT_SIZE and
+            // since FrozenObjectHeap is just an optimization, let's not fill it with huge objects.
+            return nullptr;
+        }
 
-    if (m_CurrentSegment == nullptr)
-    {
-        // Create the first segment on first allocation
-        m_CurrentSegment = new FrozenObjectSegment(FOH_SEGMENT_DEFAULT_SIZE);
-        m_FrozenSegments.Append(m_CurrentSegment);
-        _ASSERT(m_CurrentSegment != nullptr);
-    }
+        if (m_CurrentSegment == nullptr)
+        {
+            // Create the first segment on first allocation
+            m_CurrentSegment = new FrozenObjectSegment(FOH_SEGMENT_DEFAULT_SIZE);
+            m_FrozenSegments.Append(m_CurrentSegment);
+            _ASSERT(m_CurrentSegment != nullptr);
+        }
 
-    Object* obj = m_CurrentSegment->TryAllocateObject(type, objectSize);
+        obj = m_CurrentSegment->TryAllocateObject(type, objectSize);
 
-    // The only case where it can be null is when the current segment is full and we need
-    // to create a new one
-    if (obj == nullptr)
-    {
-        // Double the reserved size to reduce the number of frozen segments in apps with lots of frozen objects
-        // Use the same size in case if prevSegmentSize*2 operation overflows.
-        size_t prevSegmentSize = m_CurrentSegment->GetSize();
-        m_CurrentSegment = new FrozenObjectSegment(max(prevSegmentSize, prevSegmentSize * 2));
-        m_FrozenSegments.Append(m_CurrentSegment);
+        // The only case where it can be null is when the current segment is full and we need
+        // to create a new one
+        if (obj == nullptr)
+        {
+            // Double the reserved size to reduce the number of frozen segments in apps with lots of frozen objects
+            // Use the same size in case if prevSegmentSize*2 operation overflows.
+            size_t prevSegmentSize = m_CurrentSegment->GetSize();
+            m_CurrentSegment = new FrozenObjectSegment(max(prevSegmentSize, prevSegmentSize * 2));
+            m_FrozenSegments.Append(m_CurrentSegment);
 
-        // Try again
-        obj = m_CurrentSegment->TryAllocateObject(type, objectSize);
+            // Try again
+            obj = m_CurrentSegment->TryAllocateObject(type, objectSize);
 
-        // This time it's not expected to be null
-        _ASSERT(obj != nullptr);
+            // This time it's not expected to be null
+            _ASSERT(obj != nullptr);
+        }
+    }
+    if (publish)
+    {
+        PublishFrozenObject(obj);
     }
+
     return obj;
 #endif // !FEATURE_BASICFREEZE
 }

src/coreclr/vm/frozenobjectheap.h

@@ -27,7 +27,7 @@ class FrozenObjectHeapManager
 {
 public:
     FrozenObjectHeapManager();
-    Object* TryAllocateObject(PTR_MethodTable type, size_t objectSize);
+    Object* TryAllocateObject(PTR_MethodTable type, size_t objectSize, bool publish = true);
 
 private:
     Crst m_Crst;

src/coreclr/vm/gchelpers.cpp

@@ -342,6 +342,11 @@ void PublishObjectAndNotify(TObj* &orObject, GC_ALLOC_FLAGS flags)
 #endif // FEATURE_EVENT_TRACE
 }
 
+void PublishFrozenObject(Object*& orObject)
+{
+    PublishObjectAndNotify(orObject, GC_ALLOC_NO_FLAGS);
+}
+
 inline SIZE_T MaxArrayLength()
 {
     // Impose limits on maximum array length to prevent corner case integer overflow bugs
@@ -886,10 +891,12 @@ STRINGREF AllocateString(DWORD cchStringLength, bool preferFrozenHeap, bool* pIs
     if (preferFrozenHeap)
     {
         FrozenObjectHeapManager* foh = SystemDomain::GetFrozenObjectHeapManager();
-        orString = static_cast<StringObject*>(foh->TryAllocateObject(g_pStringClass, totalSize));
+        orString = static_cast<StringObject*>(foh->TryAllocateObject(g_pStringClass, totalSize, /* publish = */false));
         if (orString != nullptr)
         {
             orString->SetStringLength(cchStringLength);
+            // Publish needs to be postponed in this case because we need to specify string length 
+            PublishObjectAndNotify(orString, GC_ALLOC_NO_FLAGS);
             _ASSERTE(orString->GetBuffer()[cchStringLength] == W('\0'));
             orStringRef = ObjectToSTRINGREF(orString);
             *pIsFrozen = true;

src/coreclr/vm/gchelpers.h

@@ -68,4 +68,7 @@ extern void ThrowOutOfMemoryDimensionsExceeded();
 
 void ErectWriteBarrier(OBJECTREF* dst, OBJECTREF ref);
 void SetCardsAfterBulkCopy(Object **start, size_t len);
+
+void PublishFrozenObject(Object*& orObject);
+
 #endif // _GCHELPERS_H_