From dbeba7e1afe4062ef74542e299756b23540739a8 Mon Sep 17 00:00:00 2001 From: Aidan Lee Date: Wed, 3 Jan 2024 18:46:15 +0000 Subject: [PATCH 1/4] expose MemType enum and check function in the hx namespace --- include/hx/GC.h | 4 ++++ src/hx/gc/Immix.cpp | 15 +++++++++------ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/include/hx/GC.h b/include/hx/GC.h index 5370c142e..ce3957776 100644 --- a/include/hx/GC.h +++ b/include/hx/GC.h @@ -283,6 +283,10 @@ void GCPrepareMultiThreaded(); namespace hx { +enum MemType { memUnmanaged, memBlock, memLarge }; + +MemType GetMemType(void*); + #define HX_USE_INLINE_IMMIX_OPERATOR_NEW //#define HX_STACK_CTX ::hx::ImmixAllocator *_hx_stack_ctx = hx::gMultiThreadMode ? hx::tlsImmixAllocator : hx::gMainThreadAlloc; diff --git a/src/hx/gc/Immix.cpp b/src/hx/gc/Immix.cpp index 47fccd5a4..e0e1fc341 100644 --- a/src/hx/gc/Immix.cpp +++ b/src/hx/gc/Immix.cpp @@ -2907,8 +2907,6 @@ typedef hx::QuickVec BlockList; typedef hx::QuickVec LargeList; -enum MemType { memUnmanaged, memBlock, memLarge }; - @@ -5524,7 +5522,7 @@ class GlobalAllocator return false; } - MemType GetMemType(void *inPtr) + hx::MemType GetMemType(void *inPtr) { BlockData *block = (BlockData *)( ((size_t)inPtr) & IMMIX_BLOCK_BASE_MASK); @@ -5542,16 +5540,16 @@ class GlobalAllocator */ if (isBlock) - return memBlock; + return hx::memBlock; for(int i=0;iGetMemType(inPtr); +} + } // namespace hx From 6aa73136fd059dac7eab7c23c28e4e015f5dcc6c Mon Sep 17 00:00:00 2001 From: Aidan Lee Date: Fri, 5 Jan 2024 21:35:12 +0000 Subject: [PATCH 2/4] Revert "expose MemType enum and check function in the hx namespace" This reverts commit dbeba7e1afe4062ef74542e299756b23540739a8. --- include/Array.h | 20 +++++++++++++ include/hx/GC.h | 8 +++--- src/Array.cpp | 69 +++++++++++++++++++++++++++++++++++++++++++++ src/hx/gc/Immix.cpp | 61 +++++++++++++++++++++++++++++++-------- 4 files changed, 143 insertions(+), 15 deletions(-) diff --git a/include/Array.h b/include/Array.h index 45e76879f..787fe507e 100644 --- a/include/Array.h +++ b/include/Array.h @@ -126,6 +126,19 @@ class ArrayKeyValueIterator : public cpp::FastIterator_obj namespace hx { +#if (HXCPP_API_LEVEL>=500) +class HXCPP_EXTERN_CLASS_ATTRIBUTES ArrayPin { + char* ptr; + hx::Object* object; + +public: + ArrayPin(char* inPtr, hx::Object* inObject); + + ~ArrayPin(); + + char* GetBase(); +}; +#endif // Also used by cpp::VirtualArray class HXCPP_EXTERN_CLASS_ATTRIBUTES ArrayCommon : public hx::Object @@ -196,6 +209,9 @@ class HXCPP_EXTERN_CLASS_ATTRIBUTES ArrayBase : public ArrayCommon mAlloc = -1; } +#if (HXCPP_API_LEVEL>=500) + ArrayPin* Pin(); +#endif int __GetType() const { return vtArray; } @@ -407,6 +423,10 @@ class HXCPP_EXTERN_CLASS_ATTRIBUTES ArrayBase : public ArrayCommon protected: mutable int mAlloc; mutable char *mBase; + +#if (HXCPP_API_LEVEL>=500) + bool hasBeenPinned; +#endif }; } // end namespace hx for ArrayBase diff --git a/include/hx/GC.h b/include/hx/GC.h index ce3957776..e82bb76cf 100644 --- a/include/hx/GC.h +++ b/include/hx/GC.h @@ -187,6 +187,10 @@ void MarkConservative(int *inBottom, int *inTop,hx::MarkContext *__inCtx); void GCAddRoot(hx::Object **inRoot); void GCRemoveRoot(hx::Object **inRoot); +#if (HXCPP_API_LEVEL>=500) +void GCPinPtr(const void* inPtr); +void GCUnpinPtr(const void* inPtr); +#endif // This is used internally in hxcpp // It calls InternalNew, and takes care of null-terminating the result @@ -283,10 +287,6 @@ void GCPrepareMultiThreaded(); namespace hx { -enum MemType { memUnmanaged, memBlock, memLarge }; - -MemType GetMemType(void*); - #define HX_USE_INLINE_IMMIX_OPERATOR_NEW //#define HX_STACK_CTX ::hx::ImmixAllocator *_hx_stack_ctx = hx::gMultiThreadMode ? hx::tlsImmixAllocator : hx::gMainThreadAlloc; diff --git a/src/Array.cpp b/src/Array.cpp index 987204639..578dfcbd2 100644 --- a/src/Array.cpp +++ b/src/Array.cpp @@ -34,9 +34,66 @@ ArrayBase::ArrayBase(int inSize,int inReserve,int inElementSize,bool inAtomic) mAlloc = alloc; mArrayConvertId = inAtomic ? inElementSize : inElementSize==sizeof(String) ? aciStringArray : aciObjectArray; + +#if (HXCPP_API_LEVEL>=500) + hasBeenPinned = false; +#endif +} + +#if (HXCPP_API_LEVEL>=500) +ArrayPin::ArrayPin(char* inPtr, hx::Object* inObject) + : ptr(inPtr) + , object(inObject) +{ + hx::GCPinPtr(ptr); + hx::GCAddRoot(&object); } +ArrayPin::~ArrayPin() +{ + hx::GCUnpinPtr(ptr); + hx::GCAddRoot(&object); +} +char* ArrayPin::GetBase() +{ + return ptr; +} + +ArrayPin* ArrayBase::Pin() +{ + class Marker : public hx::Object + { + char* mBase; + + public: + Marker(char* inBase) : mBase(inBase) + { + HX_OBJ_WB_NEW_MARKED_OBJECT(this); + } + + void __Mark(hx::MarkContext* ctx) + { + hx::MarkAlloc(mBase, ctx); + } + }; + + if (!mBase) + { + return nullptr; + } + + // Unmanaged data, is it safe to put this in the ArrayPin if we don't know the lifetime of that pointer? + if (mAlloc < 0) + { + return new ArrayPin(mBase, nullptr); + } + + hasBeenPinned = true; + + return new ArrayPin(mBase, new Marker(mBase)); +} +#endif void ArrayBase::reserve(int inSize) const { @@ -48,7 +105,11 @@ void ArrayBase::reserve(int inSize) const if (mBase) { bool wasUnamanaged = mAlloc<0; +#if (HXCPP_API_LEVEL>=500) + if (wasUnamanaged || hasBeenPinned) +#else if (wasUnamanaged) +#endif { char *base=(char *)hx::InternalNew(bytes,false); memcpy(base,mBase,length*elemSize); @@ -99,7 +160,11 @@ void ArrayBase::Realloc(int inSize) const if (mBase) { bool wasUnamanaged = mAlloc<0; +#if (HXCPP_API_LEVEL>=500) + if (wasUnamanaged || hasBeenPinned) +#else if (wasUnamanaged) +#endif { char *base=(char *)hx::InternalNew(bytes,false); memcpy(base,mBase,length*elemSize); @@ -228,7 +293,11 @@ void ArrayBase::__SetSizeExact(int inSize) { bool wasUnamanaged = mAlloc<0; +#if (HXCPP_API_LEVEL>=500) + if (wasUnamanaged || hasBeenPinned) +#else if (wasUnamanaged) +#endif { char *base=(char *)(AllocAtomic() ? hx::NewGCPrivate(0,bytes) : hx::NewGCBytes(0,bytes)); memcpy(base,mBase,std::min(length,inSize)*elemSize); diff --git a/src/hx/gc/Immix.cpp b/src/hx/gc/Immix.cpp index e0e1fc341..61f93797e 100644 --- a/src/hx/gc/Immix.cpp +++ b/src/hx/gc/Immix.cpp @@ -741,6 +741,7 @@ struct BlockDataInfo int mMoveScore; int mUsedBytes; int mFraggedRows; + int mPinnedAllocs; bool mPinned; unsigned char mZeroed; bool mReclaimed; @@ -789,6 +790,7 @@ struct BlockDataInfo mUsedRows = 0; mUsedBytes = 0; mFraggedRows = 0; + mPinnedAllocs = 0; mPinned = false; ZERO_MEM(allocStart,sizeof(int)*IMMIX_LINES); ZERO_MEM(mPtr->mRowMarked+IMMIX_HEADER_LINES, IMMIX_USEFUL_LINES); @@ -823,7 +825,7 @@ struct BlockDataInfo void clearBlockMarks() { - mPinned = false; + mPinned = mPinnedAllocs == 0 ? false : true; #ifdef HXCPP_GC_GENERATIONAL mHasSurvivor = false; #endif @@ -2376,7 +2378,6 @@ void GCRemoveRoot(hx::Object **inRoot) sgRootSet.erase(inRoot); } - void GcAddOffsetRoot(void *inRoot, int inOffset) { AutoLock lock(*sGCRootLock); @@ -2907,6 +2908,8 @@ typedef hx::QuickVec BlockList; typedef hx::QuickVec LargeList; +enum MemType { memUnmanaged, memBlock, memLarge }; + @@ -5522,7 +5525,7 @@ class GlobalAllocator return false; } - hx::MemType GetMemType(void *inPtr) + MemType GetMemType(void *inPtr) { BlockData *block = (BlockData *)( ((size_t)inPtr) & IMMIX_BLOCK_BASE_MASK); @@ -5540,16 +5543,16 @@ class GlobalAllocator */ if (isBlock) - return hx::memBlock; + return memBlock; for(int i=0;i=500) +void GCPinPtr(const void* inPtr) +{ + if (IsConstAlloc(inPtr)) + { + return; + } + + auto ptr_i = reinterpret_cast(inPtr) - sizeof(int); + auto flags = *reinterpret_cast(ptr_i); + auto onLOH = (flags & 0xffff) == 0; + + if (!onLOH) + { + auto block = reinterpret_cast(ptr_i & IMMIX_BLOCK_BASE_MASK); + auto info = (*gBlockInfo)[block->mId]; + + info->mPinnedAllocs++; + } +} + +void GCUnpinPtr(const void* inPtr) +{ + if (IsConstAlloc(inPtr)) + { + return; + } + + auto ptr_i = reinterpret_cast(inPtr) - sizeof(int); + auto flags = *reinterpret_cast(ptr_i); + auto onLOH = (flags & 0xffff) == 0; + + if (!onLOH) + { + auto block = reinterpret_cast((reinterpret_cast(inPtr)) & IMMIX_BLOCK_BASE_MASK); + auto info = (*gBlockInfo)[block->mId]; + + info->mPinnedAllocs--; + } +} +#endif MarkChunk *MarkChunk::swapForNew() { @@ -5742,11 +5786,6 @@ void MarkConservative(int *inBottom, int *inTop,hx::MarkContext *__inCtx) #endif } -MemType GetMemType(void* inPtr) -{ - return sGlobalAlloc->GetMemType(inPtr); -} - } // namespace hx From c1b1bf0e33b475d277a92836871e69928e688588 Mon Sep 17 00:00:00 2001 From: Aidan Lee Date: Sat, 6 Jan 2024 11:45:02 +0000 Subject: [PATCH 3/4] Change to a pin set --- include/Array.h | 17 +++----- include/hx/GC.h | 4 +- src/Array.cpp | 65 ++++++------------------------- src/hx/gc/Immix.cpp | 94 +++++++++++++++++++++++++++------------------ 4 files changed, 75 insertions(+), 105 deletions(-) diff --git a/include/Array.h b/include/Array.h index 787fe507e..7e60eaa36 100644 --- a/include/Array.h +++ b/include/Array.h @@ -127,16 +127,15 @@ class ArrayKeyValueIterator : public cpp::FastIterator_obj namespace hx { #if (HXCPP_API_LEVEL>=500) -class HXCPP_EXTERN_CLASS_ATTRIBUTES ArrayPin { - char* ptr; - hx::Object* object; - +class HXCPP_EXTERN_CLASS_ATTRIBUTES ArrayPin +{ + char* ptr; public: - ArrayPin(char* inPtr, hx::Object* inObject); + ArrayPin(char* inPtr); - ~ArrayPin(); + ~ArrayPin(); - char* GetBase(); + char* GetBase(); }; #endif @@ -423,10 +422,6 @@ class HXCPP_EXTERN_CLASS_ATTRIBUTES ArrayBase : public ArrayCommon protected: mutable int mAlloc; mutable char *mBase; - -#if (HXCPP_API_LEVEL>=500) - bool hasBeenPinned; -#endif }; } // end namespace hx for ArrayBase diff --git a/include/hx/GC.h b/include/hx/GC.h index e82bb76cf..f5835cad1 100644 --- a/include/hx/GC.h +++ b/include/hx/GC.h @@ -188,8 +188,8 @@ void GCAddRoot(hx::Object **inRoot); void GCRemoveRoot(hx::Object **inRoot); #if (HXCPP_API_LEVEL>=500) -void GCPinPtr(const void* inPtr); -void GCUnpinPtr(const void* inPtr); +void GCPinPtr(void* inPtr); +void GCUnpinPtr(void* inPtr); #endif // This is used internally in hxcpp diff --git a/src/Array.cpp b/src/Array.cpp index 578dfcbd2..594156983 100644 --- a/src/Array.cpp +++ b/src/Array.cpp @@ -34,64 +34,33 @@ ArrayBase::ArrayBase(int inSize,int inReserve,int inElementSize,bool inAtomic) mAlloc = alloc; mArrayConvertId = inAtomic ? inElementSize : inElementSize==sizeof(String) ? aciStringArray : aciObjectArray; - -#if (HXCPP_API_LEVEL>=500) - hasBeenPinned = false; -#endif } #if (HXCPP_API_LEVEL>=500) -ArrayPin::ArrayPin(char* inPtr, hx::Object* inObject) - : ptr(inPtr) - , object(inObject) +ArrayPin::ArrayPin(char* inPtr) : ptr(inPtr) { - hx::GCPinPtr(ptr); - hx::GCAddRoot(&object); + hx::GCPinPtr(ptr); } ArrayPin::~ArrayPin() { - hx::GCUnpinPtr(ptr); - hx::GCAddRoot(&object); + hx::GCUnpinPtr(ptr); } char* ArrayPin::GetBase() { - return ptr; + return ptr; } ArrayPin* ArrayBase::Pin() { - class Marker : public hx::Object - { - char* mBase; - - public: - Marker(char* inBase) : mBase(inBase) - { - HX_OBJ_WB_NEW_MARKED_OBJECT(this); - } - - void __Mark(hx::MarkContext* ctx) - { - hx::MarkAlloc(mBase, ctx); - } - }; - - if (!mBase) - { - return nullptr; - } - - // Unmanaged data, is it safe to put this in the ArrayPin if we don't know the lifetime of that pointer? - if (mAlloc < 0) - { - return new ArrayPin(mBase, nullptr); - } - - hasBeenPinned = true; - - return new ArrayPin(mBase, new Marker(mBase)); + if (!mBase) + { + return nullptr; + } + + // If the array holds unmanaged data is it safe to put this in the ArrayPin if we don't know the lifetime of that pointer? + return new ArrayPin(mBase); } #endif @@ -105,11 +74,7 @@ void ArrayBase::reserve(int inSize) const if (mBase) { bool wasUnamanaged = mAlloc<0; -#if (HXCPP_API_LEVEL>=500) - if (wasUnamanaged || hasBeenPinned) -#else if (wasUnamanaged) -#endif { char *base=(char *)hx::InternalNew(bytes,false); memcpy(base,mBase,length*elemSize); @@ -160,11 +125,7 @@ void ArrayBase::Realloc(int inSize) const if (mBase) { bool wasUnamanaged = mAlloc<0; -#if (HXCPP_API_LEVEL>=500) - if (wasUnamanaged || hasBeenPinned) -#else if (wasUnamanaged) -#endif { char *base=(char *)hx::InternalNew(bytes,false); memcpy(base,mBase,length*elemSize); @@ -293,11 +254,7 @@ void ArrayBase::__SetSizeExact(int inSize) { bool wasUnamanaged = mAlloc<0; -#if (HXCPP_API_LEVEL>=500) - if (wasUnamanaged || hasBeenPinned) -#else if (wasUnamanaged) -#endif { char *base=(char *)(AllocAtomic() ? hx::NewGCPrivate(0,bytes) : hx::NewGCBytes(0,bytes)); memcpy(base,mBase,std::min(length,inSize)*elemSize); diff --git a/src/hx/gc/Immix.cpp b/src/hx/gc/Immix.cpp index 61f93797e..dfb6aea92 100644 --- a/src/hx/gc/Immix.cpp +++ b/src/hx/gc/Immix.cpp @@ -741,7 +741,6 @@ struct BlockDataInfo int mMoveScore; int mUsedBytes; int mFraggedRows; - int mPinnedAllocs; bool mPinned; unsigned char mZeroed; bool mReclaimed; @@ -790,7 +789,6 @@ struct BlockDataInfo mUsedRows = 0; mUsedBytes = 0; mFraggedRows = 0; - mPinnedAllocs = 0; mPinned = false; ZERO_MEM(allocStart,sizeof(int)*IMMIX_LINES); ZERO_MEM(mPtr->mRowMarked+IMMIX_HEADER_LINES, IMMIX_USEFUL_LINES); @@ -825,7 +823,7 @@ struct BlockDataInfo void clearBlockMarks() { - mPinned = mPinnedAllocs == 0 ? false : true; + mPinned = false; #ifdef HXCPP_GC_GENERATIONAL mHasSurvivor = false; #endif @@ -2366,6 +2364,9 @@ static RootSet sgRootSet; typedef hx::UnorderedMap OffsetRootSet; static OffsetRootSet *sgOffsetRootSet=0; +typedef hx::UnorderedSet PinSet; +static PinSet sgPinSet; + void GCAddRoot(hx::Object **inRoot) { AutoLock lock(*sGCRootLock); @@ -3185,6 +3186,14 @@ class GlobalAllocator unsigned int *blob = ((unsigned int *)inLarge) - 2; unsigned int size = *blob; mLargeListLock.Lock(); + + if (hx::sgPinSet.count(inLarge)) + { + mLargeListLock.Unlock(); + + return; + } + mLargeAllocated -= size; // Could somehow keep it in the list, but mark as recycled? mLargeList.qerase_val(blob); @@ -4716,11 +4725,38 @@ class GlobalAllocator int offset = i->second; hx::Object *obj = (hx::Object *)(ptr - offset); - if (obj) + if (!obj) hx::MarkObjectAlloc(obj , &mMarker ); } } // automark + { + hx::AutoMarkPush info(&mMarker, "Pins", "pin"); + + for (hx::PinSet::iterator i = hx::sgPinSet.begin(); i != hx::sgPinSet.end(); ++i) + { + void* const ptr = *i; + if (!ptr) + { + continue; + } + + auto ptr_i = reinterpret_cast(ptr) - sizeof(int); + auto flags = *reinterpret_cast(ptr_i); + auto onLOH = (flags & 0xffff) == 0; + + if (!onLOH) + { + BlockData* block = reinterpret_cast(reinterpret_cast(ptr) & IMMIX_BLOCK_BASE_MASK); + BlockDataInfo* info = (*gBlockInfo)[block->mId]; + + info->pin(); + } + + hx::MarkAlloc(ptr, &mMarker); + } + } + #ifdef PROFILE_COLLECT hx::rootObjects = sObjectMarks; hx::rootAllocs = sAllocMarks; @@ -5586,45 +5622,27 @@ class GlobalAllocator namespace hx { -#if (HXCPP_API_LEVEL>=500) -void GCPinPtr(const void* inPtr) +#if (HXCPP_API_LEVEL>=430) +void GCPinPtr(void* inPtr) { - if (IsConstAlloc(inPtr)) - { - return; - } - - auto ptr_i = reinterpret_cast(inPtr) - sizeof(int); - auto flags = *reinterpret_cast(ptr_i); - auto onLOH = (flags & 0xffff) == 0; - - if (!onLOH) - { - auto block = reinterpret_cast(ptr_i & IMMIX_BLOCK_BASE_MASK); - auto info = (*gBlockInfo)[block->mId]; - - info->mPinnedAllocs++; - } + if (IsConstAlloc(inPtr)) + { + return; + } + + AutoLock(sGlobalAlloc->mLargeListLock); + sgPinSet.emplace(inPtr); } -void GCUnpinPtr(const void* inPtr) +void GCUnpinPtr(void* inPtr) { - if (IsConstAlloc(inPtr)) - { - return; - } - - auto ptr_i = reinterpret_cast(inPtr) - sizeof(int); - auto flags = *reinterpret_cast(ptr_i); - auto onLOH = (flags & 0xffff) == 0; - - if (!onLOH) - { - auto block = reinterpret_cast((reinterpret_cast(inPtr)) & IMMIX_BLOCK_BASE_MASK); - auto info = (*gBlockInfo)[block->mId]; + if (IsConstAlloc(inPtr)) + { + return; + } - info->mPinnedAllocs--; - } + AutoLock(sGlobalAlloc->mLargeListLock); + sgPinSet.erase(inPtr); } #endif From 9e48683364cbce58a2d361cf4f153ce31b6bd1c9 Mon Sep 17 00:00:00 2001 From: Aidan Lee Date: Sat, 6 Jan 2024 12:57:21 +0000 Subject: [PATCH 4/4] Switch inverted if check --- src/hx/gc/Immix.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hx/gc/Immix.cpp b/src/hx/gc/Immix.cpp index dfb6aea92..b20d50a7d 100644 --- a/src/hx/gc/Immix.cpp +++ b/src/hx/gc/Immix.cpp @@ -4725,8 +4725,8 @@ class GlobalAllocator int offset = i->second; hx::Object *obj = (hx::Object *)(ptr - offset); - if (!obj) - hx::MarkObjectAlloc(obj , &mMarker ); + if (obj) + hx::MarkObjectAlloc(obj , &mMarker); } } // automark