diff --git a/src/factory/Compare.cpp b/src/factory/Compare.cpp index 6f2fd3c..237b2ca 100644 --- a/src/factory/Compare.cpp +++ b/src/factory/Compare.cpp @@ -23,7 +23,7 @@ namespace epics { namespace pvData { * 1) same instance * 2) same type (field and scalar/element), same name, same subfields (if any) */ -bool operator==(const Field& a, const Field& b) +bool compare(const Field& a, const Field& b) { if(&a==&b) return true; @@ -33,53 +33,53 @@ bool operator==(const Field& a, const Field& b) case scalar: { const Scalar &A=static_cast(a); const Scalar &B=static_cast(b); - return A==B; + return compare(A, B); } case scalarArray: { const ScalarArray &A=static_cast(a); const ScalarArray &B=static_cast(b); - return A==B; + return compare(A, B); } case structure: { const Structure &A=static_cast(a); const Structure &B=static_cast(b); - return A==B; + return compare(A, B); } case structureArray: { const StructureArray &A=static_cast(a); const StructureArray &B=static_cast(b); - return A==B; + return compare(A, B); } case union_: { const Union &A=static_cast(a); const Union &B=static_cast(b); - return A==B; + return compare(A, B); } case unionArray: { const UnionArray &A=static_cast(a); const UnionArray &B=static_cast(b); - return A==B; + return compare(A, B); } default: throw std::logic_error("Invalid Field type in comparison"); } } -bool operator==(const Scalar& a, const Scalar& b) +bool compare(const Scalar& a, const Scalar& b) { if(&a==&b) return true; return a.getScalarType()==b.getScalarType(); } -bool operator==(const ScalarArray& a, const ScalarArray& b) +bool compare(const ScalarArray& a, const ScalarArray& b) { if(&a==&b) return true; return a.getElementType()==b.getElementType(); } -bool operator==(const Structure& a, const Structure& b) +bool compare(const Structure& a, const Structure& b) { if(&a==&b) return true; @@ -101,12 +101,12 @@ bool operator==(const Structure& a, const Structure& b) return std::equal( an.begin(), an.end(), bn.begin() ); } -bool operator==(const StructureArray& a, const StructureArray& b) +bool compare(const StructureArray& a, const StructureArray& b) { return *(a.getStructure().get())==*(b.getStructure().get()); } -bool operator==(const Union& a, const Union& b) +bool compare(const Union& a, const Union& b) { if(&a==&b) return true; @@ -128,12 +128,12 @@ bool operator==(const Union& a, const Union& b) return std::equal( an.begin(), an.end(), bn.begin() ); } -bool operator==(const UnionArray& a, const UnionArray& b) +bool compare(const UnionArray& a, const UnionArray& b) { return *(a.getUnion().get())==*(b.getUnion().get()); } -bool operator==(const BoundedString& a, const BoundedString& b) +bool compare(const BoundedString& a, const BoundedString& b) { if(&a==&b) return true; diff --git a/src/factory/FieldCreateFactory.cpp b/src/factory/FieldCreateFactory.cpp index 864dd22..fc4e444 100644 --- a/src/factory/FieldCreateFactory.cpp +++ b/src/factory/FieldCreateFactory.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include @@ -35,6 +36,46 @@ static DebugLevel debugLevel = lowDebug; size_t Field::num_instances; +struct Field::Helper { + static void hash(Field *fld) { + std::ostringstream key; + // hash the output of operator<<() + // not efficient, but stable within this process. + key<<(*fld); + fld->m_hash = epicsStrHash(key.str().c_str(), 0xbadc0de1); + } +}; + +struct FieldCreate::Helper { + template + static void cache(const FieldCreate *create, std::tr1::shared_ptr& ent) { + Field::Helper::hash(ent.get()); + + Lock G(create->mutex); + // we examine raw pointers stored in create->cache, which is safe under create->mutex + + std::pair itp(create->cache.equal_range(ent->m_hash)); + for(; itp.first!=itp.second; ++itp.first) { + Field* cent(itp.first->second); + FLD* centx(dynamic_cast(cent)); + if(centx && compare(*centx, *ent)) { + try{ + ent = std::tr1::static_pointer_cast(cent->shared_from_this()); + return; + }catch(std::tr1::bad_weak_ptr&){ + // we're racing destruction. + // add a new entry. + // Field::~Field is in the process of removing this old one. + continue; + } + } + } + + create->cache.insert(std::make_pair(ent->m_hash, ent.get())); + // cache cleaned from Field::~Field + } +}; + Field::Field(Type type) : m_fieldType(type) { @@ -43,6 +84,18 @@ Field::Field(Type type) Field::~Field() { REFTRACE_DECREMENT(num_instances); + FieldCreatePtr create(getFieldCreate()); + + Lock G(create->mutex); + + std::pair itp(create->cache.equal_range(m_hash)); + for(; itp.first!=itp.second; ++itp.first) { + Field* cent(itp.first->second); + if(cent==this) { + create->cache.erase(itp.first); + return; + } + } } @@ -1075,10 +1128,9 @@ ScalarConstPtr FieldCreate::createScalar(ScalarType scalarType) const BoundedStringConstPtr FieldCreate::createBoundedString(std::size_t maxLength) const { - // TODO use std::make_shared - std::tr1::shared_ptr s(new BoundedString(maxLength), Field::Deleter()); - BoundedStringConstPtr sa = s; - return sa; + std::tr1::shared_ptr s(new BoundedString(maxLength)); + Helper::cache(this, s); + return s; } ScalarArrayConstPtr FieldCreate::createScalarArray(ScalarType elementType) const @@ -1100,10 +1152,9 @@ ScalarArrayConstPtr FieldCreate::createFixedScalarArray(ScalarType elementType, THROW_EXCEPTION2(std::invalid_argument, strm.str()); } - // TODO use std::make_shared - std::tr1::shared_ptr s(new FixedScalarArray(elementType, size), Field::Deleter()); - ScalarArrayConstPtr sa = s; - return sa; + std::tr1::shared_ptr s(new FixedScalarArray(elementType, size)); + Helper::cache(this, s); + return s; } ScalarArrayConstPtr FieldCreate::createBoundedScalarArray(ScalarType elementType, size_t size) const @@ -1114,10 +1165,9 @@ ScalarArrayConstPtr FieldCreate::createBoundedScalarArray(ScalarType elementType THROW_EXCEPTION2(std::invalid_argument, strm.str()); } - // TODO use std::make_shared - std::tr1::shared_ptr s(new BoundedScalarArray(elementType, size), Field::Deleter()); - ScalarArrayConstPtr sa = s; - return sa; + std::tr1::shared_ptr s(new BoundedScalarArray(elementType, size)); + Helper::cache(this, s); + return s; } StructureConstPtr FieldCreate::createStructure () const @@ -1174,10 +1224,9 @@ StructureConstPtr FieldCreate::createStructure ( StringArray const & fieldNames,FieldConstPtrArray const & fields) const { validateFieldNames(fieldNames); - // TODO use std::make_shared - std::tr1::shared_ptr sp(new Structure(fieldNames,fields), Field::Deleter()); - StructureConstPtr structure = sp; - return structure; + std::tr1::shared_ptr sp(new Structure(fieldNames,fields)); + Helper::cache(this, sp); + return sp; } StructureConstPtr FieldCreate::createStructure ( @@ -1186,29 +1235,26 @@ StructureConstPtr FieldCreate::createStructure ( FieldConstPtrArray const & fields) const { validateFieldNames(fieldNames); - // TODO use std::make_shared - std::tr1::shared_ptr sp(new Structure(fieldNames,fields,id), Field::Deleter()); - StructureConstPtr structure = sp; - return structure; + std::tr1::shared_ptr sp(new Structure(fieldNames,fields,id)); + Helper::cache(this, sp); + return sp; } StructureArrayConstPtr FieldCreate::createStructureArray( StructureConstPtr const & structure) const { - // TODO use std::make_shared - std::tr1::shared_ptr sp(new StructureArray(structure), Field::Deleter()); - StructureArrayConstPtr structureArray = sp; - return structureArray; + std::tr1::shared_ptr sp(new StructureArray(structure)); + Helper::cache(this, sp); + return sp; } UnionConstPtr FieldCreate::createUnion ( StringArray const & fieldNames,FieldConstPtrArray const & fields) const { validateFieldNames(fieldNames); - // TODO use std::make_shared - std::tr1::shared_ptr sp(new Union(fieldNames,fields), Field::Deleter()); - UnionConstPtr punion = sp; - return punion; + std::tr1::shared_ptr sp(new Union(fieldNames,fields)); + Helper::cache(this, sp); + return sp; } UnionConstPtr FieldCreate::createUnion ( @@ -1217,10 +1263,9 @@ UnionConstPtr FieldCreate::createUnion ( FieldConstPtrArray const & fields) const { validateFieldNames(fieldNames); - // TODO use std::make_shared - std::tr1::shared_ptr sp(new Union(fieldNames,fields,id), Field::Deleter()); - UnionConstPtr punion = sp; - return punion; + std::tr1::shared_ptr sp(new Union(fieldNames,fields,id)); + Helper::cache(this, sp); + return sp; } UnionConstPtr FieldCreate::createVariantUnion () const @@ -1231,10 +1276,9 @@ UnionConstPtr FieldCreate::createVariantUnion () const UnionArrayConstPtr FieldCreate::createUnionArray( UnionConstPtr const & punion) const { - // TODO use std::make_shared - std::tr1::shared_ptr sp(new UnionArray(punion), Field::Deleter()); - UnionArrayConstPtr unionArray = sp; - return unionArray; + std::tr1::shared_ptr sp(new UnionArray(punion)); + Helper::cache(this, sp); + return sp; } UnionArrayConstPtr FieldCreate::createVariantUnionArray () const @@ -1363,12 +1407,10 @@ FieldConstPtr FieldCreate::deserialize(ByteBuffer* buffer, DeserializableControl size_t size = SerializeHelper::readSize(buffer, control); - // TODO use std::make_shared std::tr1::shared_ptr sp( - new BoundedString(size), - Field::Deleter()); - FieldConstPtr p = sp; - return p; + new BoundedString(size)); + Helper::cache(this, sp); + return sp; } else throw std::invalid_argument("invalid type encoding"); @@ -1393,19 +1435,17 @@ FieldConstPtr FieldCreate::deserialize(ByteBuffer* buffer, DeserializableControl { // TODO use std::make_shared std::tr1::shared_ptr sp( - new FixedScalarArray(static_cast(scalarType), size), - Field::Deleter()); - FieldConstPtr p = sp; - return p; + new FixedScalarArray(static_cast(scalarType), size)); + Helper::cache(this, sp); + return sp; } else { // TODO use std::make_shared std::tr1::shared_ptr sp( - new BoundedScalarArray(static_cast(scalarType), size), - Field::Deleter()); - FieldConstPtr p = sp; - return p; + new BoundedScalarArray(static_cast(scalarType), size)); + Helper::cache(this, sp); + return sp; } } else if (typeCode == 0x80) @@ -1417,9 +1457,9 @@ FieldConstPtr FieldCreate::deserialize(ByteBuffer* buffer, DeserializableControl // Type type = Type.structureArray; StructureConstPtr elementStructure = std::tr1::static_pointer_cast(control->cachedDeserialize(buffer)); // TODO use std::make_shared - std::tr1::shared_ptr sp(new StructureArray(elementStructure), Field::Deleter()); - FieldConstPtr p = sp; - return p; + std::tr1::shared_ptr sp(new StructureArray(elementStructure)); + Helper::cache(this, sp); + return sp; } else if (typeCode == 0x81) { @@ -1430,9 +1470,9 @@ FieldConstPtr FieldCreate::deserialize(ByteBuffer* buffer, DeserializableControl // Type type = Type.unionArray; UnionConstPtr elementUnion = std::tr1::static_pointer_cast(control->cachedDeserialize(buffer)); // TODO use std::make_shared - std::tr1::shared_ptr sp(new UnionArray(elementUnion), Field::Deleter()); - FieldConstPtr p = sp; - return p; + std::tr1::shared_ptr sp(new UnionArray(elementUnion)); + Helper::cache(this, sp); + return sp; } else if (typeCode == 0x82) { @@ -1481,23 +1521,21 @@ FieldCreate::FieldCreate() { for (int i = 0; i <= MAX_SCALAR_TYPE; i++) { - // TODO use std::make_shared - std::tr1::shared_ptr sp(new Scalar(static_cast(i)), Field::Deleter()); - ScalarConstPtr p = sp; - scalars.push_back(p); + std::tr1::shared_ptr sp(new Scalar(static_cast(i))); + Helper::cache(this, sp); + scalars.push_back(sp); - // TODO use std::make_shared - std::tr1::shared_ptr spa(new ScalarArray(static_cast(i)), Field::Deleter()); - ScalarArrayConstPtr pa = spa; + std::tr1::shared_ptr spa(new ScalarArray(static_cast(i))); + Helper::cache(this, spa); scalarArrays.push_back(spa); } - // TODO use std::make_shared - std::tr1::shared_ptr su(new Union(), Field::Deleter()); + std::tr1::shared_ptr su(new Union()); + Helper::cache(this, su); variantUnion = su; - // TODO use std::make_shared - std::tr1::shared_ptr sua(new UnionArray(variantUnion), Field::Deleter()); + std::tr1::shared_ptr sua(new UnionArray(variantUnion)); + Helper::cache(this, sua); variantUnionArray = sua; } diff --git a/src/pv/pvIntrospect.h b/src/pv/pvIntrospect.h index 358faee..9ebed70 100644 --- a/src/pv/pvIntrospect.h +++ b/src/pv/pvIntrospect.h @@ -12,7 +12,9 @@ #include #include #include +#include +#include #include #include #include @@ -347,6 +349,8 @@ protected: Field(Type type); private: const Type m_fieldType; + unsigned int m_hash; + struct Helper; friend class StructureArray; friend class Structure; @@ -354,7 +358,6 @@ private: friend class StandardField; friend class BasePVStructureArray; friend class FieldCreate; - struct Deleter{void operator()(Field *p){delete p;}}; EPICS_NOT_COPYABLE(Field) }; @@ -489,7 +492,6 @@ public: virtual void serialize(ByteBuffer *buffer, SerializableControl *control) const OVERRIDE; virtual void deserialize(ByteBuffer *buffer, DeserializableControl *control) OVERRIDE FINAL; -protected: virtual ~ScalarArray(); private: const std::string getIDScalarArrayLUT() const; @@ -525,7 +527,6 @@ public: virtual void serialize(ByteBuffer *buffer, SerializableControl *control) const OVERRIDE FINAL; -protected: virtual ~BoundedScalarArray(); private: std::size_t size; @@ -558,7 +559,6 @@ public: virtual void serialize(ByteBuffer *buffer, SerializableControl *control) const OVERRIDE FINAL; -protected: virtual ~FixedScalarArray(); private: std::size_t size; @@ -599,6 +599,7 @@ protected: * @param structure The introspection interface for the elements. */ StructureArray(StructureConstPtr const & structure); +public: virtual ~StructureArray(); private: StructureConstPtr pstructure; @@ -639,6 +640,7 @@ protected: * @param _punion The introspection interface for the elements. */ UnionArray(UnionConstPtr const & _punion); +public: virtual ~UnionArray(); private: UnionConstPtr punion; @@ -1221,11 +1223,19 @@ public: private: FieldCreate(); - + + // const after ctor std::vector scalars; std::vector scalarArrays; UnionConstPtr variantUnion; UnionArrayConstPtr variantUnionArray; + + mutable Mutex mutex; + typedef std::multimap cache_t; + mutable cache_t cache; + + struct Helper; + friend class Field; EPICS_NOT_COPYABLE(FieldCreate) }; @@ -1278,32 +1288,35 @@ OP(pvDouble, double) OP(pvString, std::string) #undef OP -bool epicsShareExtern operator==(const Field&, const Field&); -bool epicsShareExtern operator==(const Scalar&, const Scalar&); -bool epicsShareExtern operator==(const ScalarArray&, const ScalarArray&); -bool epicsShareExtern operator==(const Structure&, const Structure&); -bool epicsShareExtern operator==(const StructureArray&, const StructureArray&); -bool epicsShareExtern operator==(const Union&, const Union&); -bool epicsShareExtern operator==(const UnionArray&, const UnionArray&); -bool epicsShareExtern operator==(const BoundedString&, const BoundedString&); +bool epicsShareExtern compare(const Field&, const Field&); +bool epicsShareExtern compare(const Scalar&, const Scalar&); +bool epicsShareExtern compare(const ScalarArray&, const ScalarArray&); +bool epicsShareExtern compare(const Structure&, const Structure&); +bool epicsShareExtern compare(const StructureArray&, const StructureArray&); +bool epicsShareExtern compare(const Union&, const Union&); +bool epicsShareExtern compare(const UnionArray&, const UnionArray&); +bool epicsShareExtern compare(const BoundedString&, const BoundedString&); -static inline bool operator!=(const Field& a, const Field& b) -{return !(a==b);} -static inline bool operator!=(const Scalar& a, const Scalar& b) -{return !(a==b);} -static inline bool operator!=(const ScalarArray& a, const ScalarArray& b) -{return !(a==b);} -static inline bool operator!=(const Structure& a, const Structure& b) -{return !(a==b);} -static inline bool operator!=(const StructureArray& a, const StructureArray& b) -{return !(a==b);} -static inline bool operator!=(const Union& a, const Union& b) -{return !(a==b);} -static inline bool operator!=(const UnionArray& a, const UnionArray& b) -{return !(a==b);} -static inline bool operator!=(const BoundedString& a, const BoundedString& b) -{return !(a==b);} +/** Equality with other Field + * + * The creation process of class FieldCreate ensures that identical field definitions + * will share the same instance. So pointer equality is sufficient to show defintion + * equality. If in doubt, compare() will do an full test. + */ +#define MAKE_COMPARE(CLASS) \ +static FORCE_INLINE bool operator==(const CLASS& a, const CLASS& b) {return (void*)&a==(void*)&b;} \ +static FORCE_INLINE bool operator!=(const CLASS& a, const CLASS& b) {return !(a==b);} +MAKE_COMPARE(Field) +MAKE_COMPARE(Scalar) +MAKE_COMPARE(ScalarArray) +MAKE_COMPARE(Structure) +MAKE_COMPARE(StructureArray) +MAKE_COMPARE(Union) +MAKE_COMPARE(UnionArray) +MAKE_COMPARE(BoundedString) + +#undef MAKE_COMPARE }} /** diff --git a/testApp/misc/testSerialization.cpp b/testApp/misc/testSerialization.cpp index 04b8784..33a8bd2 100644 --- a/testApp/misc/testSerialization.cpp +++ b/testApp/misc/testSerialization.cpp @@ -24,6 +24,8 @@ #include #include #include +#include +#include #include @@ -629,6 +631,8 @@ void testStructureId() { void serializationFieldTest(FieldConstPtr const & field) { + testShow()<clear(); // serialize @@ -639,7 +643,7 @@ void serializationFieldTest(FieldConstPtr const & field) FieldConstPtr deserializedField = getFieldCreate()->deserialize(buffer, control); - // must equal + testShow()<<" after "<<(void*)field.get()<<" == "<<(void*)deserializedField.get(); testOk1(*field == *deserializedField); } diff --git a/testApp/pv/testFieldBuilder.cpp b/testApp/pv/testFieldBuilder.cpp index 49895e7..c619bde 100644 --- a/testApp/pv/testFieldBuilder.cpp +++ b/testApp/pv/testFieldBuilder.cpp @@ -324,8 +324,8 @@ void test_extendStructure() <<"amended: "<(amended.get()), - static_cast(expected.get())); + testEqual(static_cast(amended.get()), + static_cast(expected.get())); testEqual(*amended, *expected); testThrows(std::runtime_error,