Merge 81f4726fc3 into 6af1459f5e

src/Notifier.cc

@@ -62,9 +62,4 @@ void Registry::Terminate() {
     }
 }
 
-Modifiable::~Modifiable() {
-    if ( num_receivers )
-        registry.Unregister(this);
-}
-
 } // namespace zeek::notifier::detail

src/Notifier.h

@@ -99,8 +99,6 @@ extern Registry registry;
  */
 class Modifiable {
 public:
-    virtual ~Modifiable();
-
     /**
      * Calling this method signals to all registered receivers that the
      * object has been modified.
@@ -110,6 +108,17 @@ public:
             registry.Modified(this);
     }
 
+    /**
+     * Unregister this instance from the registry.
+     *
+     * Classes inheriting from Modifiable are required to call
+     * this from the destructor explicitly.
+     */
+    void Unregister() {
+        if ( num_receivers )
+            registry.Unregister(this);
+    }
+
 protected:
     friend class Registry;
 

src/Type.cc

@@ -18,6 +18,7 @@
 #include "zeek/Traverse.h"
 #include "zeek/Val.h"
 #include "zeek/Var.h"
+#include "zeek/ZVal.h"
 #include "zeek/module_util.h"
 #include "zeek/zeekygen/IdentifierInfo.h"
 #include "zeek/zeekygen/Manager.h"
@@ -1105,6 +1106,7 @@ void RecordType::AddField(unsigned int field, const TypeDecl* td) {
     ASSERT(field == managed_fields.size());
 
     managed_fields.push_back(ZVal::IsManagedType(td->type));
+    field_properties.push_back({.is_managed = ZVal::IsManagedType(td->type), .type_tag = td->type->Tag()});
 
     // We defer error-checking until here so that we can keep deferred_inits
     // and managed_fields correctly tracking the associated fields.
@@ -1539,6 +1541,15 @@ bool RecordType::IsDeferrable() const {
     return std::ranges::all_of(creation_inits, is_deferrable);
 }
 
+void RecordType::InitSlots(std::span<zeek::ZValSlot> slots) const {
+    int n = NumFields();
+    if ( slots.size() != field_properties.size() )
+        zeek::reporter->InternalError("wrong number of slots and slot properties");
+
+    for ( int i = 0; i < n; i++ )
+        slots[i] = field_properties[i];
+}
+
 FileType::FileType(TypePtr yield_type) : Type(TYPE_FILE), yield(std::move(yield_type)) {}
 
 FileType::~FileType() = default;

src/Type.h

@@ -21,6 +21,7 @@ namespace zeek {
 
 class Val;
 union ZVal;
+class ZValSlot;
 class EnumVal;
 class RecordVal;
 class TableVal;
@@ -635,6 +636,11 @@ private:
 
 using type_decl_list = PList<TypeDecl>;
 
+struct RecordFieldProperties {
+    bool is_managed = false;
+    TypeTag type_tag = TYPE_VOID;
+};
+
 class RecordType final : public Type {
 public:
     explicit RecordType(type_decl_list* types);
@@ -692,7 +698,11 @@ public:
 
     // Returns flags corresponding to which fields in the record
     // have types requiring memory management (reference counting).
-    const std::vector<bool>& ManagedFields() const { return managed_fields; }
+    [[deprecated("Remove in v9.1: Unused and optimization related internal. Use FieldProperties() instead.")]]
+    const std::vector<bool>& ManagedFields() const {
+        return managed_fields;
+    }
+    const std::vector<RecordFieldProperties>& FieldProperties() const { return field_properties; }
 
     int NumFields() const { return num_fields; }
     int NumOrigFields() const { return num_orig_fields; }
@@ -771,10 +781,16 @@ private:
     const auto& DeferredInits() const { return deferred_inits; }
     const auto& CreationInits() const { return creation_inits; }
 
+    // Initialize the slots using slot_properties.
+    void InitSlots(std::span<zeek::ZValSlot> slots) const;
+
     // If we were willing to bound the size of records, then we could
     // use std::bitset here instead.
     std::vector<bool> managed_fields;
 
+    // Field properties for ZvalSlot initialization
+    std::vector<RecordFieldProperties> field_properties;
+
     // Number of fields in the type.
     int num_fields = 0;
 

src/Val.cc

@@ -31,6 +31,7 @@
 #include "zeek/IPAddr.h"
 #include "zeek/IntrusivePtr.h"
 #include "zeek/NetVar.h"
+#include "zeek/Notifier.h"
 #include "zeek/Overflow.h"
 #include "zeek/PrefixTable.h"
 #include "zeek/RE.h"
@@ -43,6 +44,8 @@
 #include "zeek/broker/Store.h"
 #include "zeek/threading/formatters/detail/json.h"
 
+#include "zeek/3rdparty/doctest.h"
+
 using namespace std;
 
 namespace zeek {
@@ -1712,6 +1715,8 @@ void TableVal::Init(TableTypePtr t, bool ordered) {
 }
 
 TableVal::~TableVal() {
+    notifier::detail::Modifiable::Unregister();
+
     if ( timer )
         detail::timer_mgr->Cancel(timer);
 
@@ -2919,56 +2924,54 @@ TableVal::TableRecordDependencies TableVal::parse_time_table_record_dependencies
 
 RecordVal::RecordTypeValMap RecordVal::parse_time_records;
 
-RecordVal::RecordVal(RecordTypePtr t, bool init_fields) : Val(t), is_managed(t->ManagedFields()) {
+RecordVal::RecordVal(RecordTypePtr t, bool init_fields) : Val(std::move(t)) {
-    rt = std::move(t);
+    auto* rt = GetRecordType();
-
-    int n = rt->NumFields();
 
     if ( run_state::is_parsing )
-        parse_time_records[rt.get()].emplace_back(NewRef{}, this);
+        parse_time_records[rt].emplace_back(NewRef{}, this);
 
     if ( init_fields ) {
-        record_val.resize(n);
+        record_val.resize(rt->NumFields());
+
+        rt->InitSlots(std::span{record_val.data(), record_val.size()});
 
         for ( auto& e : rt->CreationInits() ) {
             try {
                 record_val[e.first] = e.second->Generate();
             } catch ( InterpreterException& e ) {
                 if ( run_state::is_parsing )
-                    parse_time_records[rt.get()].pop_back();
+                    parse_time_records[rt].pop_back();
                 throw;
             }
         }
     }
 
     else
-        record_val.reserve(n);
+        // This needs to go through AppendField() which will do the right thing
+        // for the individual slots.
+        record_val.reserve(rt->NumFields());
 }
 
-RecordVal::RecordVal(RecordTypePtr t, std::vector<std::optional<ZVal>> init_vals)
+RecordVal::RecordVal(RecordTypePtr t, std::vector<std::optional<ZVal>> init_vals) : Val(std::move(t)) {
-    : Val(t), is_managed(t->ManagedFields()) {
+    // TODO: Change so that callers pass init_vals as ZValSlot instead?
-    rt = std::move(t);
+    size_t n = GetRecordType()->NumFields();
-    record_val = std::move(init_vals);
+    record_val.resize(n);
+    GetRecordType()->InitSlots(std::span{record_val.data(), record_val.size()});
+
+    for ( size_t i = 0; i < n; i++ ) {
+        if ( init_vals[i].has_value() )
+            record_val[i] = init_vals[i].value();
+    }
 }
 
-RecordVal::~RecordVal() {
+RecordVal::~RecordVal() { notifier::detail::Modifiable::Unregister(); }
-    auto n = record_val.size();
-
-    for ( unsigned int i = 0; i < n; ++i ) {
-        auto f_i = record_val[i];
-        if ( f_i && IsManaged(i) )
-            ZVal::DeleteManagedType(*f_i);
-    }
-}
 
 ValPtr RecordVal::SizeVal() const { return val_mgr->Count(GetType()->AsRecordType()->NumFields()); }
 
 void RecordVal::Assign(int field, ValPtr new_val) {
     if ( new_val ) {
-        DeleteFieldIfManaged(field);
+        const auto& t = GetRecordType()->GetFieldType(field);
-
+        record_val[field] = ZValSlot(new_val, t);
-        auto t = rt->GetFieldType(field);
-        record_val[field] = ZVal(new_val, t);
         Modified();
     }
     else
@@ -2976,16 +2979,12 @@ void RecordVal::Assign(int field, ValPtr new_val) {
 }
 
 void RecordVal::Remove(int field) {
-    auto& f_i = record_val[field];
+    bool was_set = record_val[field].IsSet();
-    if ( f_i ) {
+    record_val[field].Reset();
-        if ( IsManaged(field) )
-            ZVal::DeleteManagedType(*f_i);
-
-        f_i = std::nullopt;
 
+    if ( was_set )
         Modified();
 }
-}
 
 ValPtr RecordVal::GetFieldOrDefault(int field) const {
     auto val = GetField(field);
@@ -3009,6 +3008,7 @@ void RecordVal::ResizeParseTimeRecords(RecordType* revised_rt) {
         auto required_length = revised_rt->NumFields();
 
         if ( required_length > current_length ) {
+            rv->record_val.reserve(required_length);
             for ( auto i = current_length; i < required_length; ++i )
                 rv->AppendField(revised_rt->FieldDefault(i), revised_rt->GetFieldType(i));
         }
@@ -3099,7 +3099,7 @@ void RecordVal::Describe(ODesc* d) const {
     auto n = record_val.size();
 
     if ( d->IsBinary() ) {
-        rt->Describe(d);
+        GetRecordType()->Describe(d);
         d->SP();
         d->Add(static_cast<uint64_t>(n));
         d->SP();
@@ -3111,7 +3111,7 @@ void RecordVal::Describe(ODesc* d) const {
         if ( ! d->IsBinary() && i > 0 )
             d->Add(", ");
 
-        d->Add(rt->FieldName(i));
+        d->Add(GetRecordType()->FieldName(i));
 
         if ( ! d->IsBinary() )
             d->Add("=");
@@ -3160,14 +3160,14 @@ ValPtr RecordVal::DoClone(CloneState* state) {
     // record. As we cannot guarantee that it will be zeroed out at the
     // appropriate time (as it seems to be guaranteed for the original record)
     // we don't touch it.
-    auto rv = make_intrusive<RecordVal>(rt, false);
+    auto rv = make_intrusive<RecordVal>(GetType<zeek::RecordType>(), false);
     state->NewClone(this, rv);
 
     int n = NumFields();
     for ( auto i = 0; i < n; ++i ) {
         auto f_i = GetField(i);
         auto v = f_i ? f_i->Clone(state) : nullptr;
-        rv->AppendField(std::move(v), rt->GetFieldType(i));
+        rv->AppendField(std::move(v), GetRecordType()->GetFieldType(i));
     }
 
     return rv;
@@ -3234,6 +3234,8 @@ VectorVal::VectorVal(VectorTypePtr t, std::vector<std::optional<ZVal>>* vals) :
 }
 
 VectorVal::~VectorVal() {
+    notifier::detail::Modifiable::Unregister();
+
     if ( yield_types ) {
         int n = yield_types->size();
         for ( auto i = 0; i < n; ++i ) {
@@ -4113,3 +4115,127 @@ const PortValPtr& ValManager::Port(uint32_t port_num) {
 }
 
 } // namespace zeek
+
+TEST_SUITE_BEGIN("ZValSlot");
+
+TEST_CASE("default constructor") {
+    // default constructor doesn't do anything.
+    zeek::ZValSlot slot;
+}
+
+TEST_CASE("slot hold CountVal") {
+    auto t = zeek::base_type(zeek::TYPE_COUNT);
+    auto v = zeek::make_intrusive<zeek::CountVal>(104242);
+    zeek::ZValSlot slot = zeek::ZValSlot(v, t);
+
+    CHECK(slot.IsSet());
+    CHECK(slot.Tag() == zeek::TYPE_COUNT);
+    CHECK(! slot.IsManaged());
+    CHECK(v->RefCnt() == 1); // Not managed, so the slot does not hold a ref to the original value.
+    CHECK(slot->AsCount() == 104242);
+
+    auto nv = slot->ToVal(t);
+    CHECK(nv->RefCnt() == 1); // Not managed, so the slot does not hold a ref to the original value.
+}
+
+TEST_CASE("slot hold RecordVal") {
+    auto t = zeek::id::find_type<zeek::RecordType>("conn_id_ctx");
+    auto v = zeek::make_intrusive<zeek::RecordVal>(t);
+    CHECK(v->RefCnt() == 1);
+    zeek::ZValSlot slot = zeek::ZValSlot(v, t);
+
+    CHECK(slot.IsSet());
+    CHECK(slot.Tag() == zeek::TYPE_RECORD);
+    CHECK(slot.IsManaged());
+    CHECK(v->RefCnt() == 2); // Managed, slot takes a ref.
+
+    slot.Reset();
+
+    CHECK(v->RefCnt() == 1);
+    v = nullptr;
+}
+
+TEST_CASE("assign count ZVal to slot") {
+    auto t = zeek::base_type(zeek::TYPE_COUNT);
+    auto v1 = zeek::make_intrusive<zeek::CountVal>(42);
+    auto v2 = zeek::make_intrusive<zeek::CountVal>(100000);
+
+    zeek::ZValSlot slot = zeek::ZValSlot(v1, t);
+    CHECK(v1->RefCnt() == 1);
+
+    slot = zeek::ZVal(v2, t);
+
+    // Unmanaged
+    CHECK(v1->RefCnt() == 1);
+    CHECK(v2->RefCnt() == 1);
+
+    auto v3 = slot->ToVal(t);
+
+    // New CountVal for 100000
+    CHECK(v3 != v2);
+    CHECK(v3->RefCnt() == 1);
+}
+
+TEST_CASE("assign record ZVal to slot") {
+    auto t = zeek::id::find_type<zeek::RecordType>("conn_id_ctx");
+    auto v1 = zeek::make_intrusive<zeek::RecordVal>(t);
+    auto v2 = zeek::make_intrusive<zeek::RecordVal>(t);
+
+    zeek::ZValSlot slot = zeek::ZValSlot(v1, t);
+    CHECK(v1->RefCnt() == 2); // v1 and slot
+
+    slot = zeek::ZVal(v2, t);
+    CHECK(v1->RefCnt() == 1); // slot released
+    CHECK(v2->RefCnt() == 2); // v2 and slot
+
+    auto v3 = slot->ToVal(t);
+    CHECK(v3 == v2);
+    CHECK(v2->RefCnt() == 3); // v2, slot and v3
+}
+
+TEST_CASE("assign slot assignment") {
+    auto t = zeek::id::find_type<zeek::RecordType>("conn_id_ctx");
+    auto v1 = zeek::make_intrusive<zeek::RecordVal>(t);
+    auto v2 = zeek::make_intrusive<zeek::RecordVal>(t);
+
+    zeek::ZValSlot slot1 = zeek::ZValSlot(v1, t);
+    zeek::ZValSlot slot2 = zeek::ZValSlot(v2, t);
+
+    CHECK(v1->RefCnt() == 2); // v1 and slot1
+    CHECK(v2->RefCnt() == 2); // v2 and slot2
+
+    slot1 = slot2;
+    CHECK(v1->RefCnt() == 1); // slot1 released
+    CHECK(v2->RefCnt() == 3); // v2, slot1 and slot2
+}
+
+TEST_CASE("copy slot") {
+    auto t = zeek::id::find_type<zeek::RecordType>("conn_id_ctx");
+    auto v = zeek::make_intrusive<zeek::RecordVal>(t);
+
+    zeek::ZValSlot slot1 = zeek::ZValSlot(v, t);
+    zeek::ZValSlot slot2 = slot1;
+
+    CHECK(v->RefCnt() == 3); // v1, slot1 and slot2
+}
+
+TEST_SUITE_END();
+
+TEST_SUITE_BEGIN("RecordVal");
+
+TEST_CASE("assign string") {
+    auto t = zeek::id::find_type<zeek::RecordType>("PacketSource");
+    auto v = zeek::make_intrusive<zeek::RecordVal>(t);
+
+    std::string path = "test-path";
+
+    v->Assign(1, path);
+
+    auto sv = v->GetField(1);
+    CHECK(sv->RefCnt() == 2); // sv and v
+
+    v.reset();
+    CHECK(sv->RefCnt() == 1); // record was destroyed
+}
+
+TEST_SUITE_END();

src/Val.h

@@ -4,6 +4,7 @@
 
 #include <sys/types.h> // for u_char
 #include <array>
+#include <memory>
 #include <string>
 #include <string_view>
 #include <unordered_map>
@@ -1114,6 +1115,213 @@ struct is_zeek_val {
 template<typename T>
 inline constexpr bool is_zeek_val_v = is_zeek_val<T>::value;
 
+/**
+ * A ZValSlot holds a ZVal instance and some auxiliary information that allows automatic
+ * memory management as well as acting as an optional unset field.
+ *
+ * This class originated from the observation that a std::optional<ZVal>
+ * as previously used in VectorVal and RecordVal objects already takes up
+ * 16 bytes on 64 bit architectures with GCC. The ZValSlot class essentially
+ * uses the left-over 7 bytes from the std::optional to allow easier memory
+ * management without needing to keep external auxilarly information around.
+ *
+ * The is_managed flag and type_tag flags are meant to be immutable except
+ * when a ZValSlot is reassigned.
+ *
+ * A ZValSlot instance holds a reference to a managed value. Such ZValSlot
+ * instances can be copied or assigned and the reference count of the ZVal
+ * will be updated accordingly.
+ */
+class ZValSlot {
+public:
+    /**
+     * Totally uninitialized, watch out!
+     */
+    ZValSlot() {}
+
+    /**
+     * Initialize a set ZValSlot given a ValPtr and corresponding TypePtr.
+     *
+     * This has the same ref counting semantics as the corresponding ZVal
+     * constructor, increasing the ref count of any managed value.
+     */
+    ZValSlot(ValPtr v, const TypePtr& t)
+        : is_set(true), is_managed(ZVal::IsManagedType(t)), type_tag(t->Tag()), zval(v, t) {}
+
+    /**
+     * Initialize a ZValSlot using its properties.
+     */
+    ZValSlot(const RecordFieldProperties p) : is_set(false), is_managed(p.is_managed), type_tag(p.type_tag) {}
+
+    /**
+     * Initialize a ZValSlot with just the TypePtr.
+     *
+     * This is useful for optional fields in a record value where
+     * the type is known at construction time.
+     */
+    ZValSlot(const TypePtr& t) : is_set(false), is_managed(ZVal::IsManagedType(t)), type_tag(t->Tag()) {}
+
+    /**
+     * Copy constructor.
+     */
+    ZValSlot(const ZValSlot& s) : is_set(s.is_set), is_managed(s.is_managed), type_tag(s.type_tag), zval(s.zval) {
+        if ( is_set && is_managed )
+            Ref(zval.ManagedVal());
+    }
+
+    /**
+     * Destructor.
+     */
+    ~ZValSlot() { Reset(); }
+
+    ZValSlot& operator=(const ZValSlot& s) {
+        if ( is_set && is_managed )
+            Unref(zval.ManagedVal());
+
+        is_set = s.is_set;
+        is_managed = s.is_managed;
+        type_tag = s.type_tag;
+        zval = s.zval;
+
+        if ( is_set && is_managed )
+            Ref(zval.ManagedVal());
+
+        return *this;
+    }
+
+    /**
+     * Assign a ZVal to a slot.
+     *
+     * This uses the is_managed member to determine if the
+     * given ZVal should be treated as managed and whether.
+     *
+     * Assigning a ZVal to a managed slot adopts a reference!
+     * This is a bit quirky, but it's what the plain ZVal
+     * constructors also do.
+     */
+    ZValSlot& operator=(const ZVal& zv) {
+        if ( is_set && is_managed )
+            Unref(zval.ManagedVal());
+
+        is_set = true;
+        zval = zv;
+
+        return *this;
+    }
+
+    operator bool() const noexcept { return is_set; }
+    const ZVal* operator->() const noexcept { return &zval; }
+    ZVal& operator*() noexcept { return zval; }
+    const ZVal& operator*() const noexcept { return zval; }
+
+    bool IsSet() const noexcept { return is_set; }
+    bool IsManaged() const noexcept { return is_managed; }
+    TypeTag Tag() const noexcept { return type_tag; }
+
+    void Reset() {
+        if ( is_set && is_managed )
+            Unref(zval.ManagedVal());
+
+        is_set = false;
+    }
+
+    /**
+     * Convert a slot's ZVal to a ValPtr given a TypePtr.
+     *
+     * @param t Type to use for conversion to Val. Needs to agree with the type that was used to initialize the
+     * slot.
+     *
+     * @return A ValPtr instance for the slot.
+     */
+    ValPtr ToVal(const TypePtr& t) {
+        assert(IsSet());
+        // assert(Tag() == TYPE_ANY || Tag() == t->Tag());
+
+        return zval.ToVal(t);
+    }
+
+private:
+    bool is_set;
+    bool is_managed;
+    TypeTag type_tag;
+    ZVal zval;
+};
+
+static_assert(sizeof(ZValSlot) <= 16);
+
+namespace detail {
+/**
+ * A std::vector replacement for record slots with 32bit size and capacity so we
+ * end up with a 16 byte vector instead of 24 bytes. Anyone trying to put more than
+ * 500mio fields into a record likely has other problems to solve, first :-)
+ *
+ * Going to 16 bit doesn't save much as the whole class will still take 16 bytes
+ * with padding on a 64bit system.
+ */
+template<class T>
+class vector32 {
+public:
+    vector32() {}
+    vector32(size_t size) : d(std::make_unique<T[]>(size)), sz(size), cap(size) {}
+
+    const T& operator[](size_t i) const noexcept { return d[i]; }
+    T& operator[](size_t i) noexcept { return d[i]; }
+    const T* data() const noexcept { return d.get(); }
+    T* data() noexcept { return d.get(); }
+
+    void push_back(T slot) {
+        // No automatic resizing
+        if ( cap <= sz )
+            throw std::logic_error("capacity exceeded");
+
+        d[sz++] = slot;
+    }
+
+    void resize(size_t new_size) {
+        if ( new_size < sz || new_size < cap )
+            throw std::length_error("cannot truncate");
+
+        if ( new_size > std::numeric_limits<decltype(sz)>::max() )
+            throw std::length_error("new_size too large");
+
+        if ( new_size > cap ) {
+            std::unique_ptr<T[]> new_data = std::make_unique<T[]>(new_size);
+            for ( size_t i = 0; i < sz; i++ )
+                new_data[i] = std::move(d[i]);
+            d = std::move(new_data);
+            sz = new_size;
+            cap = new_size;
+        }
+
+        sz = new_size;
+    }
+
+    void reserve(size_t new_capacity) {
+        if ( cap > new_capacity )
+            throw std::length_error("cannot truncate");
+
+        if ( new_capacity > std::numeric_limits<decltype(cap)>::max() )
+            throw std::length_error("new_capacity too large");
+
+        if ( new_capacity > cap ) {
+            std::unique_ptr<T[]> new_data = std::make_unique<T[]>(new_capacity);
+            for ( size_t i = 0; i < sz; i++ )
+                new_data[i] = std::move(d[i]);
+            d = std::move(new_data);
+            cap = new_capacity;
+        }
+    }
+
+    size_t size() const noexcept { return sz; }
+    size_t capacity() const noexcept { return cap; }
+
+private:
+    std::unique_ptr<T[]> d = nullptr;
+    uint32_t sz = 0;
+    uint32_t cap = 0;
+};
+} // namespace detail
+
 class RecordVal final : public Val, public notifier::detail::Modifiable {
 public:
     explicit RecordVal(RecordTypePtr t, bool init_fields = true);
@@ -1186,10 +1394,7 @@ public:
     void AssignInterval(int field, double new_val) { Assign(field, new_val); }
 
     void Assign(int field, StringVal* new_val) {
-        auto& fv = record_val[field];
+        record_val[field] = ZVal(new_val);
-        if ( fv )
-            ZVal::DeleteManagedType(*fv);
-        fv = ZVal(new_val);
         AddedField(field);
     }
     void Assign(int field, const char* new_val) { Assign(field, new StringVal(new_val)); }
@@ -1203,7 +1408,7 @@ public:
      */
     template<class T>
     void AssignField(const char* field_name, T&& val) {
-        int idx = rt->FieldOffset(field_name);
+        int idx = GetRecordType()->FieldOffset(field_name);
         if ( idx < 0 )
             reporter->InternalError("missing record field: %s", field_name);
         Assign(idx, std::forward<T>(val));
@@ -1225,7 +1430,7 @@ public:
         if ( record_val[field] )
             return true;
 
-        return rt->DeferredInits()[field] != nullptr;
+        return GetRecordType()->DeferredInits()[field] != nullptr;
     }
 
     /**
@@ -1235,7 +1440,7 @@ public:
      * @return  Whether there's a value for the given field name.
      */
     bool HasField(const char* field) const {
-        int idx = rt->FieldOffset(field);
+        int idx = GetRecordType()->FieldOffset(field);
         return (idx != -1) && HasField(idx);
     }
 
@@ -1247,14 +1452,14 @@ public:
     ValPtr GetField(int field) const {
         auto& fv = record_val[field];
         if ( ! fv ) {
-            const auto& fi = rt->DeferredInits()[field];
+            const auto& fi = GetRecordType()->DeferredInits()[field];
             if ( ! fi )
                 return nullptr;
 
             fv = fi->Generate();
         }
 
-        return fv->ToVal(rt->GetFieldType(field));
+        return fv.ToVal(GetRecordType()->GetFieldType(field));
     }
 
     /**
@@ -1321,7 +1526,7 @@ public:
 
     // Returns true if the slot for the given field is initialized.
     // This helper can be used to guard GetFieldAs() accesses.
-    bool HasRawField(int field) const { return record_val[field].has_value(); }
+    bool HasRawField(int field) const { return record_val[field].IsSet(); }
 
     // The following return the given field converted to a particular
     // underlying value.  We provide these to enable efficient
@@ -1384,7 +1589,7 @@ public:
 
     template<typename T>
     auto GetFieldAs(const char* field) const {
-        int idx = rt->FieldOffset(field);
+        int idx = GetRecordType()->FieldOffset(field);
 
         if ( idx < 0 )
             reporter->InternalError("missing record field: %s", field);
@@ -1451,19 +1656,21 @@ protected:
      */
     void AppendField(ValPtr v, const TypePtr& t) {
         if ( v )
-            record_val.emplace_back(ZVal(v, t));
+            record_val.push_back(ZValSlot(v, t));
         else
-            record_val.emplace_back(std::nullopt);
+            record_val.push_back(ZValSlot(t));
     }
 
     // For internal use by low-level ZAM instructions and event tracing.
     // Caller assumes responsibility for memory management.  The first
     // version allows manipulation of whether the field is present at all.
     // The second version ensures that the optional value is present.
-    std::optional<ZVal>& RawOptField(int field) {
+    //
+    // TODO: Fix name!
+    ZValSlot& RawOptField(int field) {
         auto& f = record_val[field];
         if ( ! f ) {
-            const auto& fi = rt->DeferredInits()[field];
+            const auto& fi = GetRecordType()->DeferredInits()[field];
             if ( fi )
                 f = fi->Generate();
         }
@@ -1471,10 +1678,13 @@ protected:
         return f;
     }
 
+    // TODO: Fix name!
     ZVal& RawField(int field) {
         auto& f = RawOptField(field);
         if ( ! f )
             f = ZVal();
+
+        assert(f.IsSet());
         return *f;
     }
 
@@ -1484,33 +1694,23 @@ protected:
 
     Obj* origin = nullptr;
 
-    using RecordTypeValMap = std::unordered_map<RecordType*, std::vector<RecordValPtr>>;
+    using RecordTypeValMap = std::unordered_map<const RecordType*, std::vector<RecordValPtr>>;
     static RecordTypeValMap parse_time_records;
 
 private:
-    void DeleteFieldIfManaged(unsigned int field) {
-        auto& f = record_val[field];
-        if ( f && IsManaged(field) )
-            ZVal::DeleteManagedType(*f);
-    }
-
-    bool IsManaged(unsigned int offset) const { return is_managed[offset]; }
-
     // Just for template inferencing.
     RecordVal* Get() { return this; }
+    const RecordType* GetRecordType() const noexcept {
+        assert(GetType()->Tag() == TYPE_RECORD);
+        return static_cast<RecordType*>(GetType().get());
+    }
 
     unsigned int ComputeFootprint(std::unordered_set<const Val*>* analyzed_vals) const override;
 
-    // Keep this handy for quick access during low-level operations.
-    RecordTypePtr rt;
-
     // Low-level values of each of the fields.
     //
     // Lazily modified during GetField(), so mutable.
-    mutable std::vector<std::optional<ZVal>> record_val;
+    mutable detail::vector32<ZValSlot> record_val;
-
-    // Whether a given field requires explicit memory management.
-    const std::vector<bool>& is_managed;
 };
 
 class EnumVal final : public detail::IntValImplementation {

src/logging/Manager.cc

@@ -1654,14 +1654,16 @@ detail::LogRecord Manager::RecordToLogRecord(WriterInfo* info, Filter* filter, c
 
         for ( int index : indices ) {
             auto vr = val->AsRecord();
-            val = vr->RawOptField(index);
+            const auto& slot = vr->RawOptField(index);
 
-            if ( ! val ) {
+            if ( ! slot.IsSet() ) {
                 // Value, or any of its parents, is not set.
                 vals.emplace_back(filter->fields[i]->type, false);
+                val = std::nullopt;
                 break;
             }
 
+            val = *slot;
             vt = cast_intrusive<RecordType>(vr->GetType())->GetFieldType(index).get();
         }
 

src/script_opt/ZAM/OPs/constructors.op

@@ -81,8 +81,6 @@ macro	AssignFromRec(rhs)
 		if ( is_managed[i] )
 			{
 			zeek::Ref(rhs_i.ManagedVal());
-			if ( init_i )
-				ZVal::DeleteManagedType(*init_i);
 			}
 		init_i = rhs_i;
 		}
@@ -166,8 +164,6 @@ macro DoManagedRecAssign(lhs, rhs)
 			auto& lhs_i = lhs->RawOptField(lhs_map[i]);
 			auto rhs_i = FieldValWithCheck(rhs, rhs_map[i]);
 			zeek::Ref(rhs_i.ManagedVal());
-			if ( lhs_i )
-				ZVal::DeleteManagedType(*lhs_i);
 			lhs_i = rhs_i;
 			}
 		else
@@ -190,8 +186,6 @@ eval	SetUpRecFieldOps(map)
 		auto& lhs_i = $1->RawOptField(lhs_map[i]);
 		auto rhs_i = FieldValWithCheck($2, rhs_map[i]);
 		zeek::Ref(rhs_i.ManagedVal());
-		if ( lhs_i )
-			ZVal::DeleteManagedType(*lhs_i);
 		lhs_i = rhs_i;
 		}