VectorVal: Embed vector_val

Similar motivation as for RecordVal, save an extra malloc/free
and pointer indirection.

This breaks the `auto& RawVec()` API which previously returned
a reference to the std::vector*. It now returns a reference
to the vector instead. It's commented as intended for internal
and compiled code, so even though it's public API,

The previous `std::vector<std::optional<ZVal>>*&` return type was also very
likely not intended (all consumers just dereference it anyhow). I'm certain
this API was never meant to modify the actual pointer value.

I've switched to explicit typing, too.
This commit is contained in:
Arne Welzel 2023-09-21 15:06:50 +02:00
parent f362935a66
commit cbaf43e8ea
7 changed files with 70 additions and 81 deletions

View file

@ -314,13 +314,13 @@ void ValTrace::TraceTable(const TableValPtr& tv)
void ValTrace::TraceVector(const VectorValPtr& vv) void ValTrace::TraceVector(const VectorValPtr& vv)
{ {
auto& vec = vv->RawVec(); auto& vec = vv->RawVec();
auto n = vec->size(); auto n = vec.size();
auto& yt = vv->RawYieldType(); auto& yt = vv->RawYieldType();
auto& yts = vv->RawYieldTypes(); auto& yts = vv->RawYieldTypes();
for ( auto i = 0U; i < n; ++i ) for ( auto i = 0U; i < n; ++i )
{ {
auto& elem = (*vec)[i]; auto& elem = vec[i];
if ( elem ) if ( elem )
{ {
auto& t = yts ? (*yts)[i] : yt; auto& t = yts ? (*yts)[i] : yt;

View file

@ -1411,7 +1411,7 @@ ValPtr ForStmt::DoExec(Frame* f, Val* v, StmtFlowType& flow)
else if ( v->GetType()->Tag() == TYPE_VECTOR ) else if ( v->GetType()->Tag() == TYPE_VECTOR )
{ {
VectorVal* vv = v->AsVectorVal(); VectorVal* vv = v->AsVectorVal();
const auto& raw_vv = *vv->RawVec(); const auto& raw_vv = vv->RawVec();
for ( auto i = 0u; i < vv->Size(); ++i ) for ( auto i = 0u; i < vv->Size(); ++i )
{ {

View file

@ -3487,11 +3487,8 @@ ValPtr TypeVal::DoClone(CloneState* state)
return {NewRef{}, this}; return {NewRef{}, this};
} }
VectorVal::VectorVal(VectorTypePtr t) : VectorVal(t, new vector<std::optional<ZVal>>()) { } VectorVal::VectorVal(VectorTypePtr t) : Val(t)
VectorVal::VectorVal(VectorTypePtr t, std::vector<std::optional<ZVal>>* vals) : Val(t)
{ {
vector_val = vals;
yield_type = t->Yield(); yield_type = t->Yield();
auto y_tag = yield_type->Tag(); auto y_tag = yield_type->Tag();
@ -3499,6 +3496,12 @@ VectorVal::VectorVal(VectorTypePtr t, std::vector<std::optional<ZVal>>* vals) :
managed_yield = ZVal::IsManagedType(yield_type); managed_yield = ZVal::IsManagedType(yield_type);
} }
VectorVal::VectorVal(VectorTypePtr t, std::vector<std::optional<ZVal>>* vals) : VectorVal(t)
{
if ( vals )
vector_val = std::move(*vals);
}
VectorVal::~VectorVal() VectorVal::~VectorVal()
{ {
if ( yield_types ) if ( yield_types )
@ -3506,7 +3509,7 @@ VectorVal::~VectorVal()
int n = yield_types->size(); int n = yield_types->size();
for ( auto i = 0; i < n; ++i ) for ( auto i = 0; i < n; ++i )
{ {
auto& elem = (*vector_val)[i]; auto& elem = vector_val[i];
if ( elem ) if ( elem )
ZVal::DeleteIfManaged(*elem, (*yield_types)[i]); ZVal::DeleteIfManaged(*elem, (*yield_types)[i]);
} }
@ -3515,17 +3518,15 @@ VectorVal::~VectorVal()
else if ( managed_yield ) else if ( managed_yield )
{ {
for ( auto& elem : *vector_val ) for ( auto& elem : vector_val )
if ( elem ) if ( elem )
ZVal::DeleteManagedType(*elem); ZVal::DeleteManagedType(*elem);
} }
delete vector_val;
} }
ValPtr VectorVal::SizeVal() const ValPtr VectorVal::SizeVal() const
{ {
return val_mgr->Count(uint32_t(vector_val->size())); return val_mgr->Count(uint32_t(vector_val.size()));
} }
bool VectorVal::CheckElementType(const ValPtr& element) bool VectorVal::CheckElementType(const ValPtr& element)
@ -3540,7 +3541,7 @@ bool VectorVal::CheckElementType(const ValPtr& element)
if ( any_yield ) if ( any_yield )
{ {
int n = vector_val->size(); int n = vector_val.size();
if ( n == 0 ) if ( n == 0 )
{ {
@ -3574,14 +3575,14 @@ bool VectorVal::Assign(unsigned int index, ValPtr element)
if ( ! CheckElementType(element) ) if ( ! CheckElementType(element) )
return false; return false;
unsigned int n = vector_val->size(); unsigned int n = vector_val.size();
if ( index >= n ) if ( index >= n )
{ {
if ( index > n ) if ( index > n )
AddHoles(index - n); AddHoles(index - n);
vector_val->resize(index + 1); vector_val.resize(index + 1);
if ( yield_types ) if ( yield_types )
yield_types->resize(index + 1); yield_types->resize(index + 1);
} }
@ -3590,14 +3591,14 @@ bool VectorVal::Assign(unsigned int index, ValPtr element)
{ {
const auto& t = element->GetType(); const auto& t = element->GetType();
(*yield_types)[index] = t; (*yield_types)[index] = t;
auto& elem = (*vector_val)[index]; auto& elem = vector_val[index];
if ( elem ) if ( elem )
ZVal::DeleteIfManaged(*elem, t); ZVal::DeleteIfManaged(*elem, t);
elem = ZVal(std::move(element), t); elem = ZVal(std::move(element), t);
} }
else else
{ {
auto& elem = (*vector_val)[index]; auto& elem = vector_val[index];
if ( managed_yield && elem ) if ( managed_yield && elem )
ZVal::DeleteManagedType(*elem); ZVal::DeleteManagedType(*elem);
@ -3630,17 +3631,17 @@ bool VectorVal::Insert(unsigned int index, ValPtr element)
vector<std::optional<ZVal>>::iterator it; vector<std::optional<ZVal>>::iterator it;
vector<TypePtr>::iterator types_it; vector<TypePtr>::iterator types_it;
auto n = vector_val->size(); auto n = vector_val.size();
if ( index < n ) if ( index < n )
{ // Find location within existing vector elements. { // Find location within existing vector elements.
it = std::next(vector_val->begin(), index); it = std::next(vector_val.begin(), index);
if ( yield_types ) if ( yield_types )
types_it = std::next(yield_types->begin(), index); types_it = std::next(yield_types->begin(), index);
} }
else else
{ {
it = vector_val->end(); it = vector_val.end();
if ( yield_types ) if ( yield_types )
types_it = yield_types->end(); types_it = yield_types->end();
@ -3654,13 +3655,13 @@ bool VectorVal::Insert(unsigned int index, ValPtr element)
{ {
const auto& t = element->GetType(); const auto& t = element->GetType();
yield_types->insert(types_it, t); yield_types->insert(types_it, t);
vector_val->insert(it, ZVal(std::move(element), t)); vector_val.insert(it, ZVal(std::move(element), t));
} }
else else
vector_val->insert(it, ZVal(std::move(element), yield_type)); vector_val.insert(it, ZVal(std::move(element), yield_type));
} }
else else
vector_val->insert(it, std::nullopt); vector_val.insert(it, std::nullopt);
Modified(); Modified();
return true; return true;
@ -3673,15 +3674,15 @@ void VectorVal::AddHoles(int nholes)
fill_t = base_type(TYPE_ANY); fill_t = base_type(TYPE_ANY);
for ( auto i = 0; i < nholes; ++i ) for ( auto i = 0; i < nholes; ++i )
vector_val->emplace_back(std::nullopt); vector_val.emplace_back(std::nullopt);
} }
bool VectorVal::Remove(unsigned int index) bool VectorVal::Remove(unsigned int index)
{ {
if ( index >= vector_val->size() ) if ( index >= vector_val.size() )
return false; return false;
auto it = std::next(vector_val->begin(), index); auto it = std::next(vector_val.begin(), index);
if ( yield_types ) if ( yield_types )
{ {
@ -3697,7 +3698,7 @@ bool VectorVal::Remove(unsigned int index)
ZVal::DeleteManagedType(**it); ZVal::DeleteManagedType(**it);
} }
vector_val->erase(it); vector_val.erase(it);
Modified(); Modified();
return true; return true;
@ -3730,10 +3731,10 @@ bool VectorVal::AddTo(Val* val, bool /* is_first_init */) const
ValPtr VectorVal::At(unsigned int index) const ValPtr VectorVal::At(unsigned int index) const
{ {
if ( index >= vector_val->size() ) if ( index >= vector_val.size() )
return Val::nil; return Val::nil;
auto& elem = (*vector_val)[index]; auto& elem = vector_val[index];
if ( ! elem ) if ( ! elem )
return Val::nil; return Val::nil;
@ -3850,7 +3851,7 @@ void VectorVal::Sort(Func* cmp_func)
} }
} }
sort(vector_val->begin(), vector_val->end(), sort_func); sort(vector_val.begin(), vector_val.end(), sort_func);
} }
VectorValPtr VectorVal::Order(Func* cmp_func) VectorValPtr VectorVal::Order(Func* cmp_func)
@ -3895,7 +3896,7 @@ VectorValPtr VectorVal::Order(Func* cmp_func)
for ( i = 0; i < n; ++i ) for ( i = 0; i < n; ++i )
{ {
ind_vv[i] = i; ind_vv[i] = i;
index_map.emplace_back(&(*vector_val)[i]); index_map.emplace_back(&vector_val[i]);
} }
sort(ind_vv.begin(), ind_vv.end(), sort_func); sort(ind_vv.begin(), ind_vv.end(), sort_func);
@ -3920,14 +3921,14 @@ bool VectorVal::Concretize(const TypePtr& t)
// shouldn't happen in any case. // shouldn't happen in any case.
return yield_type->Tag() == t->Tag(); return yield_type->Tag() == t->Tag();
if ( ! vector_val ) if ( vector_val.empty() )
// Trivially concretized. // Trivially concretized.
return true; return true;
auto n = vector_val->size(); auto n = vector_val.size();
for ( auto i = 0U; i < n; ++i ) for ( auto i = 0U; i < n; ++i )
{ {
auto& v = (*vector_val)[i]; auto& v = vector_val[i];
if ( ! v ) if ( ! v )
// Vector hole does not require concretization. // Vector hole does not require concretization.
continue; continue;
@ -3960,7 +3961,7 @@ bool VectorVal::Concretize(const TypePtr& t)
unsigned int VectorVal::ComputeFootprint(std::unordered_set<const Val*>* analyzed_vals) const unsigned int VectorVal::ComputeFootprint(std::unordered_set<const Val*>* analyzed_vals) const
{ {
auto n = vector_val->size(); auto n = vector_val.size();
unsigned int fp = n; unsigned int fp = n;
for ( auto i = 0U; i < n; ++i ) for ( auto i = 0U; i < n; ++i )
@ -3975,9 +3976,9 @@ unsigned int VectorVal::ComputeFootprint(std::unordered_set<const Val*>* analyze
unsigned int VectorVal::Resize(unsigned int new_num_elements) unsigned int VectorVal::Resize(unsigned int new_num_elements)
{ {
unsigned int oldsize = vector_val->size(); unsigned int oldsize = vector_val.size();
vector_val->reserve(new_num_elements); vector_val.reserve(new_num_elements);
vector_val->resize(new_num_elements); vector_val.resize(new_num_elements);
if ( yield_types ) if ( yield_types )
{ {
@ -3990,7 +3991,7 @@ unsigned int VectorVal::Resize(unsigned int new_num_elements)
unsigned int VectorVal::ResizeAtLeast(unsigned int new_num_elements) unsigned int VectorVal::ResizeAtLeast(unsigned int new_num_elements)
{ {
unsigned int old_size = vector_val->size(); unsigned int old_size = vector_val.size();
if ( new_num_elements <= old_size ) if ( new_num_elements <= old_size )
return old_size; return old_size;
@ -3999,7 +4000,7 @@ unsigned int VectorVal::ResizeAtLeast(unsigned int new_num_elements)
void VectorVal::Reserve(unsigned int num_elements) void VectorVal::Reserve(unsigned int num_elements)
{ {
vector_val->reserve(num_elements); vector_val.reserve(num_elements);
if ( yield_types ) if ( yield_types )
yield_types->reserve(num_elements); yield_types->reserve(num_elements);
@ -4008,10 +4009,10 @@ void VectorVal::Reserve(unsigned int num_elements)
ValPtr VectorVal::DoClone(CloneState* state) ValPtr VectorVal::DoClone(CloneState* state)
{ {
auto vv = make_intrusive<VectorVal>(GetType<VectorType>()); auto vv = make_intrusive<VectorVal>(GetType<VectorType>());
vv->Reserve(vector_val->size()); vv->Reserve(vector_val.size());
state->NewClone(this, vv); state->NewClone(this, vv);
int n = vector_val->size(); int n = vector_val.size();
for ( auto i = 0; i < n; ++i ) for ( auto i = 0; i < n; ++i )
{ {
@ -4026,7 +4027,7 @@ void VectorVal::ValDescribe(ODesc* d) const
{ {
d->Add("["); d->Add("[");
size_t vector_size = vector_val->size(); size_t vector_size = vector_val.size();
if ( vector_size != 0 ) if ( vector_size != 0 )
{ {

View file

@ -1592,7 +1592,7 @@ public:
// Returns true if successful. // Returns true if successful.
bool AddTo(Val* v, bool is_first_init) const override; bool AddTo(Val* v, bool is_first_init) const override;
unsigned int Size() const { return vector_val->size(); } unsigned int Size() const { return vector_val.size(); }
// Is there any way to reclaim previously-allocated memory when you // Is there any way to reclaim previously-allocated memory when you
// shrink a vector? The return value is the old size. // shrink a vector? The return value is the old size.
@ -1662,10 +1662,7 @@ public:
ValPtr ValAt(unsigned int index) const { return At(index); } ValPtr ValAt(unsigned int index) const { return At(index); }
bool Has(unsigned int index) const bool Has(unsigned int index) const { return index < vector_val.size() && vector_val[index]; }
{
return index < vector_val->size() && (*vector_val)[index];
}
/** /**
* Returns the given element in a given underlying representation. * Returns the given element in a given underlying representation.
@ -1675,26 +1672,17 @@ public:
* @param index The position in the vector of the element to return. * @param index The position in the vector of the element to return.
* @return The element's underlying value. * @return The element's underlying value.
*/ */
zeek_int_t IntAt(unsigned int index) const { return (*vector_val)[index]->int_val; } zeek_int_t IntAt(unsigned int index) const { return vector_val[index]->int_val; }
zeek_uint_t CountAt(unsigned int index) const { return (*vector_val)[index]->uint_val; } zeek_uint_t CountAt(unsigned int index) const { return vector_val[index]->uint_val; }
double DoubleAt(unsigned int index) const { return (*vector_val)[index]->double_val; } double DoubleAt(unsigned int index) const { return vector_val[index]->double_val; }
const RecordVal* RecordValAt(unsigned int index) const const RecordVal* RecordValAt(unsigned int index) const { return vector_val[index]->record_val; }
{ bool BoolAt(unsigned int index) const { return static_cast<bool>(vector_val[index]->uint_val); }
return (*vector_val)[index]->record_val; const StringVal* StringValAt(unsigned int index) const { return vector_val[index]->string_val; }
}
bool BoolAt(unsigned int index) const
{
return static_cast<bool>((*vector_val)[index]->uint_val);
}
const StringVal* StringValAt(unsigned int index) const
{
return (*vector_val)[index]->string_val;
}
const String* StringAt(unsigned int index) const { return StringValAt(index)->AsString(); } const String* StringAt(unsigned int index) const { return StringValAt(index)->AsString(); }
// Only intended for low-level access by internal or compiled code. // Only intended for low-level access by internal or compiled code.
const auto& RawVec() const { return vector_val; } const std::vector<std::optional<ZVal>>& RawVec() const { return vector_val; }
auto& RawVec() { return vector_val; } std::vector<std::optional<ZVal>>& RawVec() { return vector_val; }
const auto& RawYieldType() const { return yield_type; } const auto& RawYieldType() const { return yield_type; }
const auto& RawYieldTypes() const { return yield_types; } const auto& RawYieldTypes() const { return yield_types; }
@ -1730,7 +1718,7 @@ private:
// Add the given number of "holes" to the end of a vector. // Add the given number of "holes" to the end of a vector.
void AddHoles(int nholes); void AddHoles(int nholes);
std::vector<std::optional<ZVal>>* vector_val; std::vector<std::optional<ZVal>> vector_val;
// For homogeneous vectors (the usual case), the type of the // For homogeneous vectors (the usual case), the type of the
// elements. Will be TYPE_VOID for empty vectors created using // elements. Will be TYPE_VOID for empty vectors created using

View file

@ -1113,7 +1113,7 @@ threading::Value* Manager::ValToLogVal(std::optional<ZVal>& val, Type* ty)
for ( zeek_int_t i = 0; i < lval->val.vector_val.size; i++ ) for ( zeek_int_t i = 0; i < lval->val.vector_val.size; i++ )
{ {
lval->val.vector_val.vals[i] = ValToLogVal((*vv)[i], vt.get()); lval->val.vector_val.vals[i] = ValToLogVal(vv[i], vt.get());
} }
break; break;

View file

@ -364,7 +364,7 @@ eval auto vv = frame[z.v1].vector_val;
vv->Assign(0, $1.ToVal(z.t)); vv->Assign(0, $1.ToVal(z.t));
else else
{ {
vv->RawVec()->push_back(CopyVal($1)); vv->RawVec().push_back(CopyVal($1));
vv->Modified(); vv->Modified();
} }
@ -876,9 +876,9 @@ eval AssignV1(frame[z.v2].int_val ? CopyVal(z.c) : CopyVal(frame[z.v3]))
op Bool-Vec-Cond op Bool-Vec-Cond
type VVVV type VVVV
set-type $2 set-type $2
eval auto& vsel = *frame[z.v2].vector_val->RawVec(); eval auto& vsel = frame[z.v2].vector_val->RawVec();
auto& v1 = *frame[z.v3].vector_val->RawVec(); auto& v1 = frame[z.v3].vector_val->RawVec();
auto& v2 = *frame[z.v4].vector_val->RawVec(); auto& v2 = frame[z.v4].vector_val->RawVec();
auto n = v1.size(); auto n = v1.size();
auto res = new vector<std::optional<ZVal>>(n); auto res = new vector<std::optional<ZVal>>(n);
for ( auto i = 0U; i < n; ++i ) for ( auto i = 0U; i < n; ++i )
@ -968,11 +968,11 @@ eval EvalIndexVec(frame[z.v3].uint_val)
macro EvalIndexVec(index) macro EvalIndexVec(index)
auto& vv = frame[z.v2].vector_val->RawVec(); auto& vv = frame[z.v2].vector_val->RawVec();
const auto& vec = *vv; const auto& vec = vv;
zeek_int_t ind = index; zeek_int_t ind = index;
if ( ind < 0 ) if ( ind < 0 )
ind += vv->size(); ind += vv.size();
if ( ind < 0 || ind >= vv->size() ) if ( ind < 0 || ind >= vv.size() )
ZAM_run_time_error(z.loc, "no such index"); ZAM_run_time_error(z.loc, "no such index");
AssignV1(CopyVal(*vec[ind])) AssignV1(CopyVal(*vec[ind]))
@ -1847,7 +1847,7 @@ internal-op Init-Vector-Loop
type VV type VV
op1-read op1-read
eval auto& vv = frame[z.v1].vector_val->RawVec(); eval auto& vv = frame[z.v1].vector_val->RawVec();
step_iters[z.v2].InitLoop(vv); step_iters[z.v2].InitLoop(&vv);
macro NextVectorIterCore(info, branch) macro NextVectorIterCore(info, branch)
auto& si = step_iters[info]; auto& si = step_iters[info];

View file

@ -60,7 +60,7 @@ static bool copy_vec_elem(VectorVal* vv, zeek_uint_t ind, ZVal zv, const TypePtr
if ( vv->Size() <= ind ) if ( vv->Size() <= ind )
vv->Resize(ind + 1); vv->Resize(ind + 1);
auto& elem = (*vv->RawVec())[ind]; auto& elem = vv->RawVec()[ind];
if ( ! ZVal::IsManagedType(t) ) if ( ! ZVal::IsManagedType(t) )
{ {
@ -96,12 +96,12 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con
#define VEC_COERCE(tag, lhs_type, cast, rhs_accessor, ov_check, ov_err) \ #define VEC_COERCE(tag, lhs_type, cast, rhs_accessor, ov_check, ov_err) \
static VectorVal* vec_coerce_##tag(VectorVal* vec, const ZInst& z) \ static VectorVal* vec_coerce_##tag(VectorVal* vec, const ZInst& z) \
{ \ { \
auto& v = *vec->RawVec(); \ auto& v = vec->RawVec(); \
auto yt = make_intrusive<VectorType>(base_type(lhs_type)); \ auto yt = make_intrusive<VectorType>(base_type(lhs_type)); \
auto res_zv = new VectorVal(yt); \ auto res_zv = new VectorVal(yt); \
auto n = v.size(); \ auto n = v.size(); \
res_zv->Resize(n); \ res_zv->Resize(n); \
auto& res = *res_zv->RawVec(); \ auto& res = res_zv->RawVec(); \
for ( auto i = 0U; i < n; ++i ) \ for ( auto i = 0U; i < n; ++i ) \
if ( v[i] ) \ if ( v[i] ) \
{ \ { \
@ -479,7 +479,7 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con
// well move the whole kit-and-caboodle into the Exec method). But // well move the whole kit-and-caboodle into the Exec method). But
// that seems like a lot of code bloat for only a very modest gain. // that seems like a lot of code bloat for only a very modest gain.
auto& vec2 = *v2->RawVec(); auto& vec2 = v2->RawVec();
auto n = vec2.size(); auto n = vec2.size();
auto vec1_ptr = new vector<std::optional<ZVal>>(n); auto vec1_ptr = new vector<std::optional<ZVal>>(n);
auto& vec1 = *vec1_ptr; auto& vec1 = *vec1_ptr;
@ -511,8 +511,8 @@ static void vec_exec(ZOp op, TypePtr t, VectorVal*& v1, const VectorVal* v2, con
{ {
// See comment above re further speed-up. // See comment above re further speed-up.
auto& vec2 = *v2->RawVec(); auto& vec2 = v2->RawVec();
auto& vec3 = *v3->RawVec(); auto& vec3 = v3->RawVec();
auto n = vec2.size(); auto n = vec2.size();
auto vec1_ptr = new vector<std::optional<ZVal>>(n); auto vec1_ptr = new vector<std::optional<ZVal>>(n);
auto& vec1 = *vec1_ptr; auto& vec1 = *vec1_ptr;