From 24293e7726717fcf76a6788c7ad3a44d483d1386 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 27 Apr 2023 03:03:05 +0200 Subject: [PATCH 1/5] Implement ICorProfilerInfo14::GetNonGCHeapBounds --- src/coreclr/inc/corprof.idl | 16 ++++++ src/coreclr/pal/prebuilt/inc/corprof.h | 22 ++++++++ src/coreclr/vm/frozenobjectheap.cpp | 4 +- src/coreclr/vm/frozenobjectheap.h | 2 + src/coreclr/vm/proftoeeinterfaceimpl.cpp | 54 +++++++++++++++++++ src/coreclr/vm/proftoeeinterfaceimpl.h | 4 ++ .../profiler/native/nongcheap/nongcheap.cpp | 46 +++++++++++++++- 7 files changed, 144 insertions(+), 4 deletions(-) diff --git a/src/coreclr/inc/corprof.idl b/src/coreclr/inc/corprof.idl index 11ce62e89f20b..1ae14ab4fdbad 100644 --- a/src/coreclr/inc/corprof.idl +++ b/src/coreclr/inc/corprof.idl @@ -2035,6 +2035,17 @@ typedef struct COR_PRF_GC_GENERATION_RANGE } COR_PRF_GC_GENERATION_RANGE; +/* + * COR_PRF_NONGC_GENERATION_RANGE describes a range of memory in the GetNonGCHeapBounds function. + */ +typedef struct COR_PRF_NONGC_HEAP_RANGE +{ + ObjectID rangeStart; // the start of the range + UINT_PTR rangeLength; // the used length of the range + UINT_PTR rangeLengthReserved; // the amount of memory reserved for the range (including rangeLength) + +} COR_PRF_NONGC_HEAP_RANGE; + /* * COR_PRF_CLAUSE_TYPE defines the various clause codes for the EX clauses @@ -4254,6 +4265,11 @@ interface ICorProfilerInfo13 : ICorProfilerInfo12 interface ICorProfilerInfo14 : ICorProfilerInfo13 { HRESULT EnumerateNonGCObjects([out] ICorProfilerObjectEnum** ppEnum); + + HRESULT GetNonGCHeapBounds( + [in] ULONG cObjectRanges, + [out] ULONG *pcObjectRanges, + [out, size_is(cObjectRanges), length_is(*pcObjectRanges)] COR_PRF_NONGC_HEAP_RANGE ranges[]); } /* diff --git a/src/coreclr/pal/prebuilt/inc/corprof.h b/src/coreclr/pal/prebuilt/inc/corprof.h index 7682710acea95..8023d897de13e 100644 --- a/src/coreclr/pal/prebuilt/inc/corprof.h +++ b/src/coreclr/pal/prebuilt/inc/corprof.h @@ -1666,6 +1666,13 @@ typedef struct COR_PRF_GC_GENERATION_RANGE UINT_PTR rangeLengthReserved; } COR_PRF_GC_GENERATION_RANGE; +typedef struct COR_PRF_NONGC_HEAP_RANGE + { + ObjectID rangeStart; + UINT_PTR rangeLength; + UINT_PTR rangeLengthReserved; + } COR_PRF_NONGC_HEAP_RANGE; + typedef /* [public][public][public] */ enum __MIDL___MIDL_itf_corprof_0000_0001_0005 { @@ -23231,6 +23238,11 @@ EXTERN_C const IID IID_ICorProfilerInfo14; virtual HRESULT STDMETHODCALLTYPE EnumerateNonGCObjects( /* [out] */ ICorProfilerObjectEnum **ppEnum) = 0; + virtual HRESULT STDMETHODCALLTYPE GetNonGCHeapBounds( + /* [in] */ ULONG cObjectRanges, + /* [out] */ ULONG *pcObjectRanges, + /* [length_is][size_is][out] */ COR_PRF_NONGC_HEAP_RANGE ranges[ ]) = 0; + }; @@ -24039,6 +24051,13 @@ EXTERN_C const IID IID_ICorProfilerInfo14; ICorProfilerInfo14 * This, /* [out] */ ICorProfilerObjectEnum **ppEnum); + DECLSPEC_XFGVIRT(ICorProfilerInfo14, GetNonGCHeapBounds) + HRESULT ( STDMETHODCALLTYPE *GetNonGCHeapBounds )( + ICorProfilerInfo14 * This, + /* [in] */ ULONG cObjectRanges, + /* [out] */ ULONG *pcObjectRanges, + /* [length_is][size_is][out] */ COR_PRF_NONGC_HEAP_RANGE ranges[ ]); + END_INTERFACE } ICorProfilerInfo14Vtbl; @@ -24402,6 +24421,9 @@ EXTERN_C const IID IID_ICorProfilerInfo14; #define ICorProfilerInfo14_EnumerateNonGCObjects(This,ppEnum) \ ( (This)->lpVtbl -> EnumerateNonGCObjects(This,ppEnum) ) +#define ICorProfilerInfo14_GetNonGCHeapBounds(This,cObjectRanges,pcObjectRanges,ranges) \ + ( (This)->lpVtbl -> GetNonGCHeapBounds(This,cObjectRanges,pcObjectRanges,ranges) ) + #endif /* COBJMACROS */ diff --git a/src/coreclr/vm/frozenobjectheap.cpp b/src/coreclr/vm/frozenobjectheap.cpp index 036177d1b7e95..a2efc7dcf000c 100644 --- a/src/coreclr/vm/frozenobjectheap.cpp +++ b/src/coreclr/vm/frozenobjectheap.cpp @@ -226,9 +226,7 @@ Object* FrozenObjectSegment::GetNextObject(Object* obj) const uint8_t* nextObj = (reinterpret_cast(obj) + ALIGN_UP(obj->GetSize(), DATA_ALIGNMENT)); if (nextObj < m_pCurrent) { - Object* result = reinterpret_cast(nextObj); - INDEBUG(result->Validate()); - return result; + return reinterpret_cast(nextObj); } // Current object is the last one in the segment diff --git a/src/coreclr/vm/frozenobjectheap.h b/src/coreclr/vm/frozenobjectheap.h index d8cc77861ece1..d2c0bb62f134a 100644 --- a/src/coreclr/vm/frozenobjectheap.h +++ b/src/coreclr/vm/frozenobjectheap.h @@ -35,6 +35,7 @@ class FrozenObjectHeapManager FrozenObjectSegment* m_CurrentSegment; friend class ProfilerObjectEnum; + friend class ProfToEEInterfaceImpl; }; class FrozenObjectSegment @@ -72,6 +73,7 @@ class FrozenObjectSegment INDEBUG(size_t m_ObjectsCount); friend class ProfilerObjectEnum; + friend class ProfToEEInterfaceImpl; }; #endif // _FROZENOBJECTHEAP_H diff --git a/src/coreclr/vm/proftoeeinterfaceimpl.cpp b/src/coreclr/vm/proftoeeinterfaceimpl.cpp index e6f5cb25fbf1a..8c8143f3c5bbf 100644 --- a/src/coreclr/vm/proftoeeinterfaceimpl.cpp +++ b/src/coreclr/vm/proftoeeinterfaceimpl.cpp @@ -125,6 +125,7 @@ #include "safemath.h" #include "threadsuspend.h" #include "inlinetracking.h" +#include "frozenobjectheap.h" #ifdef PROFILING_SUPPORTED #include "profilinghelper.h" @@ -7598,6 +7599,7 @@ HRESULT ProfToEEInterfaceImpl::EnumerateNonGCObjects(ICorProfilerObjectEnum** pp GC_NOTRIGGER; MODE_ANY; EE_THREAD_NOT_REQUIRED; + CAN_TAKE_LOCK; } CONTRACTL_END; @@ -7624,6 +7626,58 @@ HRESULT ProfToEEInterfaceImpl::EnumerateNonGCObjects(ICorProfilerObjectEnum** pp return hr; } +HRESULT ProfToEEInterfaceImpl::GetNonGCHeapBounds(ULONG cObjectRanges, + ULONG *pcObjectRanges, + COR_PRF_NONGC_HEAP_RANGE ranges[]) +{ + CONTRACTL + { + NOTHROW; + GC_NOTRIGGER; + MODE_ANY; + EE_THREAD_NOT_REQUIRED; + CAN_TAKE_LOCK; + } + CONTRACTL_END; + + if ((cObjectRanges > 0) && (ranges == nullptr)) + { + // Copy GetGenerationBounds's behavior for consistency + return E_INVALIDARG; + } + + FrozenObjectHeapManager* foh = SystemDomain::GetFrozenObjectHeapManager(); + CrstHolder ch(&foh->m_Crst); + + const unsigned segmentsCount = foh->m_FrozenSegments.GetCount(); + FrozenObjectSegment** segments = foh->m_FrozenSegments.GetElements(); + if (segments != nullptr && segmentsCount > 0) + { + const ULONG segmentsToInspect = min(cObjectRanges, (ULONG)segmentsCount); + + for (unsigned segIdx = 0; segIdx < segmentsToInspect; segIdx++) + { + uint8_t* firstObj = segments[segIdx]->m_pStart + sizeof(ObjHeader); + + // Start of the segment (first object) + ranges[segIdx].rangeStart = (ObjectID)firstObj; + + // Total size reserved for a segment + ranges[segIdx].rangeLengthReserved = (UINT_PTR)segments[segIdx]->m_Size; + + // Size of the segment that is currently in use + ranges[segIdx].rangeLength = (UINT_PTR)(segments[segIdx]->m_pCurrent - firstObj); + } + + *pcObjectRanges = segmentsToInspect; + } + else + { + *pcObjectRanges = 0; + } + return S_OK; +} + /* * GetStringLayout * diff --git a/src/coreclr/vm/proftoeeinterfaceimpl.h b/src/coreclr/vm/proftoeeinterfaceimpl.h index fe0807a57f37e..a0f9ab681d200 100644 --- a/src/coreclr/vm/proftoeeinterfaceimpl.h +++ b/src/coreclr/vm/proftoeeinterfaceimpl.h @@ -727,6 +727,10 @@ class ProfToEEInterfaceImpl : public ICorProfilerInfo14 COM_METHOD EnumerateNonGCObjects( ICorProfilerObjectEnum** ppEnum); + COM_METHOD GetNonGCHeapBounds(ULONG cObjectRanges, + ULONG * pcObjectRanges, + COR_PRF_NONGC_HEAP_RANGE ranges[]); + // end ICorProfilerInfo14 protected: diff --git a/src/tests/profiler/native/nongcheap/nongcheap.cpp b/src/tests/profiler/native/nongcheap/nongcheap.cpp index 167901c2f1251..cebf27add6b1b 100644 --- a/src/tests/profiler/native/nongcheap/nongcheap.cpp +++ b/src/tests/profiler/native/nongcheap/nongcheap.cpp @@ -59,10 +59,44 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished() _garbageCollections++; + COR_PRF_NONGC_HEAP_RANGE segments[16]; + ULONG segCount; + ObjectID firstObj = 0; + HRESULT hr = pCorProfilerInfo->GetNonGCHeapBounds(16, &segCount, segments); + if (FAILED(hr)) + { + printf("GetNonGCHeapBounds returned an error\n!"); + _failures++; + } + else if (segCount == 0 || segCount > 16) + { + printf("GetNonGCHeapBounds invalid segCount (%lu)\n!", segCount); + _failures++; + } + else + { + // Save very first object ID to compare with EnumerateNonGCObjects + firstObj = segments[0].rangeStart; + + printf("\nGetNonGCHeapBounds (segCount = %lu):\n", segCount); + for (ULONG i = 0; i < segCount; i++) + { + printf("\tseg#%ld, rangeStart=%p, rangeLength=%lu, rangeLengthReserved=%lu\n", + i, (void*)segments[i].rangeStart, (ULONG)segments[i].rangeLength, (ULONG)segments[i].rangeLengthReserved); + + if ((ULONG)segments[i].rangeLength > (ULONG)segments[i].rangeLengthReserved) + { + printf("GetNonGCHeapBounds: rangeLength > rangeLengthReserved"); + _failures++; + } + } + printf("\n"); + } + // Let's make sure we got the same number of objects as we got from the callback // by testing the EnumerateNonGCObjects API. ICorProfilerObjectEnum* pEnum = NULL; - HRESULT hr = pCorProfilerInfo->EnumerateNonGCObjects(&pEnum); + hr = pCorProfilerInfo->EnumerateNonGCObjects(&pEnum); if (FAILED(hr)) { printf("EnumerateNonGCObjects returned an error\n!"); @@ -72,8 +106,18 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished() { int nonGcObjectsEnumerated = 0; ObjectID obj; + bool isFirstObj = true; while (pEnum->Next(1, &obj, NULL) == S_OK) { + if (isFirstObj) + { + if (firstObj != obj) + { + printf("EnumerateNonGCObjects: firstObj != obj\n!"); + _failures++; + } + } + isFirstObj = false; nonGcObjectsEnumerated++; } From 4af8097c160e8d21466c1f4f6b0a2404cb44d11f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 27 Apr 2023 11:38:18 +0200 Subject: [PATCH 2/5] Fix tests --- .../profiler/native/nongcheap/nongcheap.cpp | 28 +++++++++++++++---- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/tests/profiler/native/nongcheap/nongcheap.cpp b/src/tests/profiler/native/nongcheap/nongcheap.cpp index cebf27add6b1b..e352607f4950d 100644 --- a/src/tests/profiler/native/nongcheap/nongcheap.cpp +++ b/src/tests/profiler/native/nongcheap/nongcheap.cpp @@ -59,18 +59,19 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished() _garbageCollections++; - COR_PRF_NONGC_HEAP_RANGE segments[16]; + const int MAX_NON_GC_HEAP_SEGMENTS = 16; + COR_PRF_NONGC_HEAP_RANGE segments[MAX_NON_GC_HEAP_SEGMENTS]; ULONG segCount; ObjectID firstObj = 0; - HRESULT hr = pCorProfilerInfo->GetNonGCHeapBounds(16, &segCount, segments); + HRESULT hr = pCorProfilerInfo->GetNonGCHeapBounds(MAX_NON_GC_HEAP_SEGMENTS, &segCount, segments); if (FAILED(hr)) { printf("GetNonGCHeapBounds returned an error\n!"); _failures++; } - else if (segCount == 0 || segCount > 16) + else if (segCount == 0 || segCount > MAX_NON_GC_HEAP_SEGMENTS) { - printf("GetNonGCHeapBounds invalid segCount (%lu)\n!", segCount); + printf("GetNonGCHeapBounds: invalid segCount (%u)\n!", segCount); _failures++; } else @@ -81,7 +82,7 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished() printf("\nGetNonGCHeapBounds (segCount = %lu):\n", segCount); for (ULONG i = 0; i < segCount; i++) { - printf("\tseg#%ld, rangeStart=%p, rangeLength=%lu, rangeLengthReserved=%lu\n", + printf("\tseg#%ld, rangeStart=%p, rangeLength=%u, rangeLengthReserved=%u\n", i, (void*)segments[i].rangeStart, (ULONG)segments[i].rangeLength, (ULONG)segments[i].rangeLengthReserved); if ((ULONG)segments[i].rangeLength > (ULONG)segments[i].rangeLengthReserved) @@ -89,6 +90,12 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished() printf("GetNonGCHeapBounds: rangeLength > rangeLengthReserved"); _failures++; } + + if (!segments[i].rangeStart) + { + printf("GetNonGCHeapBounds: rangeStart is null"); + _failures++; + } } printf("\n"); } @@ -117,6 +124,17 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished() _failures++; } } + + // Add test coverage for IsFrozenObject API, currently, it is expected to return true + // for objects from non-GC heap (it might also return true for frozen segments we don't track) + BOOL isFrozen; + hr = pCorProfilerInfo->IsFrozenObject(obj, &isFrozen); + if (FAILED(hr) || !isFrozen) + { + printf("EnumerateNonGCObjects: IsFrozenObject failed\n!"); + _failures++; + } + isFirstObj = false; nonGcObjectsEnumerated++; } From 4efbcd2d8b236694cd2c9822cecdef2629de1d5a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 27 Apr 2023 11:42:05 +0200 Subject: [PATCH 3/5] Add comments --- src/coreclr/vm/proftoeeinterfaceimpl.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/vm/proftoeeinterfaceimpl.cpp b/src/coreclr/vm/proftoeeinterfaceimpl.cpp index 8c8143f3c5bbf..8328dfd0df05e 100644 --- a/src/coreclr/vm/proftoeeinterfaceimpl.cpp +++ b/src/coreclr/vm/proftoeeinterfaceimpl.cpp @@ -7599,6 +7599,8 @@ HRESULT ProfToEEInterfaceImpl::EnumerateNonGCObjects(ICorProfilerObjectEnum** pp GC_NOTRIGGER; MODE_ANY; EE_THREAD_NOT_REQUIRED; + + // FrozenObjectHeapManager takes a lock CAN_TAKE_LOCK; } CONTRACTL_END; @@ -7636,6 +7638,8 @@ HRESULT ProfToEEInterfaceImpl::GetNonGCHeapBounds(ULONG cObjectRanges, GC_NOTRIGGER; MODE_ANY; EE_THREAD_NOT_REQUIRED; + + // FrozenObjectHeapManager takes a lock CAN_TAKE_LOCK; } CONTRACTL_END; From 3516ffe27b4120efc1fcf8bc90317b43bab4e1b3 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 27 Apr 2023 12:04:32 +0200 Subject: [PATCH 4/5] fix build --- src/tests/profiler/native/nongcheap/nongcheap.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/profiler/native/nongcheap/nongcheap.cpp b/src/tests/profiler/native/nongcheap/nongcheap.cpp index e352607f4950d..eca9270d33b43 100644 --- a/src/tests/profiler/native/nongcheap/nongcheap.cpp +++ b/src/tests/profiler/native/nongcheap/nongcheap.cpp @@ -79,10 +79,10 @@ HRESULT NonGcHeapProfiler::GarbageCollectionFinished() // Save very first object ID to compare with EnumerateNonGCObjects firstObj = segments[0].rangeStart; - printf("\nGetNonGCHeapBounds (segCount = %lu):\n", segCount); + printf("\nGetNonGCHeapBounds (segCount = %u):\n", segCount); for (ULONG i = 0; i < segCount; i++) { - printf("\tseg#%ld, rangeStart=%p, rangeLength=%u, rangeLengthReserved=%u\n", + printf("\tseg#%u, rangeStart=%p, rangeLength=%u, rangeLengthReserved=%u\n", i, (void*)segments[i].rangeStart, (ULONG)segments[i].rangeLength, (ULONG)segments[i].rangeLengthReserved); if ((ULONG)segments[i].rangeLength > (ULONG)segments[i].rangeLengthReserved) From 71163ca91750813f0a7ffd7d48653a64bd326808 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Tue, 2 May 2023 15:58:16 +0200 Subject: [PATCH 5/5] Add a nullcheck (address feedback) --- src/coreclr/vm/proftoeeinterfaceimpl.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/proftoeeinterfaceimpl.cpp b/src/coreclr/vm/proftoeeinterfaceimpl.cpp index 8328dfd0df05e..f2b67a94cf2ad 100644 --- a/src/coreclr/vm/proftoeeinterfaceimpl.cpp +++ b/src/coreclr/vm/proftoeeinterfaceimpl.cpp @@ -7673,11 +7673,17 @@ HRESULT ProfToEEInterfaceImpl::GetNonGCHeapBounds(ULONG cObjectRanges, ranges[segIdx].rangeLength = (UINT_PTR)(segments[segIdx]->m_pCurrent - firstObj); } - *pcObjectRanges = segmentsToInspect; + if (pcObjectRanges != nullptr) + { + *pcObjectRanges = segmentsToInspect; + } } else { - *pcObjectRanges = 0; + if (pcObjectRanges != nullptr) + { + *pcObjectRanges = 0; + } } return S_OK; }