From 09dc074a261fc4545387e7615c191ad301b9b658 Mon Sep 17 00:00:00 2001 From: Vern Paxson Date: Sun, 2 May 2021 12:33:20 -0700 Subject: [PATCH] switched RecordVal's to use std::optional for tracking missing fields --- src/Val.cc | 31 ++++---------------- src/Val.h | 83 +++++++++++++++++++----------------------------------- 2 files changed, 34 insertions(+), 80 deletions(-) diff --git a/src/Val.cc b/src/Val.cc index ca2670185d..f93e5beb88 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -2846,11 +2846,9 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields) int n = rt->NumFields(); - record_val = new std::vector; + record_val = new std::vector>; record_val->reserve(n); - is_in_record = new std::vector(n, false); - if ( run_state::is_parsing ) parse_time_records[rt.get()].emplace_back(NewRef{}, this); @@ -2876,7 +2874,6 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields) parse_time_records[rt.get()].pop_back(); delete record_val; - delete is_in_record; throw; } @@ -2908,15 +2905,9 @@ RecordVal::RecordVal(RecordTypePtr t, bool init_fields) } if ( def ) - { record_val->emplace_back(ZVal(def, def->GetType())); - (*is_in_record)[i] = true; - } else - { - record_val->emplace_back(ZVal()); - (*is_in_record)[i] = false; - } + record_val->emplace_back(std::nullopt); } } @@ -2926,10 +2917,9 @@ RecordVal::~RecordVal() for ( unsigned int i = 0; i < n; ++i ) if ( HasField(i) && IsManaged(i) ) - ZVal::DeleteManagedType((*record_val)[i]); + ZVal::DeleteManagedType(*(*record_val)[i]); delete record_val; - delete is_in_record; } ValPtr RecordVal::SizeVal() const @@ -2945,7 +2935,6 @@ void RecordVal::Assign(int field, ValPtr new_val) auto t = rt->GetFieldType(field); (*record_val)[field] = ZVal(new_val, t); - (*is_in_record)[field] = true; Modified(); } else @@ -2957,10 +2946,9 @@ void RecordVal::Remove(int field) if ( HasField(field) ) { if ( IsManaged(field) ) - ZVal::DeleteManagedType((*record_val)[field]); + ZVal::DeleteManagedType(*(*record_val)[field]); - (*record_val)[field] = ZVal(); - (*is_in_record)[field] = false; + (*record_val)[field] = std::nullopt; Modified(); } @@ -2992,8 +2980,6 @@ void RecordVal::ResizeParseTimeRecords(RecordType* revised_rt) if ( required_length > current_length ) { - rv->Reserve(required_length); - for ( auto i = current_length; i < required_length; ++i ) rv->AppendField(revised_rt->FieldDefault(i)); } @@ -3203,13 +3189,6 @@ unsigned int RecordVal::MemoryAllocation() const size += util::pad_size(record_val->capacity() * sizeof(ZVal)); size += padded_sizeof(*record_val); - // It's tricky sizing is_in_record since it's a std::vector - // specialization. We approximate this by not scaling capacity() - // by sizeof(bool) but just using its raw value. That's still - // presumably going to be an overestimate. - size += util::pad_size(is_in_record->capacity()); - size += padded_sizeof(*is_in_record); - return size + padded_sizeof(*this); } diff --git a/src/Val.h b/src/Val.h index 2f3fb29e21..979460cefa 100644 --- a/src/Val.h +++ b/src/Val.h @@ -1103,13 +1103,13 @@ public: // The following provide efficient record field assignments. void Assign(int field, bool new_val) { - (*record_val)[field].int_val = int(new_val); + (*record_val)[field] = ZVal(bro_int_t(new_val)); AddedField(field); } void Assign(int field, int new_val) { - (*record_val)[field].int_val = new_val; + (*record_val)[field] = ZVal(bro_int_t(new_val)); AddedField(field); } @@ -1118,18 +1118,18 @@ public: // than the other. void Assign(int field, uint32_t new_val) { - (*record_val)[field].uint_val = new_val; + (*record_val)[field] = ZVal(bro_uint_t(new_val)); AddedField(field); } void Assign(int field, uint64_t new_val) { - (*record_val)[field].uint_val = new_val; + (*record_val)[field] = ZVal(bro_uint_t(new_val)); AddedField(field); } void Assign(int field, double new_val) { - (*record_val)[field].double_val = new_val; + (*record_val)[field] = ZVal(new_val); AddedField(field); } @@ -1144,8 +1144,9 @@ public: void Assign(int field, StringVal* new_val) { - ZVal::DeleteManagedType((*record_val)[field]); - (*record_val)[field].string_val = new_val; + if ( HasField(field) ) + ZVal::DeleteManagedType(*(*record_val)[field]); + (*record_val)[field] = ZVal(new_val); AddedField(field); } void Assign(int field, const char* new_val) @@ -1177,29 +1178,9 @@ public: void AppendField(ValPtr v) { if ( v ) - { - (*is_in_record)[record_val->size()] = true; record_val->emplace_back(ZVal(v, v->GetType())); - } else - { - (*is_in_record)[record_val->size()] = false; - record_val->emplace_back(ZVal()); - } - } - - /** - * Ensures that the record has enough internal storage for the - * given number of fields. - * @param n The number of fields. - */ - void Reserve(unsigned int n) - { - record_val->reserve(n); - is_in_record->reserve(n); - - for ( auto i = is_in_record->size(); i < n; ++i ) - is_in_record->emplace_back(false); + record_val->emplace_back(std::nullopt); } /** @@ -1217,7 +1198,7 @@ public: */ bool HasField(int field) const { - return (*is_in_record)[field]; + return (*record_val)[field] ? true : false; } /** @@ -1230,7 +1211,7 @@ public: if ( ! HasField(field) ) return nullptr; - return (*record_val)[field].ToVal(rt->GetFieldType(field)); + return (*record_val)[field]->ToVal(rt->GetFieldType(field)); } /** @@ -1304,33 +1285,33 @@ public: if constexpr ( std::is_same_v || std::is_same_v || std::is_same_v ) - return record_val->operator[](field).int_val; + return record_val->operator[](field)->int_val; else if constexpr ( std::is_same_v ) - return record_val->operator[](field).uint_val; + return record_val->operator[](field)->uint_val; else if constexpr ( std::is_same_v || std::is_same_v || std::is_same_v ) - return record_val->operator[](field).double_val; + return record_val->operator[](field)->double_val; else if constexpr ( std::is_same_v ) - return val_mgr->Port(record_val->at(field).uint_val); + return val_mgr->Port(record_val->at(field)->uint_val); else if constexpr ( std::is_same_v ) - return record_val->operator[](field).string_val->Get(); + return record_val->operator[](field)->string_val->Get(); else if constexpr ( std::is_same_v ) - return record_val->operator[](field).addr_val->Get(); + return record_val->operator[](field)->addr_val->Get(); else if constexpr ( std::is_same_v ) - return record_val->operator[](field).subnet_val->Get(); + return record_val->operator[](field)->subnet_val->Get(); else if constexpr ( std::is_same_v ) - return *(record_val->operator[](field).file_val); + return *(record_val->operator[](field)->file_val); else if constexpr ( std::is_same_v ) - return *(record_val->operator[](field).func_val); + return *(record_val->operator[](field)->func_val); else if constexpr ( std::is_same_v ) - return record_val->operator[](field).re_val->Get(); + return record_val->operator[](field)->re_val->Get(); else if constexpr ( std::is_same_v ) - return record_val->operator[](field).record_val; + return record_val->operator[](field)->record_val; else if constexpr ( std::is_same_v ) - return record_val->operator[](field).vector_val; + return record_val->operator[](field)->vector_val; else if constexpr ( std::is_same_v ) - return record_val->operator[](field).table_val->Get(); + return record_val->operator[](field)->table_val->Get(); else { // It's an error to reach here, although because of @@ -1345,12 +1326,12 @@ public: T GetFieldAs(int field) const { if constexpr ( std::is_integral_v && std::is_signed_v ) - return record_val->operator[](field).int_val; + return record_val->operator[](field)->int_val; else if constexpr ( std::is_integral_v && std::is_unsigned_v ) - return record_val->operator[](field).uint_val; + return record_val->operator[](field)->uint_val; else if constexpr ( std::is_floating_point_v ) - return record_val->operator[](field).double_val; + return record_val->operator[](field)->double_val; // Note: we could add other types here using type traits, // such as is_same_v, etc. @@ -1415,7 +1396,6 @@ protected: void AddedField(int field) { - (*is_in_record)[field] = true; Modified(); } @@ -1428,7 +1408,7 @@ private: void DeleteFieldIfManaged(unsigned int field) { if ( HasField(field) && IsManaged(field) ) - ZVal::DeleteManagedType((*record_val)[field]); + ZVal::DeleteManagedType(*(*record_val)[field]); } bool IsManaged(unsigned int offset) const @@ -1441,12 +1421,7 @@ private: RecordTypePtr rt; // Low-level values of each of the fields. - std::vector* record_val; - - // Whether a given field exists - for optional fields, and because - // Zeek does not enforce that non-optional fields are actually - // present. - std::vector* is_in_record; + std::vector>* record_val; // Whether a given field requires explicit memory management. const std::vector& is_managed;