diff --git a/Foundation/include/Poco/Any.h b/Foundation/include/Poco/Any.h index 517e6d344..165b8a3ff 100644 --- a/Foundation/include/Poco/Any.h +++ b/Foundation/include/Poco/Any.h @@ -38,6 +38,16 @@ template class VarHolderImpl; } +template +struct TypeSizeLE: + std::integral_constant{}; + + +template +struct TypeSizeGT: + std::integral_constant S)>{}; + + #ifndef POCO_NO_SOO @@ -58,14 +68,38 @@ public: static const unsigned int value = SizeV; }; - Placeholder() + Placeholder(const Placeholder&) = delete; + Placeholder(Placeholder&&) = delete; + Placeholder& operator=(const Placeholder&) = delete; + Placeholder& operator=(Placeholder&&) = delete; + + Placeholder(): pHolder(0) { - erase(); + std::memset(holder, 0, sizeof(Placeholder)); + } + + ~Placeholder() + { + destruct(false); + } + + void swap(Placeholder& other) + { + if (!isLocal() && !other.isLocal()) + std::swap(pHolder, other.pHolder); + else + throw Poco::InvalidAccessException("Placeholder::swap()"); } void erase() { - std::memset(holder, 0, sizeof(Placeholder)); + destruct(true); + } + + bool isEmpty() const + { + char buf[POCO_SMALL_OBJECT_SIZE] = {}; + return 0 == std::memcmp(holder, buf, POCO_SMALL_OBJECT_SIZE); } bool isLocal() const @@ -78,6 +112,24 @@ public: holder[SizeV] = local ? 1 : 0; } + template + PlaceholderT* assignStack(const V& value) + { + erase(); + new (reinterpret_cast(holder)) T(value); + setLocal(true); + return reinterpret_cast(holder); + } + + template + PlaceholderT* assignHeap(const V& value) + { + erase(); + pHolder = new T(value); + setLocal(false); + return pHolder; + } + PlaceholderT* content() const { if (isLocal()) @@ -86,20 +138,25 @@ public: return pHolder; } -// MSVC71,80 won't extend friendship to nested class (Any::Holder) -#if !defined(POCO_MSVC_VERSION) || (defined(POCO_MSVC_VERSION) && (POCO_MSVC_VERSION > 80)) private: -#endif - typedef typename std::aligned_storage::type AlignerType; + typedef typename std::aligned_storage::type AlignerType; - PlaceholderT* pHolder; - mutable char holder[SizeV + 1]; - AlignerType aligner; + void destruct(bool clear) + { + if (!isEmpty()) + { + if (!isLocal()) + delete pHolder; + else + reinterpret_cast(holder)->~PlaceholderT(); - friend class Any; - friend class Dynamic::Var; - friend class Dynamic::VarHolder; - template friend class Dynamic::VarHolderImpl; + if (clear) std::memset(holder, 0, sizeof(Placeholder)); + } + } + + PlaceholderT* pHolder; + mutable unsigned char holder[SizeV+1]; + AlignerType aligner; }; @@ -185,13 +242,6 @@ public: /// Destructor. If Any is locally held, calls ValueHolder destructor; /// otherwise, deletes the placeholder from the heap. { - if (!empty()) - { - if (_valueHolder.isLocal()) - destruct(); - else - delete content(); - } } Any& swap(Any& other) @@ -205,14 +255,13 @@ public: if (!_valueHolder.isLocal() && !other._valueHolder.isLocal()) { - std::swap(_valueHolder.pHolder, other._valueHolder.pHolder); + _valueHolder.swap(other._valueHolder); } else { Any tmp(*this); try { - if (_valueHolder.isLocal()) destruct(); construct(other); other = tmp; } @@ -252,8 +301,7 @@ public: bool empty() const /// Returns true if the Any is empty. { - char buf[POCO_SMALL_OBJECT_SIZE] = { 0 }; - return 0 == std::memcmp(_valueHolder.holder, buf, POCO_SMALL_OBJECT_SIZE); + return _valueHolder.isEmpty(); } const std::type_info & type() const @@ -290,21 +338,27 @@ private: virtual void clone(Placeholder* pPlaceholder) const { - if ((sizeof(Holder) <= POCO_SMALL_OBJECT_SIZE)) - { - new ((ValueHolder*) pPlaceholder->holder) Holder(_held); - pPlaceholder->setLocal(true); - } - else - { - pPlaceholder->pHolder = new Holder(_held); - pPlaceholder->setLocal(false); - } + cloneSOO(pPlaceholder, _held); } ValueType _held; private: + + template, Placeholder::Size::value>::value>::type* = nullptr> + static void cloneSOO(Placeholder* pPlaceholder, VT& held) + { + pPlaceholder->assignStack, ValueType>(held); + } + + template, Placeholder::Size::value>::value>::type* = nullptr> + static void cloneSOO(Placeholder* pPlaceholder, VT& held) + { + pPlaceholder->assignHeap, ValueType>(held); + } + Holder & operator = (const Holder &); }; @@ -313,19 +367,18 @@ private: return _valueHolder.content(); } - template + template, Placeholder::Size::value>::value>::type* = nullptr> void construct(const ValueType& value) { - if (sizeof(Holder) <= Placeholder::Size::value) - { - new (reinterpret_cast(_valueHolder.holder)) Holder(value); - _valueHolder.setLocal(true); - } - else - { - _valueHolder.pHolder = new Holder(value); - _valueHolder.setLocal(false); - } + _valueHolder.assignStack, ValueType>(value); + } + + template, Placeholder::Size::value>::value>::type* = nullptr> + void construct(const ValueType& value) + { + _valueHolder.assignHeap, ValueType>(value); } void construct(const Any& other) @@ -336,11 +389,6 @@ private: _valueHolder.erase(); } - void destruct() - { - content()->~ValueHolder(); - } - Placeholder _valueHolder; diff --git a/Foundation/include/Poco/Config.h b/Foundation/include/Poco/Config.h index fb3cca8f4..4027a2bee 100644 --- a/Foundation/include/Poco/Config.h +++ b/Foundation/include/Poco/Config.h @@ -74,19 +74,15 @@ // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! // !!! NOTE: Any/Dynamic::Var SOO will NOT work reliably !!! // !!! without C++11 (std::aligned_storage in particular). !!! -// !!! Only comment this out if your compiler has support !!! -// !!! for std::aligned_storage. !!! // !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! // -#ifndef POCO_ENABLE_SOO -#define POCO_NO_SOO -#endif +//#define POCO_NO_SOO // Small object size in bytes. When assigned to Any or Var, // objects larger than this value will be alocated on the heap, // while those smaller will be placement new-ed into an -// internal buffer. +// internal stack-auto-allocated buffer. #if !defined(POCO_SMALL_OBJECT_SIZE) && !defined(POCO_NO_SOO) #define POCO_SMALL_OBJECT_SIZE 32 #endif diff --git a/Foundation/include/Poco/Dynamic/Var.h b/Foundation/include/Poco/Dynamic/Var.h index 9825ba2bb..a6667a90b 100644 --- a/Foundation/include/Poco/Dynamic/Var.h +++ b/Foundation/include/Poco/Dynamic/Var.h @@ -621,10 +621,10 @@ private: void destruct() { - if (!isEmpty()) delete content(); + delete _pHolder; } - VarHolder* _pHolder; + VarHolder* _pHolder = nullptr; #else @@ -633,53 +633,41 @@ private: return _placeholder.content(); } + void destruct() + { + } + + template, Placeholder::Size::value>::value>::type* = nullptr> + void constructSOO(const ValueType& value) + { + _placeholder.assignStack, ValueType>(value); + } + + template, Placeholder::Size::value>::value>::type* = nullptr> + void constructSOO(const ValueType& value) + { + _placeholder.assignHeap, ValueType>(value); + } + template void construct(const ValueType& value) { - if (sizeof(VarHolderImpl) <= Placeholder::Size::value) - { - new (reinterpret_cast(_placeholder.holder)) VarHolderImpl(value); - _placeholder.setLocal(true); - } - else - { - _placeholder.pHolder = new VarHolderImpl(value); - _placeholder.setLocal(false); - } + constructSOO(value); } void construct(const char* value) { std::string val(value); - if (sizeof(VarHolderImpl) <= Placeholder::Size::value) - { - new (reinterpret_cast(_placeholder.holder)) VarHolderImpl(val); - _placeholder.setLocal(true); - } - else - { - _placeholder.pHolder = new VarHolderImpl(val); - _placeholder.setLocal(false); - } + constructSOO(val); } void construct(const Var& other) { + _placeholder.erase(); if (!other.isEmpty()) other.content()->clone(&_placeholder); - else - _placeholder.erase(); - } - - void destruct() - { - if (!isEmpty()) - { - if (_placeholder.isLocal()) - content()->~VarHolder(); - else - delete content(); - } } Placeholder _placeholder; @@ -709,14 +697,13 @@ inline void Var::swap(Var& other) if (!_placeholder.isLocal() && !other._placeholder.isLocal()) { - std::swap(_placeholder.pHolder, other._placeholder.pHolder); + _placeholder.swap(other._placeholder); } else { Var tmp(*this); try { - if (_placeholder.isLocal()) destruct(); construct(other); other = tmp; } diff --git a/Foundation/include/Poco/Dynamic/VarHolder.h b/Foundation/include/Poco/Dynamic/VarHolder.h index c1a17aecf..7688cd50a 100644 --- a/Foundation/include/Poco/Dynamic/VarHolder.h +++ b/Foundation/include/Poco/Dynamic/VarHolder.h @@ -295,31 +295,22 @@ protected: template VarHolder* cloneHolder(Placeholder* pVarHolder, const T& val) const - /// Instantiates value holder wrapper. If size of the wrapper is - /// larger than POCO_SMALL_OBJECT_SIZE, holder is instantiated on + /// Instantiates value holder wrapper. + /// + /// Called from clone() member function of the implementation. + /// + /// When the smal object optimization is enabled (POCO_NO_SOO not + /// defined), if size of the wrapper is larger than + /// POCO_SMALL_OBJECT_SIZE, holder is instantiated on /// the heap, otherwise it is instantiated in-place (in the /// pre-allocated buffer inside the holder). - /// - /// Called from clone() member function of the implementation when - /// small object optimization is enabled. { #ifdef POCO_NO_SOO (void)pVarHolder; return new VarHolderImpl(val); #else poco_check_ptr (pVarHolder); - if ((sizeof(VarHolderImpl) <= Placeholder::Size::value)) - { - new ((VarHolder*) pVarHolder->holder) VarHolderImpl(val); - pVarHolder->setLocal(true); - return (VarHolder*) pVarHolder->holder; - } - else - { - pVarHolder->pHolder = new VarHolderImpl(val); - pVarHolder->setLocal(false); - return pVarHolder->pHolder; - } + return makeSOOHolder(pVarHolder, val); #endif } @@ -420,6 +411,25 @@ protected: } private: + +#ifndef POCO_NO_SOO + template, Placeholder::Size::value>::value>::type* = nullptr> + VarHolder* makeSOOHolder(Placeholder* pVarHolder, const T& val) const + { + poco_check_ptr (pVarHolder); + return pVarHolder->assignStack, T>(val); + } + + template, Placeholder::Size::value>::value>::type* = nullptr> + VarHolder* makeSOOHolder(Placeholder* pVarHolder, const T& val) const + { + poco_check_ptr (pVarHolder); + return pVarHolder->assignHeap, T>(val); + } +#endif + template void checkUpperLimit(const F& from) const { diff --git a/Foundation/src/Var.cpp b/Foundation/src/Var.cpp index 4398292ac..b3f4e6d89 100644 --- a/Foundation/src/Var.cpp +++ b/Foundation/src/Var.cpp @@ -47,7 +47,9 @@ Var::Var(const char* pVal) Var::Var(const Var& other) #ifdef POCO_NO_SOO - : _pHolder(other._pHolder ? other._pHolder->clone() : 0) + :_pHolder((this != &other) ? + (other._pHolder ? other._pHolder->clone() : nullptr) : + other._pHolder) { } #else @@ -70,9 +72,10 @@ Var& Var::operator = (const Var& rhs) Var tmp(rhs); swap(tmp); #else - if ((this != &rhs) && !rhs.isEmpty()) + if (this == &rhs) return *this; + if (!rhs.isEmpty()) construct(rhs); - else if ((this != &rhs) && rhs.isEmpty()) + else _placeholder.erase(); #endif return *this; @@ -330,8 +333,6 @@ void Var::empty() delete _pHolder; _pHolder = 0; #else - if (_placeholder.isLocal()) this->~Var(); - else delete content(); _placeholder.erase(); #endif } @@ -343,8 +344,6 @@ void Var::clear() delete _pHolder; _pHolder = 0; #else - if (_placeholder.isLocal()) this->~Var(); - else delete content(); _placeholder.erase(); #endif } diff --git a/Foundation/testsuite/src/AnyTest.cpp b/Foundation/testsuite/src/AnyTest.cpp index 6d8288565..434184303 100644 --- a/Foundation/testsuite/src/AnyTest.cpp +++ b/Foundation/testsuite/src/AnyTest.cpp @@ -13,8 +13,10 @@ #include "CppUnit/TestSuite.h" #include "Poco/Exception.h" #include "Poco/Any.h" +#include "Poco/SharedPtr.h" #include "Poco/Bugcheck.h" #include +#include #if defined(_MSC_VER) && _MSC_VER < 1400 @@ -272,6 +274,85 @@ void AnyTest::testVector() } +void AnyTest::testPlaceholder() +{ +#ifndef POCO_NO_SOO + Placeholder ph; + assertTrue(ph.isEmpty()); + assertFalse(ph.isLocal()); + + char c = *ph.assignStack(1); + assertTrue(1 == c); + assertFalse(ph.isEmpty()); + assertTrue(ph.isLocal()); + + c = *ph.assignHeap(2); + assertTrue(2 == c); + assertFalse(ph.isEmpty()); + assertFalse(ph.isLocal()); + + ph.erase(); + assertTrue(ph.isEmpty()); + assertFalse(ph.isLocal()); + + if (sizeof(std::shared_ptr) <= POCO_SMALL_OBJECT_SIZE) + { + Placeholder> sph; + assertTrue(sph.isEmpty()); + assertFalse(sph.isLocal()); + + int i = **sph.assignStack, int*>(new int(1)); + assertTrue(1 == i); + assertFalse(sph.isEmpty()); + assertTrue(sph.isLocal()); + + i = **sph.assignHeap, int*>(new int(1)); + assertTrue(1 == i); + assertFalse(sph.isEmpty()); + assertFalse(sph.isLocal()); + } + + if (sizeof(Poco::SharedPtr) <= POCO_SMALL_OBJECT_SIZE) + { + Placeholder> psph; + assertTrue(psph.isEmpty()); + assertFalse(psph.isLocal()); + + int i = **psph.assignStack, int*>(new int(1)); + assertTrue(1 == i); + assertFalse(psph.isEmpty()); + assertTrue(psph.isLocal()); + + i = **psph.assignHeap, int*>(new int(1)); + assertTrue(1 == i); + assertFalse(psph.isEmpty()); + assertFalse(psph.isLocal()); + } + + if (sizeof(std::vector) <= POCO_SMALL_OBJECT_SIZE) + { + Placeholder> vph; + assertTrue(vph.isEmpty()); + assertFalse(vph.isLocal()); + + std::vector inv{1,2,3}; + std::vector outv = *vph.assignStack, std::vector>(inv); + assertTrue(inv == outv); + assertFalse(vph.isEmpty()); + assertTrue(vph.isLocal()); + + outv.clear(); + outv = *vph.assignHeap, std::vector>(inv); + assertTrue(inv == outv); + assertFalse(vph.isEmpty()); + assertFalse(vph.isLocal()); + } + + // ... +#endif +} + + void AnyTest::setUp() { } @@ -298,6 +379,7 @@ CppUnit::Test* AnyTest::suite() CppUnit_addTest(pSuite, AnyTest, testInt); CppUnit_addTest(pSuite, AnyTest, testComplexType); CppUnit_addTest(pSuite, AnyTest, testVector); + CppUnit_addTest(pSuite, AnyTest, testPlaceholder); return pSuite; } diff --git a/Foundation/testsuite/src/AnyTest.h b/Foundation/testsuite/src/AnyTest.h index f5d1a9635..97eb0c414 100644 --- a/Foundation/testsuite/src/AnyTest.h +++ b/Foundation/testsuite/src/AnyTest.h @@ -36,7 +36,9 @@ public: void testInt(); void testComplexType(); void testVector(); - + + void testPlaceholder(); + void setUp(); void tearDown(); static CppUnit::Test* suite(); diff --git a/Foundation/testsuite/src/VarTest.cpp b/Foundation/testsuite/src/VarTest.cpp index 29891d3e1..b3ef96df5 100644 --- a/Foundation/testsuite/src/VarTest.cpp +++ b/Foundation/testsuite/src/VarTest.cpp @@ -29,6 +29,9 @@ using namespace Poco; using namespace Poco::Dynamic; +namespace { + + class Dummy { public: @@ -54,6 +57,8 @@ private: int _val; }; +} + VarTest::VarTest(const std::string& rName): CppUnit::TestCase(rName) { @@ -3068,6 +3073,20 @@ void VarTest::testIterator() } +void VarTest::testSharedPtr() +{ + Poco::SharedPtr p = new int(42); + { + Var v; + v = p; + Var v1; + v = v1; + v1 = v; + } + assertTrue(p.referenceCount() == 1); +} + + void VarTest::setUp() { } @@ -3116,6 +3135,7 @@ CppUnit::Test* VarTest::suite() CppUnit_addTest(pSuite, VarTest, testOrderedDynamicStructString); CppUnit_addTest(pSuite, VarTest, testDynamicStructInt); CppUnit_addTest(pSuite, VarTest, testOrderedDynamicStructInt); + CppUnit_addTest(pSuite, VarTest, testSharedPtr); CppUnit_addTest(pSuite, VarTest, testArrayToString); CppUnit_addTest(pSuite, VarTest, testArrayToStringEscape); CppUnit_addTest(pSuite, VarTest, testStructToString); diff --git a/Foundation/testsuite/src/VarTest.h b/Foundation/testsuite/src/VarTest.h index d5a1a97ae..fc8a1d8fd 100644 --- a/Foundation/testsuite/src/VarTest.h +++ b/Foundation/testsuite/src/VarTest.h @@ -77,6 +77,7 @@ public: void testUUID(); void testEmpty(); void testIterator(); + void testSharedPtr(); void setUp(); void tearDown();