HPCC-18389 Implement begin and end on hash tables

Signed-off-by: Gavin Halliday <gavin.halliday@lexisnexis.com>

ecl/hql/hqlexpr.cpp

@@ -3868,10 +3868,8 @@ void displayHqlCacheStats()
     static HqlExprCopyArray prev;
     DBGLOG("CachedItems = %d", exprCache->count());
     exprCache->dumpStats();
-    JavaHashIteratorOf<IHqlExpression> iter(*exprCache, false);
-    ForEach(iter)
+    for (CHqlExpression & ret : *exprCache)
     {
-        IHqlExpression & ret = iter.query();
         if (!prev.contains(ret))
         {
             StringBuffer s;
@@ -3881,9 +3879,9 @@ void displayHqlCacheStats()
     }
 
     prev.kill();
-    ForEach(iter)
+    for (auto & iter2 : *exprCache)
     {
-        prev.append(iter.query());
+        prev.append(iter2);
     }
 #endif
 }
@@ -5330,10 +5328,8 @@ void CUsedTablesBuilder::addActiveTable(IHqlExpression * expr)
 void CUsedTablesBuilder::cleanupProduction()
 {
     HqlExprCopyArray toRemove;
-    SuperHashIteratorOf<IHqlExpression> iter(inScopeTables);
-    ForEach(iter)
+    for (IHqlExpression& cur : inScopeTables)
     {
-        IHqlExpression & cur = iter.query();
         switch (cur.getOperator())
         {
         case no_matchattr:
@@ -5393,10 +5389,8 @@ void CUsedTablesBuilder::removeRows(IHqlExpression * expr, IHqlExpression * left
 void CUsedTablesBuilder::removeActiveRecords()
 {
     HqlExprCopyArray toRemove;
-    SuperHashIteratorOf<IHqlExpression> iter(inScopeTables);
-    ForEach(iter)
+    for (IHqlExpression&cur : inScopeTables)
     {
-        IHqlExpression & cur = iter.query();
         if (cur.isRecord())
             toRemove.append(cur);
     }
@@ -5407,12 +5401,8 @@ void CUsedTablesBuilder::removeActiveRecords()
 
 inline void expand(HqlExprCopyArray & target, const UsedExpressionHashTable & source)
 {
-    SuperHashIteratorOf<IHqlExpression> iter(source);
-    ForEach(iter)
-    {
-        IHqlExpression & cur = iter.query();
+    for (auto& cur : source)
         target.append(cur);
-    }
 }
 
 void CUsedTablesBuilder::set(CUsedTables & tables)
@@ -8388,13 +8378,13 @@ bool CHqlScope::getProp(IAtom * a, StringBuffer &ret) const
 void CHqlScope::getSymbols(HqlExprArray& exprs) const
 {
     SymbolTable & localSymbols = const_cast<SymbolTable &>(symbols);
-    SymbolTableIterator iter(localSymbols);
-    for (iter.first(); iter.isValid(); iter.next()) 
+    SymbolTableLock lock(localSymbols);
+    for (auto & iter : localSymbols)
     {
-        IHqlExpression *cur = localSymbols.mapToValue(&iter.query());
+        IHqlExpression *cur = localSymbols.mapToValue(&iter);
         if (!cur)
         {
-            IAtom * name = * static_cast<IAtom * const *>(iter.query().getKey());
+            IAtom * name = * static_cast<IAtom * const *>(iter.getKey());
             throw MakeStringException(ERR_INTERNAL_NOEXPR, "INTERNAL: getSymbol %s has no associated expression", name ? str(name) : "");
         }
 

ecl/hql/hqlexpr.ipp

@@ -936,6 +936,7 @@ class CHqlExternalDatasetCall: public CHqlExternalCall, implements IHqlDataset
 #ifdef THREAD_SAFE_SYMBOLS
 class SymbolTable
 {
+    typedef MapXToMyClass<IAtom *, IAtom *, IHqlExpression> MAP;
 public:
     inline void setValue(IAtom * name, IHqlExpression * value)
     {
@@ -984,11 +985,29 @@ public:
         return map.count();
     }
 
+    MAP::ConstHashItem begin() const { return map.begin(); }
+    MAP::ConstHashItem end() const { return map.end(); }
+
 protected:
-    MapXToMyClass<IAtom *, IAtom *, IHqlExpression> map;
+    MAP map;
     mutable CriticalSection cs;
 };
 
+class SymbolTableLock
+{
+public:
+    SymbolTableLock(SymbolTable & _table) : table(_table)
+    {
+        table.lock();
+    }
+    ~SymbolTableLock()
+    {
+        table.unlock();
+    }
+protected:
+    SymbolTable & table;
+};
+
 class SymbolTableIterator : public HashIterator
 {
 public:
@@ -1015,6 +1034,14 @@ public:
     }
 };
 typedef HashIterator SymbolTableIterator;
+class SymbolTableLock
+{
+public:
+    SymbolTableLock(SymbolTable &) {}
+    ~SymbolTableLock() {}
+};
+
+
 #endif
 
 inline IHqlExpression * lookupSymbol(SymbolTable & symbols, IIdAtom * searchName, bool sharedOK)

ecl/hqlcpp/hqlhtcpp.cpp

@@ -18813,13 +18813,8 @@ bool HqlCppTranslator::recordContainsIfBlock(IHqlExpression * record)
 
 void HqlCppTranslator::buildRowAccessors()
 {
-    HashIterator iter(recordMap);
-    ForEach(iter)
-    {
-        ColumnToOffsetMap * map = static_cast<ColumnToOffsetMap *>(&iter.query());
-        buildRowAccessor(map);
-    }
-
+    for (auto& map : recordMap)
+        buildRowAccessor(&map);
 }
 
 void HqlCppTranslator::buildRowAccessor(ColumnToOffsetMap * map)

system/jlib/javahash.hpp

@@ -40,6 +40,7 @@ class JavaHashTableOf
     : public SuperHashTableOf<ELEMENT, ELEMENT>, implements IObserver
 {
     typedef JavaHashTableOf<ELEMENT> _SELF;
+
 private:
     virtual void onAdd(void *next) override;
     virtual void onRemove(void *) override;

system/jlib/jhash.ipp

@@ -255,37 +255,67 @@ class HashKeyOf : public MappingBase
 };
 
 
+template <class MAPPING>
+class KeptHashTableBase : extends KeptHashTable
+{
+  public:
+    typedef KeptHashTableBase<MAPPING> SELF;
+    friend class ConstHashItem;
+    class ConstHashItem
+    {
+    public:
+        ConstHashItem(const SELF & _self, unsigned _idx) : self(_self), idx(_idx) {}
+
+        MAPPING & operator * () const { return self.element(idx); }
+        bool operator != (const ConstHashItem & other) const { return &self != &other.self || idx != other.idx; }
+        ConstHashItem & operator ++ () { idx = self.validIdx(idx+1); return *this; }
+    private:
+        const SELF & self;
+        unsigned idx;
+    };
+
+  private:
+    bool remove(MAPPING * mem);
+    MAPPING & element(unsigned idx) const { return *static_cast<MAPPING *>(this->table[idx]); }
+
+  public:
+    KeptHashTableBase(int _keysize, bool _ignorecase) : KeptHashTable(_keysize, _ignorecase) {}
+    KeptHashTableBase(unsigned initsize, int _keysize, bool _ignorecase) : KeptHashTable(initsize, _keysize, _ignorecase) {}
+    ~KeptHashTableBase() {}
+
+    inline bool add(MAPPING &mem)               { return KeptHashTable::add(mem); }
+    inline bool removeExact(MAPPING * mem)           { return KeptHashTable::removeExact(mem); }
+    ConstHashItem begin() const { return ConstHashItem(*this, firstIdx()); }
+    ConstHashItem end() const { return ConstHashItem(*this, SELF::tablesize); }
+};
+
 template <class KEY, class MAPPING>
-class MapOf : extends KeptHashTable
+class MapOf : extends KeptHashTableBase<MAPPING>
 {
   private:
     bool remove(MAPPING * mem);
+
   public:
-    MapOf() : KeptHashTable(sizeof(KEY), false) {}
-    MapOf(unsigned initsize) : KeptHashTable(initsize, sizeof(KEY), false) {}
+    MapOf() : KeptHashTableBase<MAPPING>(sizeof(KEY), false) {}
+    MapOf(unsigned initsize) : KeptHashTableBase<MAPPING>(initsize, sizeof(KEY), false) {}
 
     ~MapOf()                                            {}
 
-    inline bool add(MAPPING &mem)               { return KeptHashTable::add(mem); }
     inline bool remove(const KEY & key)         { return KeptHashTable::remove(&key); }
-    inline bool removeExact(MAPPING * mem)           { return KeptHashTable::removeExact(mem); }
-    inline MAPPING * find(const KEY & key) const     { return (MAPPING *)KeptHashTable::find(&key); }
-    inline MAPPING * findLink(const KEY & key) const { return (MAPPING *)KeptHashTable::findLink(&key); }
+    inline MAPPING * find(const KEY & key) const     { return (MAPPING *)KeptHashTableBase<MAPPING>::find(&key); }
+    inline MAPPING * findLink(const KEY & key) const { return (MAPPING *)KeptHashTableBase<MAPPING>::findLink(&key); }
+
 };
 
 template <class MAPPING>
-class StringMapOf : extends KeptHashTable
+class StringMapOf : extends KeptHashTableBase<MAPPING>
 {
-  private:
-    bool remove(MAPPING * mem);
   public:
-    StringMapOf(bool _ignorecase) : KeptHashTable(0, _ignorecase) {}
-    StringMapOf(unsigned initsize, bool _ignorecase) : KeptHashTable(initsize, 0, _ignorecase) {}
+    StringMapOf(bool _ignorecase) : KeptHashTableBase<MAPPING>(0, _ignorecase) {}
+    StringMapOf(unsigned initsize, bool _ignorecase) : KeptHashTableBase<MAPPING>(initsize, 0, _ignorecase) {}
     ~StringMapOf()                                             {}
 
-    inline bool add(MAPPING &mem)               { return KeptHashTable::add(mem); }
     inline bool remove(const char *key)         { return KeptHashTable::remove(key); }
-    inline bool removeExact(MAPPING * mem)           { return KeptHashTable::removeExact(mem); }
     inline MAPPING * find(const char *key) const     { return (MAPPING *)KeptHashTable::find(key); }
 
 };

system/jlib/jstats.cpp

@@ -1641,9 +1641,8 @@ public:
                 maxValue.set(name.str());
         }
 
-        SuperHashIteratorOf<CStatisticCollection> iter(children, false);
-        for (iter.first(); iter.isValid(); iter.next())
-            iter.query().getMinMaxScope(minValue, maxValue, searchScopeType);
+        for (auto & curChild : children)
+            curChild.getMinMaxScope(minValue, maxValue, searchScopeType);
     }
 
     virtual void getMinMaxActivity(unsigned & minValue, unsigned & maxValue) const override

system/jlib/jsuperhash.cpp

@@ -100,14 +100,14 @@ SuperHashTable::~SuperHashTable()
     doKill();
 }
 
-#ifdef TRACE_HASH
 void SuperHashTable::dumpStats() const
 {
+#ifdef TRACE_HASH
     if (tablecount && search_tot && search_num)
         printf("Hash table %d entries, %d size, average search length %d(%d/%d) max %d\n", tablecount, tablesize,
                (int) (search_tot/search_num), search_tot, search_num, search_max);
-}
 #endif
+}
  
 #ifdef TRACE_HASH
 void SuperHashTable::note_searchlen(int len) const
@@ -448,6 +448,13 @@ bool SuperHashTable::removeExact(void *et)
     return true;
 }
 
+unsigned SuperHashTable::validIdx(unsigned i) const
+{
+    while (i < tablesize && !table[i])
+        i++;
+    return i;
+}
+
 void *SuperHashTable::findElement(unsigned hash, const void * findEt) const
 {
     unsigned vm = doFindElement(hash, findEt);

system/jlib/jsuperhash.hpp

@@ -48,9 +48,7 @@ public:
     void             ensure(unsigned mincount);
     void             releaseAll(); // like kill(), but does not resize the table
 
-#ifdef TRACE_HASH
     void dumpStats() const;
-#endif
 
 protected:
     void             init(unsigned initsize);
@@ -75,6 +73,9 @@ protected:
     inline unsigned  doFind(const void * findParam) const
       { return doFind(getHashFromFindParam(findParam), findParam); }
 
+    unsigned firstIdx() const { return validIdx(0); }
+    unsigned validIdx(unsigned i) const;
+
 private:
     bool             doAdd(void *, bool);
     void             doDeleteElement(unsigned);
@@ -111,6 +112,22 @@ protected:
 template <class ET, class FP>
 class SuperHashTableOf : public SuperHashTable
 {
+public:
+    typedef SuperHashTableOf<ET, FP> SELF;
+    friend class ConstHashItem;
+    class ConstHashItem
+    {
+    public:
+        ConstHashItem(const SELF & _self, unsigned _idx) : self(_self), idx(_idx) {}
+
+        ET & operator * () const { return self.element(idx); }
+        bool operator != (const ConstHashItem & other) const { return &self != &other.self || idx != other.idx; }
+        ConstHashItem & operator ++ () { idx = self.validIdx(idx+1); return *this; }
+    private:
+        const SELF & self;
+        unsigned idx;
+    };
+
   public:
     SuperHashTableOf(void) : SuperHashTable() {}
     SuperHashTableOf(unsigned initsize) : SuperHashTable(initsize) {}
@@ -134,6 +151,13 @@ class SuperHashTableOf : public SuperHashTable
       { return SuperHashTable::remove((const void *)(fp)); }
     inline bool      removeExact(ET * et)
       { return SuperHashTable::removeExact(et); }
+
+    ConstHashItem begin() const { return ConstHashItem(*this, firstIdx()); }
+    ConstHashItem end() const { return ConstHashItem(*this, SELF::tablesize); }
+
+  private:
+    ET & element(unsigned idx) const { return *static_cast<ET *>(this->table[idx]); }
+
 };
 
 // Macro to provide find method taking reference instead of pointer