Browse Source

HPCC-13564 Reduce packing overrides and fix misaligned atomic access

Also reduce the memory required by some PTree related structures

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>
Gavin Halliday 10 years ago
parent
commit
90c6d015f1
3 changed files with 22 additions and 51 deletions
  1. 0 9
      dali/base/dasds.cpp
  2. 3 15
      dali/base/dasds.ipp
  3. 19 27
      system/jlib/jptree.ipp

+ 0 - 9
dali/base/dasds.cpp

@@ -2353,10 +2353,6 @@ CRemoteTreeBase *CRemoteTreeBase::createChild(int pos, const char *childName)
 static CheckedCriticalSection suppressedOrphanUnlockCrit; // to temporarily suppress unlockall
 static bool suppressedOrphanUnlock=false;
 
-#ifdef __64BIT__
-#pragma pack(push,1)    // 64bit pack CServerRemoteTree's    (could do for 32bit also)
-#endif
-
 #if defined(new)
 #define __old_new new
 #undef new
@@ -2819,11 +2815,6 @@ public:
 #endif
 
 
-#ifdef __64BIT__
-#pragma pack(pop)   // 64bit pack CServerRemoteTree's    (could do for 32bit also)
-#endif
-
-
 void populateWithServerIds(IPropertyTree *matchParent, CRemoteTreeBase *parent)
 {
     matchParent->setPropInt64("@serverId", parent->queryServerId());

+ 3 - 15
dali/base/dasds.ipp

@@ -105,12 +105,10 @@ interface ITrackChanges
     virtual void registerPropAppend(size32_t l) = 0;
 };
 
-class ChangeInfo : public CInterface, implements IInterface
+class ChangeInfo : public CInterfaceOf<IInterface>
 {
     DECL_NAMEDCOUNT;
 public:
-    IMPLEMENT_IINTERFACE;
-
     ChangeInfo(IPropertyTree &_owner) : owner(&_owner) { INIT_NAMEDCOUNT; tree.setown(createPTree(RESERVED_CHANGE_NODE)); }
     const IPropertyTree *queryOwner() const { return owner; }
     const void *queryFindParam() const { return &owner; }
@@ -193,7 +191,7 @@ private: // data
 
 ///////////////////
 
-class CPTStack : public CInterface, public IArrayOf<PTree>
+class CPTStack : public IArrayOf<PTree>
 {
     bool _fill(IPropertyTree &root, const char *xpath, IPropertyTree &tail);
 public:
@@ -227,10 +225,6 @@ class CBranchChange;
 ///////////////////
 class CSubscriberContainerList;
 
-#ifdef __64BIT__
-#pragma pack(push,1)    // 64bit pack CRemoteTree's  (could probably do for 32bit also)
-#endif
-
 class CRemoteTreeBase : public PTree
 {
 public:
@@ -267,10 +261,6 @@ protected: // data
     __int64 serverId;
 };
 
-#ifdef __64BIT__
-#pragma pack(pop)   
-#endif
-
 class CTrackChanges
 {
 public:
@@ -581,7 +571,7 @@ public:
     virtual unsigned queryConnections() { return connections.ordinality(); }
 };
 
-class CXPathIterator : public CInterface, implements IPropertyTreeIterator
+class CXPathIterator : public CInterfaceOf<IPropertyTreeIterator>
 {
     DECL_NAMEDCOUNT;
     IPropertyTree *root;
@@ -593,8 +583,6 @@ class CXPathIterator : public CInterface, implements IPropertyTreeIterator
     IPTIteratorCodes flags;
     bool validateServerIds;
 public:
-    IMPLEMENT_IINTERFACE;
-
     CXPathIterator(IPropertyTree *_root, IPropertyTree *_matchTree, IPTIteratorCodes _flags) : root(_root), matchTree(_matchTree), flags(_flags)
     {
         INIT_NAMEDCOUNT;

+ 19 - 27
system/jlib/jptree.ipp

@@ -29,12 +29,6 @@
 #include "jptree.hpp"
 #include "jbuff.hpp"
 
-#ifdef __64BIT__
-#pragma pack(push,1)    // 64bit pack PTree's    (could possibly do for 32bit also but may be compatibility problems)
-#endif
-
-
-
 #define ANE_APPEND -1
 #define ANE_SET -2
 ///////////////////
@@ -254,6 +248,13 @@ struct AttrValue
 #define AM_NOCASE_FLAG (0x8000)
 #define AM_NOCASE_MASK (0x7fff)
 
+#ifdef __64BIT__
+#pragma pack(push,1)
+// Byte-Pack AttrMap and PTree because very large numbers are created.  However, this may cause problems on systems that
+// require aligned pointer access.  Without overriding the packing the structure currently wastes 8 bytes.
+// Ideally the classes would be restructured to avoid this, but it would probably require AttrMap to move into PTree
+#endif
+
 class jlib_decl AttrMap
 {
     AttrValue *attrs;
@@ -415,15 +416,20 @@ private:
     void replaceSelf(IPropertyTree *val);
 
 protected: // data
+    byte flags;
     IPropertyTree *parent; // ! currently only used if tree embedded into array, used to locate position.
 
     HashKeyElement *name;
     ChildMap *children;
     IPTArrayValue *value;
     AttrMap attributes;
-    byte flags;
 };
 
+#ifdef __64BIT__
+#pragma pack(pop)
+#endif
+
+
 jlib_decl IPropertyTree *createPropBranch(IPropertyTree *tree, const char *xpath, bool createIntermediates=false, IPropertyTree **created=NULL, IPropertyTree **createdParent=NULL);
 
 class LocalPTree : public PTree
@@ -446,11 +452,9 @@ public:
 };
 
 class PTree;
-class SingleIdIterator : public CInterface, implements IPropertyTreeIterator
+class SingleIdIterator : public CInterfaceOf<IPropertyTreeIterator>
 {
 public:
-    IMPLEMENT_IINTERFACE;
-
     SingleIdIterator(const PTree &_tree, unsigned pos=1, unsigned _many=(unsigned)-1);
     ~SingleIdIterator();
     void setCurrent(unsigned pos);
@@ -468,11 +472,9 @@ private:
 };
 
 
-class PTLocalIteratorBase : public CInterface, implements IPropertyTreeIterator
+class PTLocalIteratorBase : public CInterfaceOf<IPropertyTreeIterator>
 {
 public:
-    IMPLEMENT_IINTERFACE;
-
     PTLocalIteratorBase(const PTree *tree, const char *_id, bool _nocase, bool sort);
 
     ~PTLocalIteratorBase();
@@ -485,9 +487,9 @@ public:
     virtual IPropertyTree & query() { return iter->query(); }
 
 protected:
+    bool nocase, sort;  // pack with the link count
     IPropertyTreeIterator *baseIter;
     StringAttr id;
-    bool nocase, sort;
 private:
     const PTree *tree;
     IPropertyTreeIterator *iter;
@@ -498,8 +500,6 @@ private:
 class PTIdMatchIterator : public PTLocalIteratorBase
 {
 public:
-    IMPLEMENT_IINTERFACE;
-
     PTIdMatchIterator(const PTree *tree, const char *id, bool nocase, bool sort) : PTLocalIteratorBase(tree, id, nocase, sort) { }
 
     virtual bool match();
@@ -507,11 +507,9 @@ public:
 
 class StackElement;
 
-class PTStackIterator : public CInterface, implements IPropertyTreeIterator
+class PTStackIterator : public CInterfaceOf<IPropertyTreeIterator>
 {
 public:
-    IMPLEMENT_IINTERFACE;
-
     PTStackIterator(IPropertyTreeIterator *_iter, const char *_xpath);
     ~PTStackIterator();
 
@@ -536,11 +534,11 @@ private: // data
     StackElement *stack;
 };
 
-class CPTreeMaker : public CInterface, implements IPTreeMaker
+class CPTreeMaker : public CInterfaceOf<IPTreeMaker>
 {
+    bool rootProvided, noRoot;
     IPropertyTree *root;
     ICopyArrayOf<IPropertyTree> ptreeStack;
-    bool rootProvided, noRoot;
     IPTreeNodeCreator *nodeCreator;
     class CDefaultNodeCreator : public CInterface, implements IPTreeNodeCreator
     {
@@ -555,8 +553,6 @@ class CPTreeMaker : public CInterface, implements IPTreeMaker
 protected:
     IPropertyTree *currentNode;
 public:
-    IMPLEMENT_IINTERFACE;
-
     CPTreeMaker(byte flags=ipt_none, IPTreeNodeCreator *_nodeCreator=NULL, IPropertyTree *_root=NULL, bool _noRoot=false) : noRoot(_noRoot)
     {
         if (_nodeCreator)
@@ -643,9 +639,5 @@ public:
     }
 };
 
-#ifdef __64BIT__
-#pragma pack(pop)   
-#endif
-
 
 #endif