diff --git a/src/EventTrace.cc b/src/EventTrace.cc index 3298447e89..6c7792a90b 100644 --- a/src/EventTrace.cc +++ b/src/EventTrace.cc @@ -226,9 +226,9 @@ void ValTrace::TraceRecord(const RecordValPtr& rv) { auto rt = rv->GetType(); for ( auto i = 0U; i < n; ++i ) { - auto f = rv->RawOptField(i); - if ( f ) { - auto val = f->ToVal(rt->GetFieldType(i)); + auto slot = rv->RawOptField(i); + if ( slot.IsSet() ) { + auto val = slot.ToVal(rt->GetFieldType(i)); elems.emplace_back(std::make_shared(val)); } else diff --git a/src/Val.cc b/src/Val.cc index a064b8aab1..0016a9a335 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2931,43 +2931,56 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields) : Val(t), is_managed(t-> for ( auto& e : rt->CreationInits() ) { try { - record_val[e.first] = e.second->Generate(); + record_val[e.first].Set(e.second->Generate()); } catch ( InterpreterException& e ) { if ( run_state::is_parsing ) parse_time_records[rt.get()].pop_back(); throw; } } + + // Initialize the managed flag of each slot so that we don't need + // to go back to record type for figuring out if it's managed. + for ( size_t i = 0; i < record_val.size(); i++ ) + record_val[i].SetManaged(IsManaged(i)); } else + // This has to go through AppendField() which will + // initialize the slots managed flag properly. record_val.reserve(n); } RecordVal::RecordVal(RecordTypePtr t, std::vector> init_vals) : Val(t), is_managed(t->ManagedFields()) { rt = std::move(t); - record_val = std::move(init_vals); + + record_val.resize(rt->NumFields()); + + // std::fprintf(stderr, "optimized\n"); + + for ( size_t i = 0; i < record_val.size(); ++i ) { + if ( init_vals[i] ) + record_val[i].Set(*init_vals[i]); + + record_val[i].SetManaged(IsManaged(i)); + } } RecordVal::~RecordVal() { - 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); - } + for ( auto& slot : record_val ) + slot.Delete(); } ValPtr RecordVal::SizeVal() const { return val_mgr->Count(GetType()->AsRecordType()->NumFields()); } void RecordVal::Assign(int field, ValPtr new_val) { + auto& slot = record_val[field]; if ( new_val ) { - DeleteFieldIfManaged(field); + slot.Delete(); auto t = rt->GetFieldType(field); - record_val[field] = ZVal(new_val, t); + slot.Set(ZVal(new_val, t)); Modified(); } else @@ -2975,12 +2988,12 @@ void RecordVal::Assign(int field, ValPtr new_val) { } void RecordVal::Remove(int field) { - auto& f_i = record_val[field]; - if ( f_i ) { - if ( IsManaged(field) ) - ZVal::DeleteManagedType(*f_i); - - f_i = std::nullopt; + auto& slot = record_val[field]; + if ( slot.IsSet() ) { + assert(! slot.IsDeleted()); + slot.Delete(); + assert(! slot.IsSet()); + assert(slot.IsDeleted()); Modified(); } diff --git a/src/Val.h b/src/Val.h index 5f2fc740a6..713d83a366 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1114,6 +1114,80 @@ struct is_zeek_val { template inline constexpr bool is_zeek_val_v = is_zeek_val::value; +namespace detail { + +enum class RecordValSlotFlags : uint8_t { + None = 0, + Managed = 1, // replicates + Set = 2, + Deleted = 4, +}; + +constexpr RecordValSlotFlags operator|(RecordValSlotFlags l, RecordValSlotFlags r) { + return static_cast(static_cast(l) | static_cast(r)); +} +constexpr RecordValSlotFlags operator&(RecordValSlotFlags l, RecordValSlotFlags r) { + return static_cast(static_cast(l) & static_cast(r)); +} + +constexpr RecordValSlotFlags operator~(RecordValSlotFlags x) { + return static_cast(~static_cast(x)); +} + +/** + * Representation of a single slot for a field in a RecordVal. + * + * Previously this was a std::optional that was using 16bytes. This has + * been switched to a struct to be able to have access to the wasted bytes to + * store some additional flags. + */ +class RecordValSlot { +public: + RecordValSlot() {} + RecordValSlot(ZVal zval, RecordValSlotFlags flags) : zval(zval), flags(flags) {} + + bool IsManaged() const noexcept { return (flags & RecordValSlotFlags::Managed) == RecordValSlotFlags::Managed; } + bool IsDeleted() const noexcept { return (flags & RecordValSlotFlags::Deleted) == RecordValSlotFlags::Deleted; } + bool IsSet() const noexcept { return (flags & RecordValSlotFlags::Set) == RecordValSlotFlags::Set; } + + void SetManaged(bool is_managed) noexcept { + if ( is_managed ) + flags = flags | detail::RecordValSlotFlags::Managed; + else + flags = flags & (~detail::RecordValSlotFlags::Managed); + } + + void Set(ZVal new_zval) noexcept { + flags = flags | RecordValSlotFlags::Set; + flags = flags & (~RecordValSlotFlags::Deleted); + zval = new_zval; + } + + void Delete() { + if ( IsSet() && IsManaged() ) + ZVal::DeleteManagedType(zval); + + flags = flags | RecordValSlotFlags::Deleted; + flags = flags & (~RecordValSlotFlags::Set); + } + + ValPtr ToVal(const TypePtr& t) const { + assert(IsSet()); + return zval.ToVal(t); + } + + auto& GetZVal() noexcept { return zval; } + const auto& GetZVal() const noexcept { return zval; } + +private: + friend class zeek::detail::CPPRuntime; + friend class zeek::RecordVal; + ZVal zval; + RecordValSlotFlags flags = RecordValSlotFlags::None; +}; + +} // namespace detail + class RecordVal final : public Val, public notifier::detail::Modifiable { public: explicit RecordVal(RecordTypePtr t, bool init_fields = true); @@ -1150,31 +1224,31 @@ public: // The following provide efficient record field assignments. void Assign(int field, bool new_val) { - record_val[field] = ZVal(zeek_int_t(new_val)); + record_val[field].Set(ZVal(zeek_int_t(new_val))); AddedField(field); } // For int types, we provide both [u]int32_t and [u]int64_t versions for // convenience, since sometimes the caller has one rather than the other. void Assign(int field, int32_t new_val) { - record_val[field] = ZVal(zeek_int_t(new_val)); + record_val[field].Set(ZVal(zeek_int_t(new_val))); AddedField(field); } void Assign(int field, int64_t new_val) { - record_val[field] = ZVal(zeek_int_t(new_val)); + record_val[field].Set(ZVal(zeek_int_t(new_val))); AddedField(field); } void Assign(int field, uint32_t new_val) { - record_val[field] = ZVal(zeek_uint_t(new_val)); + record_val[field].Set(ZVal(zeek_uint_t(new_val))); AddedField(field); } void Assign(int field, uint64_t new_val) { - record_val[field] = ZVal(zeek_uint_t(new_val)); + record_val[field].Set(ZVal(zeek_uint_t(new_val))); AddedField(field); } void Assign(int field, double new_val) { - record_val[field] = ZVal(new_val); + record_val[field].Set(ZVal(new_val)); AddedField(field); } @@ -1186,10 +1260,12 @@ public: void AssignInterval(int field, double new_val) { Assign(field, new_val); } void Assign(int field, StringVal* new_val) { - auto& fv = record_val[field]; - if ( fv ) - ZVal::DeleteManagedType(*fv); - fv = ZVal(new_val); + auto& slot = record_val[field]; + if ( slot.IsSet() ) { + assert(slot.IsManaged()); + ZVal::DeleteManagedType(slot.zval); + } + slot.Set(ZVal(new_val)); AddedField(field); } void Assign(int field, const char* new_val) { Assign(field, new StringVal(new_val)); } @@ -1222,9 +1298,13 @@ public: * @return Whether there's a value for the given field index. */ bool HasField(int field) const { - if ( record_val[field] ) + const auto& slot = record_val[field]; + if ( slot.IsSet() ) return true; + if ( slot.IsDeleted() ) + return false; + return rt->DeferredInits()[field] != nullptr; } @@ -1245,16 +1325,18 @@ public: * @return The value at the given field index. */ ValPtr GetField(int field) const { - auto& fv = record_val[field]; - if ( ! fv ) { + auto& slot = record_val[field]; + if ( slot.IsDeleted() ) + return nullptr; + + if ( ! slot.IsSet() ) { const auto& fi = rt->DeferredInits()[field]; if ( ! fi ) return nullptr; - fv = fi->Generate(); + slot.Set(fi->Generate()); } - - return fv->ToVal(rt->GetFieldType(field)); + return slot.ToVal(rt->GetFieldType(field)); } /** @@ -1321,7 +1403,10 @@ 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 { + assert((record_val[field].IsSet() && ! record_val[field].IsDeleted()) || ! record_val[field].IsSet()); + return record_val[field].IsSet(); + } // The following return the given field converted to a particular // underlying value. We provide these to enable efficient @@ -1330,33 +1415,34 @@ public: // record (using HasRawField(), if necessary). template, bool> = true> auto GetFieldAs(int field) const -> std::invoke_result_t { + const auto& slot = record_val[field]; if constexpr ( std::is_same_v || std::is_same_v || std::is_same_v ) - return record_val[field]->int_val; + return slot.zval.int_val; else if constexpr ( std::is_same_v ) - return record_val[field]->uint_val; + return slot.zval.uint_val; else if constexpr ( std::is_same_v || std::is_same_v || std::is_same_v ) - return record_val[field]->double_val; + return slot.zval.double_val; else if constexpr ( std::is_same_v ) - return val_mgr->Port(record_val[field]->uint_val); + return val_mgr->Port(slot.zval.uint_val); else if constexpr ( std::is_same_v ) - return record_val[field]->string_val->Get(); + return slot.zval.string_val->Get(); else if constexpr ( std::is_same_v ) - return record_val[field]->addr_val->Get(); + return slot.zval.addr_val->Get(); else if constexpr ( std::is_same_v ) - return record_val[field]->subnet_val->Get(); + return slot.zval.subnet_val->Get(); else if constexpr ( std::is_same_v ) - return *(record_val[field]->file_val); + return *(slot.zval.file_val); else if constexpr ( std::is_same_v ) - return *(record_val[field]->func_val); + return *(slot.zval.func_val); else if constexpr ( std::is_same_v ) - return record_val[field]->re_val->Get(); + return slot.zval.re_val->Get(); else if constexpr ( std::is_same_v ) - return record_val[field]->record_val; + return slot.zval.record_val; else if constexpr ( std::is_same_v ) - return record_val[field]->vector_val; + return slot.zval.vector_val; else if constexpr ( std::is_same_v ) - return record_val[field]->table_val->Get(); + return slot.zval.table_val->Get(); else { // It's an error to reach here, although because of // the type trait we really shouldn't ever wind up @@ -1368,11 +1454,11 @@ public: template, bool> = true> T GetFieldAs(int field) const { if constexpr ( std::is_integral_v && std::is_signed_v ) - return record_val[field]->int_val; + return record_val[field].zval.int_val; else if constexpr ( std::is_integral_v && std::is_unsigned_v ) - return record_val[field]->uint_val; + return record_val[field].zval.uint_val; else if constexpr ( std::is_floating_point_v ) - return record_val[field]->double_val; + return record_val[field].zval.double_val; // Note: we could add other types here using type traits, // such as is_same_v, etc. @@ -1448,33 +1534,31 @@ protected: * @param t The type associated with the field. */ void AppendField(ValPtr v, const TypePtr& t) { + detail::RecordValSlotFlags managed_flag = + IsManaged(record_val.size()) ? detail::RecordValSlotFlags::Managed : detail::RecordValSlotFlags::None; + if ( v ) - record_val.emplace_back(ZVal(v, t)); + record_val.emplace_back(ZVal(v, t), managed_flag | detail::RecordValSlotFlags::Set); else - record_val.emplace_back(std::nullopt); + record_val.emplace_back(ZVal(), managed_flag); } // 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& RawOptField(int field) { - auto& f = record_val[field]; - if ( ! f ) { + detail::RecordValSlot& RawOptField(int field) { + auto& slot = record_val[field]; + if ( ! slot.IsSet() && ! slot.IsDeleted() ) { const auto& fi = rt->DeferredInits()[field]; if ( fi ) - f = fi->Generate(); + slot.Set(fi->Generate()); } - return f; + return slot; } - ZVal& RawField(int field) { - auto& f = RawOptField(field); - if ( ! f ) - f = ZVal(); - return *f; - } + ZVal& RawField(int field) { return record_val[field].zval; } ValPtr DoClone(CloneState* state) override; @@ -1486,13 +1570,12 @@ protected: 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 { + bool r = GetType()->ManagedFields()[offset]; + assert(r == is_managed[offset]); + return r; } - bool IsManaged(unsigned int offset) const { return is_managed[offset]; } // Just for template inferencing. RecordVal* Get() { return this; } @@ -1505,7 +1588,7 @@ private: // Low-level values of each of the fields. // // Lazily modified during GetField(), so mutable. - mutable std::vector> record_val; + mutable std::vector record_val; // Whether a given field requires explicit memory management. const std::vector& is_managed; diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index eb22695e05..2fbd93f125 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -1583,14 +1583,16 @@ detail::LogRecord Manager::RecordToLogRecord(const Stream* stream, Filter* filte 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.GetZVal(); vt = cast_intrusive(vr->GetType())->GetFieldType(index).get(); } diff --git a/testing/btest/Baseline/language.delete-field-set/output b/testing/btest/Baseline/language.delete-field-set/output index e99c5e4bf8..85fc672ea8 100644 --- a/testing/btest/Baseline/language.delete-field-set/output +++ b/testing/btest/Baseline/language.delete-field-set/output @@ -1,6 +1,2 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -[a={ - -}, b={ - -}, c=[]] +[a=, b=, c=]