streamlining of constructing script-level tables

src/Type.cc

@@ -10,6 +10,7 @@
 #include <unordered_set>
 
 #include "zeek/Attr.h"
+#include "zeek/CompHash.h"
 #include "zeek/Desc.h"
 #include "zeek/Expr.h"
 #include "zeek/Reporter.h"
@@ -478,8 +479,13 @@ TableType::TableType(TypeListPtr ind, TypePtr yield) : IndexType(TYPE_TABLE, std
             break;
         }
     }
+
+    if ( Tag() != TYPE_ERROR )
+        RegenerateHash();
 }
 
+TableType::~TableType() { delete table_hash; }
+
 bool TableType::CheckExpireFuncCompatibility(const detail::AttrPtr& attr) {
     if ( reported_error )
         return false;
@@ -493,6 +499,11 @@ bool TableType::CheckExpireFuncCompatibility(const detail::AttrPtr& attr) {
 
 TypePtr TableType::ShallowClone() { return make_intrusive<TableType>(indices, yield_type); }
 
+void TableType::RegenerateHash() {
+    delete table_hash;
+    table_hash = new detail::CompositeHash(GetIndices());
+}
+
 bool TableType::IsUnspecifiedTable() const {
     // Unspecified types have an empty list of indices.
     return indices->GetTypes().empty();

src/Type.h

@@ -29,6 +29,7 @@ using TableValPtr = IntrusivePtr<TableVal>;
 
 namespace detail {
 
+class CompositeHash;
 class Expr;
 class ListExpr;
 class Attributes;
@@ -354,22 +355,20 @@ public:
     void DescribeReST(ODesc* d, bool roles_only = false) const override;
 
     // Returns true if this table is solely indexed by subnet.
-    bool IsSubNetIndex() const {
+    bool IsSubNetIndex() const { return is_subnet_index; }
-        const auto& types = indices->GetTypes();
-        return types.size() == 1 && types[0]->Tag() == TYPE_SUBNET;
-    }
 
     // Returns true if this table has a single index of type pattern.
-    bool IsPatternIndex() const {
+    bool IsPatternIndex() const { return is_pattern_index; }
-        const auto& types = indices->GetTypes();
-        return types.size() == 1 && types[0]->Tag() == TYPE_PATTERN;
-    }
 
     detail::TraversalCode Traverse(detail::TraversalCallback* cb) const override;
 
 protected:
     IndexType(TypeTag t, TypeListPtr arg_indices, TypePtr arg_yield_type)
-        : Type(t), indices(std::move(arg_indices)), yield_type(std::move(arg_yield_type)) {}
+        : Type(t), indices(std::move(arg_indices)), yield_type(std::move(arg_yield_type)) {
+        const auto& types = indices->GetTypes();
+        is_subnet_index = types.size() == 1 && types[0]->Tag() == TYPE_SUBNET;
+        is_pattern_index = types.size() == 1 && types[0]->Tag() == TYPE_PATTERN;
+    }
 
     ~IndexType() override = default;
 
@@ -377,12 +376,17 @@ protected:
 
     TypeListPtr indices;
     TypePtr yield_type;
+
+    bool is_subnet_index;
+    bool is_pattern_index;
 };
 
 class TableType : public IndexType {
 public:
     TableType(TypeListPtr ind, TypePtr yield);
 
+    ~TableType();
+
     /**
      * Assesses whether an &expire_func attribute's function type is compatible
      * with this table type.
@@ -398,9 +402,17 @@ public:
     // what one gets using an empty "set()" or "table()" constructor.
     bool IsUnspecifiedTable() const;
 
+    const detail::CompositeHash* GetTableHash() const { return table_hash; }
+
+    // Called to rebuild the associated hash function when a record type
+    // (that this table type depends on) gets redefined during parsing.
+    void RegenerateHash();
+
 private:
     bool DoExpireCheck(const detail::AttrPtr& attr);
 
+    detail::CompositeHash* table_hash = nullptr;
+
     // Used to prevent repeated error messages.
     bool reported_error = false;
 };

src/Val.cc

@@ -1568,7 +1568,6 @@ void TableVal::Init(TableTypePtr t, bool ordered) {
     if ( table_type->IsPatternIndex() )
         pattern_matcher = std::make_unique<detail::TablePatternMatcher>(this, table_type->Yield());
 
-    table_hash = new detail::CompositeHash(table_type->GetIndices());
     if ( ordered )
         table_val = new PDict<TableEntryVal>(DictOrder::ORDERED);
     else
@@ -1581,7 +1580,6 @@ TableVal::~TableVal() {
     if ( timer )
         detail::timer_mgr->Cancel(timer);
 
-    delete table_hash;
     delete table_val;
     delete expire_iterator;
 }
@@ -1756,7 +1754,7 @@ bool TableVal::AddTo(Val* val, bool is_first_init, bool propagate_ops) const {
         auto* v = tble.value;
 
         if ( is_first_init && t->AsTable()->Lookup(k.get()) ) {
-            auto key = table_hash->RecoverVals(*k);
+            auto key = GetTableHash()->RecoverVals(*k);
             // ### Shouldn't complain if their values are equal.
             key->Warn("multiple initializations for index");
             continue;
@@ -2081,7 +2079,7 @@ bool TableVal::UpdateTimestamp(Val* index) {
     return true;
 }
 
-ListValPtr TableVal::RecreateIndex(const detail::HashKey& k) const { return table_hash->RecoverVals(k); }
+ListValPtr TableVal::RecreateIndex(const detail::HashKey& k) const { return GetTableHash()->RecoverVals(k); }
 
 void TableVal::CallChangeFunc(const ValPtr& index, const ValPtr& old_value, OnChangeType tpe) {
     if ( ! change_func || ! index || in_change_func )
@@ -2270,7 +2268,7 @@ ValPtr TableVal::Remove(const detail::HashKey& k, bool* iterators_invalidated) {
         va = v->GetVal() ? v->GetVal() : IntrusivePtr{NewRef{}, this};
 
     if ( subnets ) {
-        auto index = table_hash->RecoverVals(k);
+        auto index = GetTableHash()->RecoverVals(k);
 
         if ( ! subnets->Remove(index.get()) )
             reporter->InternalWarning("index not in prefix table");
@@ -2281,7 +2279,7 @@ ValPtr TableVal::Remove(const detail::HashKey& k, bool* iterators_invalidated) {
     Modified();
 
     if ( va && (change_func || ! broker_store.empty()) ) {
-        auto index = table_hash->RecoverVals(k);
+        auto index = GetTableHash()->RecoverVals(k);
         if ( ! broker_store.empty() )
             SendToStore(index.get(), nullptr, ELEMENT_REMOVED);
 
@@ -2297,7 +2295,7 @@ ListValPtr TableVal::ToListVal(TypeTag t) const {
 
     for ( const auto& tble : *table_val ) {
         auto k = tble.GetHashKey();
-        auto index = table_hash->RecoverVals(*k);
+        auto index = GetTableHash()->RecoverVals(*k);
 
         if ( t == TYPE_ANY )
             l->Append(std::move(index));
@@ -2329,7 +2327,7 @@ std::unordered_map<ValPtr, ValPtr> TableVal::ToMap() const {
     for ( const auto& iter : *table_val ) {
         auto k = iter.GetHashKey();
         auto v = iter.value;
-        auto vl = table_hash->RecoverVals(*k);
+        auto vl = GetTableHash()->RecoverVals(*k);
 
         res[std::move(vl)] = v->GetVal();
     }
@@ -2366,7 +2364,7 @@ void TableVal::Describe(ODesc* d) const {
         auto k = iter->GetHashKey();
         auto* v = iter->value;
 
-        auto vl = table_hash->RecoverVals(*k);
+        auto vl = GetTableHash()->RecoverVals(*k);
         int dim = vl->Length();
 
         ODesc intermediary_d;
@@ -2705,7 +2703,7 @@ unsigned int TableVal::ComputeFootprint(std::unordered_set<const Val*>* analyzed
 
     for ( const auto& iter : *table_val ) {
         auto k = iter.GetHashKey();
-        auto vl = table_hash->RecoverVals(*k);
+        auto vl = GetTableHash()->RecoverVals(*k);
         auto v = iter.value->GetVal();
 
         fp += vl->Footprint(analyzed_vals);
@@ -2717,7 +2715,7 @@ unsigned int TableVal::ComputeFootprint(std::unordered_set<const Val*>* analyzed
 }
 
 std::unique_ptr<detail::HashKey> TableVal::MakeHashKey(const Val& index) const {
-    return table_hash->MakeHashKey(index, true);
+    return GetTableHash()->MakeHashKey(index, true);
 }
 
 void TableVal::SaveParseTimeTableState(RecordType* rt) {
@@ -2733,8 +2731,17 @@ void TableVal::SaveParseTimeTableState(RecordType* rt) {
 }
 
 void TableVal::RebuildParseTimeTables() {
-    for ( auto& [tv, ptts] : parse_time_table_states )
+    std::set<TableType*> table_types; // regenerate hash just once per table type
+
+    for ( auto& [tv, ptts] : parse_time_table_states ) {
+        auto* tt = tv->table_type.get();
+        if ( table_types.count(tt) == 0 ) {
+            tt->RegenerateHash();
+            table_types.insert(tt);
+        }
+
         tv->RebuildTable(std::move(ptts));
+    }
 
     parse_time_table_states.clear();
 }
@@ -2755,9 +2762,6 @@ TableVal::ParseTimeTableState TableVal::DumpTableState() {
 }
 
 void TableVal::RebuildTable(ParseTimeTableState ptts) {
-    delete table_hash;
-    table_hash = new detail::CompositeHash(table_type->GetIndices());
-
     for ( auto& [key, val] : ptts )
         Assign(std::move(key), std::move(val));
 }

src/Val.h

@@ -49,7 +49,6 @@ namespace detail {
 class ScriptFunc;
 class Frame;
 class PrefixTable;
-class CompositeHash;
 class HashKey;
 class TablePatternMatcher;
 
@@ -930,7 +929,7 @@ public:
 
     const PDict<TableEntryVal>* Get() const { return table_val; }
 
-    const detail::CompositeHash* GetTableHash() const { return table_hash; }
+    const detail::CompositeHash* GetTableHash() const { return table_type->GetTableHash(); }
 
     // Returns the size of the table.
     int Size() const;
@@ -1043,7 +1042,6 @@ protected:
     ValPtr DoClone(CloneState* state) override;
 
     TableTypePtr table_type;
-    detail::CompositeHash* table_hash;
     detail::AttributesPtr attrs;
     detail::ExprPtr expire_time;
     detail::ExprPtr expire_func;