diff --git a/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx b/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx index aed6efdb62..8e00cabea8 100644 --- a/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx +++ b/src/FoundationClasses/TKernel/GTests/NCollection_LocalArray_Test.cxx @@ -342,4 +342,202 @@ TEST(NCollection_LocalArrayTest, ReallocateAsGrowableStack) { EXPECT_EQ(i * 3, aStack[--aTop]); } +} + +// ============ Non-trivially-copyable type tests ============ + +// Tracker for construction/destruction call counts. +static int THE_CTOR_COUNT = 0; +static int THE_DTOR_COUNT = 0; +static int THE_MOVE_COUNT = 0; + +struct NCollection_LocalArray_Tracked +{ + int Value = 0; + + NCollection_LocalArray_Tracked() { ++THE_CTOR_COUNT; } + + ~NCollection_LocalArray_Tracked() { ++THE_DTOR_COUNT; } + + NCollection_LocalArray_Tracked(const NCollection_LocalArray_Tracked& theOther) + : Value(theOther.Value) + { + ++THE_CTOR_COUNT; + } + + NCollection_LocalArray_Tracked(NCollection_LocalArray_Tracked&& theOther) noexcept + : Value(theOther.Value) + { + theOther.Value = -1; + ++THE_MOVE_COUNT; + } + + NCollection_LocalArray_Tracked& operator=(NCollection_LocalArray_Tracked&& theOther) noexcept + { + Value = theOther.Value; + theOther.Value = -1; + ++THE_MOVE_COUNT; + return *this; + } + + NCollection_LocalArray_Tracked& operator=(const NCollection_LocalArray_Tracked&) = default; +}; + +static void resetTrackedCounters() +{ + THE_CTOR_COUNT = 0; + THE_DTOR_COUNT = 0; + THE_MOVE_COUNT = 0; +} + +static_assert(!std::is_trivially_copyable_v, + "Tracked type must be non-trivially copyable for these tests"); + +TEST(NCollection_LocalArrayTest, NonTrivial_SmallAllocation) +{ + resetTrackedCounters(); + { + NCollection_LocalArray anArr(4); + EXPECT_EQ(4u, anArr.Size()); + EXPECT_EQ(4, THE_CTOR_COUNT); + + anArr[0].Value = 10; + anArr[1].Value = 20; + anArr[2].Value = 30; + anArr[3].Value = 40; + EXPECT_EQ(30, anArr[2].Value); + } + // All 4 elements destroyed. + EXPECT_EQ(4, THE_DTOR_COUNT); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_HeapAllocation) +{ + resetTrackedCounters(); + { + NCollection_LocalArray anArr(8); + EXPECT_EQ(8u, anArr.Size()); + + for (size_t i = 0; i < 8; ++i) + anArr[i].Value = static_cast(i * 100); + + for (size_t i = 0; i < 8; ++i) + EXPECT_EQ(static_cast(i * 100), anArr[i].Value); + } + EXPECT_EQ(THE_CTOR_COUNT + THE_MOVE_COUNT, THE_DTOR_COUNT); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_ReallocateWithCopy_InlineToInline) +{ + NCollection_LocalArray anArr(4); + for (size_t i = 0; i < 4; ++i) + anArr[i].Value = static_cast(i + 1); + + anArr.Reallocate(8, true); + EXPECT_EQ(8u, anArr.Size()); + + // Original elements preserved. + for (size_t i = 0; i < 4; ++i) + EXPECT_EQ(static_cast(i + 1), anArr[i].Value); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_ReallocateWithCopy_InlineToHeap) +{ + NCollection_LocalArray anArr(4); + for (size_t i = 0; i < 4; ++i) + anArr[i].Value = static_cast(i * 10); + + anArr.Reallocate(16, true); + EXPECT_EQ(16u, anArr.Size()); + + for (size_t i = 0; i < 4; ++i) + EXPECT_EQ(static_cast(i * 10), anArr[i].Value); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_ReallocateWithCopy_HeapToHeap) +{ + NCollection_LocalArray anArr(8); + for (size_t i = 0; i < 8; ++i) + anArr[i].Value = static_cast(i * 5); + + anArr.Reallocate(16, true); + EXPECT_EQ(16u, anArr.Size()); + + for (size_t i = 0; i < 8; ++i) + EXPECT_EQ(static_cast(i * 5), anArr[i].Value); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_Shrink_DestroysExcess) +{ + resetTrackedCounters(); + { + NCollection_LocalArray anArr(8); + EXPECT_EQ(8, THE_CTOR_COUNT); + + anArr.Reallocate(3, true); + EXPECT_EQ(3u, anArr.Size()); + // 5 excess elements destroyed. + EXPECT_EQ(5, THE_DTOR_COUNT); + } + // Remaining 3 destroyed in destructor. + EXPECT_EQ(8, THE_DTOR_COUNT); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_MoveConstructor_FromInline) +{ + NCollection_LocalArray aSrc(4); + for (size_t i = 0; i < 4; ++i) + aSrc[i].Value = static_cast(i * 7); + + NCollection_LocalArray aDst(std::move(aSrc)); + + EXPECT_EQ(4u, aDst.Size()); + for (size_t i = 0; i < 4; ++i) + EXPECT_EQ(static_cast(i * 7), aDst[i].Value); + + EXPECT_EQ(0u, aSrc.Size()); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_MoveConstructor_FromHeap) +{ + NCollection_LocalArray aSrc(8); + for (size_t i = 0; i < 8; ++i) + aSrc[i].Value = static_cast(i * 3); + + NCollection_LocalArray aDst(std::move(aSrc)); + + EXPECT_EQ(8u, aDst.Size()); + for (size_t i = 0; i < 8; ++i) + EXPECT_EQ(static_cast(i * 3), aDst[i].Value); + + EXPECT_EQ(0u, aSrc.Size()); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_MoveAssignment) +{ + NCollection_LocalArray aSrc(4); + for (size_t i = 0; i < 4; ++i) + aSrc[i].Value = static_cast(i + 100); + + NCollection_LocalArray aDst(2); + aDst = std::move(aSrc); + + EXPECT_EQ(4u, aDst.Size()); + for (size_t i = 0; i < 4; ++i) + EXPECT_EQ(static_cast(i + 100), aDst[i].Value); + + EXPECT_EQ(0u, aSrc.Size()); +} + +TEST(NCollection_LocalArrayTest, NonTrivial_DestructorCallBalance) +{ + resetTrackedCounters(); + { + NCollection_LocalArray anArr(4); + anArr.Reallocate(8, true); // inline -> heap: 4 moves + 4 new + 4 destroy old + anArr.Reallocate(2, true); // shrink: 6 destroyed + anArr.Reallocate(16, true); // heap -> bigger heap: 2 moves + 14 new + 2 destroy old + } + // Total constructions (ctor + move) must equal total destructions. + EXPECT_EQ(THE_CTOR_COUNT + THE_MOVE_COUNT, THE_DTOR_COUNT); } \ No newline at end of file diff --git a/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx b/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx index 41ba425d7a..b80a6fd194 100644 --- a/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx +++ b/src/FoundationClasses/TKernel/NCollection/NCollection_LocalArray.hxx @@ -21,85 +21,151 @@ #include #include #include +#include //! Auxiliary class optimizing creation of array buffer //! (using stack allocation for small arrays). +//! +//! For trivially copyable types the fast memcpy / Standard::Reallocate path +//! is used. For non-trivially-copyable types (Handle, TopLoc_Location, etc.) +//! the class uses placement new, move semantics, and explicit destructors +//! while keeping Standard::Allocate / Standard::Free for heap management. +//! +//! Non-trivially-copyable types must be default-constructible and +//! nothrow-move-constructible. template class NCollection_LocalArray { - static_assert(std::is_trivially_copyable::value, - "NCollection_LocalArray uses memcpy/realloc and requires trivially copyable types"); + static constexpr bool IS_TRIVIAL = std::is_trivially_copyable_v; public: explicit NCollection_LocalArray(const size_t theSize) - : myPtr(myBuffer), + : myPtr(inlinePtr()), mySize(0) { Allocate(theSize); } NCollection_LocalArray() noexcept - : myPtr(myBuffer), + : myPtr(inlinePtr()), mySize(0) { } - ~NCollection_LocalArray() { Deallocate(); } + ~NCollection_LocalArray() + { + if constexpr (!IS_TRIVIAL) + { + for (size_t i = 0; i < mySize; ++i) + myPtr[i].~theItem(); + } + Deallocate(); + } void Allocate(const size_t theSize) { Reallocate(theSize, false); } //! Reallocate the array to a new size. //! @param[in] theNewSize new number of elements - //! @param[in] theToCopy if true, existing elements are copied to the new buffer + //! @param[in] theToCopy if true, existing elements are copied/moved to the new buffer void Reallocate(const size_t theNewSize, bool theToCopy = true) { if (theNewSize <= mySize) { - // Shrinking - just update the logical size, keep existing allocation + // Shrinking - just update the logical size, keep existing allocation. + if constexpr (!IS_TRIVIAL) + { + for (size_t i = theNewSize; i < mySize; ++i) + myPtr[i].~theItem(); + } mySize = theNewSize; return; } - const bool isOnHeap = (myPtr != myBuffer); - const size_t aNewBytes = theNewSize * sizeof(theItem); + const bool aWasInline = isInline(); + const size_t aNewBytes = theNewSize * sizeof(theItem); if (theNewSize <= static_cast(MAX_ARRAY_SIZE)) { - // New size fits in stack buffer - if (isOnHeap) + // New size fits in inline buffer. + if (!aWasInline) { - if (theToCopy && mySize > 0) + if constexpr (IS_TRIVIAL) { - std::memcpy(myBuffer, myPtr, std::min(mySize, theNewSize) * sizeof(theItem)); + if (theToCopy && mySize > 0) + { + std::memcpy(inlinePtr(), myPtr, std::min(mySize, theNewSize) * sizeof(theItem)); + } + Standard::Free(myPtr); + } + else + { + theItem* anOldPtr = myPtr; + const size_t anOldSize = mySize; + const size_t aCopy = theToCopy ? std::min(anOldSize, theNewSize) : 0; + myPtr = inlinePtr(); + for (size_t i = 0; i < aCopy; ++i) + new (myPtr + i) theItem(std::move(anOldPtr[i])); + for (size_t i = aCopy; i < theNewSize; ++i) + new (myPtr + i) theItem(); + for (size_t i = 0; i < anOldSize; ++i) + anOldPtr[i].~theItem(); + Standard::Free(anOldPtr); + } + myPtr = inlinePtr(); + } + else + { + // Already inline, growing within buffer. + if constexpr (!IS_TRIVIAL) + { + for (size_t i = mySize; i < theNewSize; ++i) + new (myPtr + i) theItem(); } - Standard::Free(myPtr); - myPtr = myBuffer; } mySize = theNewSize; return; } - if (isOnHeap) + // Need heap allocation (theNewSize > MAX_ARRAY_SIZE). + if constexpr (IS_TRIVIAL) { - // Already on heap - use Standard::Reallocate (preserves content when growing) - if (theToCopy) + if (!aWasInline) { - myPtr = (theItem*)Standard::Reallocate(myPtr, aNewBytes); + if (theToCopy) + { + myPtr = static_cast(Standard::Reallocate(myPtr, aNewBytes)); + } + else + { + Standard::Free(myPtr); + myPtr = static_cast(Standard::Allocate(aNewBytes)); + } } else { - Standard::Free(myPtr); - myPtr = (theItem*)Standard::Allocate(aNewBytes); + theItem* aNewPtr = static_cast(Standard::Allocate(aNewBytes)); + if (theToCopy && mySize > 0) + { + std::memcpy(aNewPtr, myPtr, std::min(mySize, theNewSize) * sizeof(theItem)); + } + myPtr = aNewPtr; } } else { - // Stack to heap transition - myPtr = (theItem*)Standard::Allocate(aNewBytes); - if (theToCopy && mySize > 0) - { - std::memcpy(myPtr, myBuffer, std::min(mySize, theNewSize) * sizeof(theItem)); - } + // Non-trivial: Standard::Reallocate (realloc) cannot be used because + // it does not call constructors or destructors. + theItem* aNewPtr = static_cast(Standard::Allocate(aNewBytes)); + const size_t aCopy = theToCopy ? std::min(mySize, theNewSize) : 0; + for (size_t i = 0; i < aCopy; ++i) + new (aNewPtr + i) theItem(std::move(myPtr[i])); + for (size_t i = aCopy; i < theNewSize; ++i) + new (aNewPtr + i) theItem(); + for (size_t i = 0; i < mySize; ++i) + myPtr[i].~theItem(); + if (!aWasInline) + Standard::Free(myPtr); + myPtr = aNewPtr; } mySize = theNewSize; } @@ -109,48 +175,87 @@ public: operator theItem*() const noexcept { return myPtr; } NCollection_LocalArray(NCollection_LocalArray&& theOther) noexcept - : myPtr(myBuffer), + : myPtr(inlinePtr()), mySize(theOther.mySize) { - if (theOther.myPtr == theOther.myBuffer) + if (theOther.isInline()) { - std::memcpy(myBuffer, theOther.myBuffer, mySize * sizeof(theItem)); + if constexpr (IS_TRIVIAL) + { + std::memcpy(inlinePtr(), theOther.inlinePtr(), mySize * sizeof(theItem)); + } + else + { + for (size_t i = 0; i < mySize; ++i) + new (inlinePtr() + i) theItem(std::move(theOther.inlinePtr()[i])); + for (size_t i = 0; i < mySize; ++i) + theOther.inlinePtr()[i].~theItem(); + } } else { myPtr = theOther.myPtr; - theOther.myPtr = theOther.myBuffer; + theOther.myPtr = theOther.inlinePtr(); } theOther.mySize = 0; } NCollection_LocalArray& operator=(NCollection_LocalArray&& theOther) noexcept { - if (this != &theOther) + if (this == &theOther) + return *this; + + if constexpr (IS_TRIVIAL) { mySize = theOther.mySize; - if (theOther.myPtr == theOther.myBuffer) + if (theOther.isInline()) { - // Source on stack: copy data to our buffer Deallocate(); - myPtr = myBuffer; - std::memcpy(myBuffer, theOther.myBuffer, mySize * sizeof(theItem)); + myPtr = inlinePtr(); + std::memcpy(inlinePtr(), theOther.inlinePtr(), mySize * sizeof(theItem)); } - else if (myPtr != myBuffer) + else if (!isInline()) { - // Both on heap: swap pointers, theOther frees our old allocation on destruction + // Both on heap: swap pointers, theOther frees our old allocation on destruction. theItem* anOldPtr = myPtr; myPtr = theOther.myPtr; theOther.myPtr = anOldPtr; } else { - // this on stack, theOther on heap: take the pointer + // this on inline, theOther on heap: take the pointer. myPtr = theOther.myPtr; - theOther.myPtr = theOther.myBuffer; + theOther.myPtr = theOther.inlinePtr(); } - theOther.mySize = 0; } + else + { + // Destroy our current elements. + for (size_t i = 0; i < mySize; ++i) + myPtr[i].~theItem(); + + if (theOther.isInline()) + { + if (!isInline()) + Standard::Free(myPtr); + myPtr = inlinePtr(); + mySize = theOther.mySize; + for (size_t i = 0; i < mySize; ++i) + new (inlinePtr() + i) theItem(std::move(theOther.inlinePtr()[i])); + for (size_t i = 0; i < mySize; ++i) + theOther.inlinePtr()[i].~theItem(); + } + else + { + // Take ownership of theOther's heap allocation directly. + if (!isInline()) + Standard::Free(myPtr); + myPtr = theOther.myPtr; + mySize = theOther.mySize; + theOther.myPtr = theOther.inlinePtr(); + } + } + theOther.mySize = 0; return *this; } @@ -160,14 +265,45 @@ public: protected: void Deallocate() { - if (myPtr != myBuffer) + if (!isInline()) Standard::Free(myPtr); } + //! Pointer to inline buffer storage. + theItem* inlinePtr() noexcept { return myStorage.ptr(); } + + const theItem* inlinePtr() const noexcept { return myStorage.ptr(); } + + //! True if currently using inline (stack) storage. + bool isInline() const noexcept { return myPtr == myStorage.ptr(); } + + //! Inline storage for trivial types: typed array with direct element access. + struct InlineStorageTrivial + { + theItem myData[MAX_ARRAY_SIZE]; + + theItem* ptr() noexcept { return myData; } + + const theItem* ptr() const noexcept { return myData; } + }; + + //! Inline storage for non-trivial types: raw aligned bytes for placement new. + struct InlineStorageNonTrivial + { + alignas(theItem) char myData[MAX_ARRAY_SIZE * sizeof(theItem)]; + + theItem* ptr() noexcept { return reinterpret_cast(myData); } + + const theItem* ptr() const noexcept { return reinterpret_cast(myData); } + }; + + using InlineStorage = + std::conditional_t; + protected: - theItem myBuffer[MAX_ARRAY_SIZE]; - theItem* myPtr; - size_t mySize; + InlineStorage myStorage; //!< Inline buffer + theItem* myPtr; //!< Points to inline or heap buffer + size_t mySize; //!< Logical element count }; #endif // _NCollection_LocalArray_HeaderFile diff --git a/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.cxx b/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.cxx index 9135580c41..b308202790 100644 --- a/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.cxx +++ b/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.cxx @@ -38,8 +38,7 @@ inline bool isMoreComplex(const TopAbs_ShapeEnum theType, const TopAbs_ShapeEnum //================================================================================================= TopExp_Explorer::TopExp_Explorer() noexcept - : myStack(20), - toFind(TopAbs_SHAPE), + : toFind(TopAbs_SHAPE), toAvoid(TopAbs_SHAPE), hasMore(false) { @@ -50,8 +49,7 @@ TopExp_Explorer::TopExp_Explorer() noexcept TopExp_Explorer::TopExp_Explorer(const TopoDS_Shape& S, const TopAbs_ShapeEnum ToFind, const TopAbs_ShapeEnum ToAvoid) - : myStack(20), - toFind(ToFind), + : toFind(ToFind), toAvoid(ToAvoid), hasMore(false) { @@ -109,7 +107,7 @@ void TopExp_Explorer::Init(const TopoDS_Shape& S, void TopExp_Explorer::Next() { - if (myStack.IsEmpty()) + if (myStackTop < 0) { TopAbs_ShapeEnum ty = myShape.ShapeType(); @@ -125,15 +123,15 @@ void TopExp_Explorer::Next() } else { - myStack.Append(TopoDS_Iterator(myShape)); + pushIterator(TopoDS_Iterator(myShape)); } } else - myStack.ChangeLast().Next(); + myStack[myStackTop].Next(); for (;;) { - TopoDS_Iterator& aTopIter = myStack.ChangeLast(); + TopoDS_Iterator& aTopIter = myStack[myStackTop]; if (aTopIter.More()) { @@ -147,8 +145,8 @@ void TopExp_Explorer::Next() } else if (isMoreComplex(ty, toFind) && !shouldAvoid(ty, toAvoid)) { - myStack.Append(TopoDS_Iterator(aShapTop)); - // aTopIter reference is now invalid after Append + pushIterator(TopoDS_Iterator(aShapTop)); + // aTopIter reference is now invalid after push } else { @@ -157,10 +155,10 @@ void TopExp_Explorer::Next() } else { - myStack.EraseLast(); - if (myStack.IsEmpty()) + popIterator(); + if (myStackTop < 0) break; - myStack.ChangeLast().Next(); + myStack[myStackTop].Next(); } } hasMore = false; @@ -170,20 +168,46 @@ void TopExp_Explorer::Next() const TopoDS_Shape& TopExp_Explorer::Current() const noexcept { - return myStack.IsEmpty() ? myShape : myStack.Last().Value(); + return myStackTop < 0 ? myShape : myStack[myStackTop].Value(); } //================================================================================================= int TopExp_Explorer::Depth() const noexcept { - return myStack.Length(); + return myStackTop + 1; } //================================================================================================= void TopExp_Explorer::Clear() { - hasMore = false; - myStack.Clear(); + // Shrink to 0 - NCollection_LocalArray destroys all live elements. + if (myStack.Size() > 0) + myStack.Reallocate(0); + myStackTop = -1; + hasMore = false; +} + +//================================================================================================= + +void TopExp_Explorer::pushIterator(TopoDS_Iterator&& theIter) +{ + ++myStackTop; + if (static_cast(myStackTop) >= myStack.Size()) + { + const size_t aNewSize = + std::max(myStack.Size() * 2, static_cast(THE_INLINE_STACK_SIZE)); + myStack.Reallocate(aNewSize, true); + } + myStack[myStackTop] = std::move(theIter); +} + +//================================================================================================= + +void TopExp_Explorer::popIterator() +{ + // Reset the popped iterator to release shape references immediately. + myStack[myStackTop] = TopoDS_Iterator(); + --myStackTop; } diff --git a/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.hxx b/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.hxx index fbc5faa169..d153473363 100644 --- a/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.hxx +++ b/src/ModelingData/TKBRep/TopExp/TopExp_Explorer.hxx @@ -17,7 +17,7 @@ #ifndef _TopExp_Explorer_HeaderFile #define _TopExp_Explorer_HeaderFile -#include +#include #include #include #include @@ -139,11 +139,22 @@ public: Standard_EXPORT ~TopExp_Explorer(); private: - NCollection_Vector myStack; - TopoDS_Shape myShape; - TopAbs_ShapeEnum toFind; - TopAbs_ShapeEnum toAvoid; - bool hasMore; + //! Push a new iterator onto the stack (placement new on first use, assign on reuse). + void pushIterator(TopoDS_Iterator&& theIter); + + //! Pop the top iterator (explicit destructor call). + void popIterator(); + + static constexpr int THE_INLINE_STACK_SIZE = + 20; //!< Inline stack capacity (covers all topology depths) + + NCollection_LocalArray + myStack; //!< DFS stack (lazy allocation) + int myStackTop = -1; //!< Top of stack index (-1 = empty) + TopoDS_Shape myShape; + TopAbs_ShapeEnum toFind; + TopAbs_ShapeEnum toAvoid; + bool hasMore; }; #endif // _TopExp_Explorer_HeaderFile