Skip to content

Commit

Permalink
refactor(userspace/libsinsp): minor changes in state API, expose new …
Browse files Browse the repository at this point in the history
…threadinfo fields

* Optimize memory allocations for static fields
* Ground work for supporting subtable access and complex state types
* Inlining functions wherever possible
* Improve checks in static and dynamic state fields defintions
* Expose new fields in thread infos

Co-authored-by: Gianmatteo Palmieri <[email protected]>
Signed-off-by: Jason Dellaluce <[email protected]>
  • Loading branch information
2 people authored and poiana committed May 6, 2024
1 parent 00cb7b2 commit 24696e0
Show file tree
Hide file tree
Showing 9 changed files with 222 additions and 163 deletions.
28 changes: 14 additions & 14 deletions userspace/libsinsp/plugin_table_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ static inline std::string table_input_error_prefix(const sinsp_plugin* o, ss_plu
return "error in state table '" + std::string(i->name) + "' defined by plugin '" + o->name() + "': ";
}

static const libsinsp::state::static_struct::field_infos s_empty_static_infos;

// wraps instances of ss_plugin_table_input and makes them comply
// to the libsinsp::state::table state tables definition.
template <typename KeyType>
Expand Down Expand Up @@ -275,11 +277,11 @@ struct plugin_table_wrapper: public libsinsp::state::table<KeyType>
ds::field_info f;
#define _X(_type, _dtype) \
{ \
f = ds::field_info::build<_type>(res[i].name, i, this, res[i].read_only); \
f = ds::field_info::build<_type>(res[i].name, i, (uintptr_t) this, res[i].read_only); \
}
__PLUGIN_STATETYPE_SWITCH(res[i].field_type);
#undef _X
ds::field_infos::add_field(f);
ds::field_infos::add_field_info(f);
}
}

Expand Down Expand Up @@ -310,7 +312,7 @@ struct plugin_table_wrapper: public libsinsp::state::table<KeyType>
return ret;
}

virtual const ds::field_info& add_field(const ds::field_info& field) override
virtual const ds::field_info& add_field_info(const ds::field_info& field) override
{
auto ret = m_input->fields_ext->add_table_field(m_input->table, field.name().c_str(), typeinfo_to_state_type(field.info()));
if (ret == NULL)
Expand All @@ -327,7 +329,7 @@ struct plugin_table_wrapper: public libsinsp::state::table<KeyType>

// lastly, we leverage the base-class implementation to obtain
// a reference from our local field definitions copy.
return ds::field_infos::add_field(field);
return ds::field_infos::add_field_info(field);
}
};

Expand Down Expand Up @@ -456,10 +458,9 @@ struct plugin_table_wrapper: public libsinsp::state::table<KeyType>
};

plugin_table_wrapper(sinsp_plugin* o, const ss_plugin_table_input* i)
: libsinsp::state::table<KeyType>(i->name, ss::field_infos()),
: libsinsp::state::table<KeyType>(i->name, &s_empty_static_infos),
m_owner(o),
m_input(copy_and_check_table_input(o, i)),
m_static_fields(),
m_dyn_fields(std::make_shared<plugin_field_infos>(o, m_input))
{
auto t = libsinsp::state::typeinfo::of<KeyType>();
Expand All @@ -477,14 +478,13 @@ struct plugin_table_wrapper: public libsinsp::state::table<KeyType>

sinsp_plugin* m_owner;
owned_table_input_t m_input;
libsinsp::state::static_struct::field_infos m_static_fields;
std::shared_ptr<plugin_field_infos> m_dyn_fields;

const libsinsp::state::static_struct::field_infos& static_fields() const override
const libsinsp::state::static_struct::field_infos* static_fields() const override
{
// note: always empty, plugin-defined table have no "static" fields,
// all of them are dynamically-discovered at runtime
return m_static_fields;
return &s_empty_static_infos;
}

std::shared_ptr<ds::field_infos> dynamic_fields() const override
Expand Down Expand Up @@ -751,7 +751,7 @@ struct sinsp_table_wrapper

__CATCH_ERR_MSG(t->m_owner_plugin->m_last_owner_err, {
t->m_field_list.clear();
for (auto& info : t->m_table->static_fields())
for (auto& info : *t->m_table->static_fields())
{
ss_plugin_table_fieldinfo i;
i.name = info.second.name().c_str();
Expand Down Expand Up @@ -797,9 +797,9 @@ struct sinsp_table_wrapper
return static_cast<ss_plugin_table_field_t*>(&it->second);
}

fixed_it = t->m_table->static_fields().find(name);
fixed_it = t->m_table->static_fields()->find(name);
dyn_it = t->m_table->dynamic_fields()->fields().find(name);
if (fixed_it != t->m_table->static_fields().end()
if (fixed_it != t->m_table->static_fields()->end()
&& dyn_it != t->m_table->dynamic_fields()->fields().end())
{
// todo(jasondellaluce): plugins are not aware of the difference
Expand All @@ -820,7 +820,7 @@ struct sinsp_table_wrapper
return &t->m_field_accessors[name]; \
}
__CATCH_ERR_MSG(t->m_owner_plugin->m_last_owner_err, {
if (fixed_it != t->m_table->static_fields().end())
if (fixed_it != t->m_table->static_fields()->end())
{
if (data_type != typeinfo_to_state_type(fixed_it->second.info()))
{
Expand Down Expand Up @@ -872,7 +872,7 @@ struct sinsp_table_wrapper
return ret;
}

if (t->m_table->static_fields().find(name) != t->m_table->static_fields().end())
if (t->m_table->static_fields()->find(name) != t->m_table->static_fields()->end())
{
t->m_owner_plugin->m_last_owner_err = "can't add dynamic field already defined as static: " + std::string(name);
return NULL;
Expand Down
95 changes: 55 additions & 40 deletions userspace/libsinsp/state/dynamic_struct.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,46 +43,54 @@ class dynamic_struct
{
public:
template<typename T>
static field_info build(const std::string& name, size_t index, void* defsptr, bool readonly=false)
static inline field_info build(const std::string& name, size_t index, uintptr_t defsptr, bool readonly=false)
{
return field_info(name, index, libsinsp::state::typeinfo::of<T>(), defsptr, readonly);
}

field_info(const std::string& n, size_t in, const typeinfo& i, void* defsptr, bool r)
inline field_info(const std::string& n, size_t in, const typeinfo& i, uintptr_t defsptr, bool r)
: m_readonly(r),
m_index(in),
m_name(n),
m_info(i),
m_defsptr(defsptr) {}
field_info():
m_defs_id(defsptr) {}
inline field_info():
m_readonly(true),
m_index((size_t) -1),
m_name(""),
m_info(typeinfo::of<uint8_t>()),
m_defsptr(NULL) {}
~field_info() = default;
field_info(field_info&&) = default;
field_info& operator = (field_info&&) = default;
field_info(const field_info& s) = default;
field_info& operator = (const field_info& s) = default;
m_defs_id((uintptr_t) NULL) {}
inline ~field_info() = default;
inline field_info(field_info&&) = default;
inline field_info& operator = (field_info&&) = default;
inline field_info(const field_info& s) = default;
inline field_info& operator = (const field_info& s) = default;

friend inline bool operator==(const field_info& a, const field_info& b)
{
return a.info() == b.info()
&& a.name() == b.name()
&& a.m_index == b.m_index
&& a.m_defsptr == b.m_defsptr;
&& a.m_defs_id == b.m_defs_id;
};

friend inline bool operator!=(const field_info& a, const field_info& b)
{
return !(a == b);
};

/**
* @brief Returns the id of the shared definitions this info belongs to.
*/
inline uintptr_t defs_id() const
{
return m_defs_id;
}

/**
* @brief Returns true if the field is read only.
*/
bool readonly() const
inline bool readonly() const
{
return m_readonly;
}
Expand All @@ -98,23 +106,23 @@ class dynamic_struct
/**
* @brief Returns the name of the field.
*/
const std::string& name() const
inline const std::string& name() const
{
return m_name;
}

/**
* @brief Returns the index of the field.
*/
size_t index() const
inline size_t index() const
{
return m_index;
}

/**
* @brief Returns the type info of the field.
*/
const libsinsp::state::typeinfo& info() const
inline const libsinsp::state::typeinfo& info() const
{
return m_info;
}
Expand All @@ -125,7 +133,7 @@ class dynamic_struct
* all instances of structs where it is defined.
*/
template <typename T>
field_accessor<T> new_accessor() const
inline field_accessor<T> new_accessor() const
{
if (!valid())
{
Expand All @@ -146,7 +154,7 @@ class dynamic_struct
size_t m_index;
std::string m_name;
libsinsp::state::typeinfo m_info;
void* m_defsptr;
uintptr_t m_defs_id;

friend class dynamic_struct;
};
Expand All @@ -159,23 +167,23 @@ class dynamic_struct
class field_accessor
{
public:
field_accessor() = default;
~field_accessor() = default;
field_accessor(field_accessor&&) = default;
field_accessor& operator = (field_accessor&&) = default;
field_accessor(const field_accessor& s) = default;
field_accessor& operator = (const field_accessor& s) = default;
inline field_accessor() = default;
inline ~field_accessor() = default;
inline field_accessor(field_accessor&&) = default;
inline field_accessor& operator = (field_accessor&&) = default;
inline field_accessor(const field_accessor& s) = default;
inline field_accessor& operator = (const field_accessor& s) = default;

/**
* @brief Returns the info about the field to which this accessor is tied.
*/
const field_info& info() const
inline const field_info& info() const
{
return m_info;
}

private:
field_accessor(const field_info& info): m_info(info) { };
inline explicit field_accessor(const field_info& info): m_info(info) { };

field_info m_info;

Expand All @@ -192,12 +200,18 @@ class dynamic_struct
class field_infos
{
public:
field_infos() = default;
inline field_infos(): m_defs_id((uintptr_t) this) { };
inline explicit field_infos(uintptr_t defs_id): m_defs_id(defs_id) { };
virtual ~field_infos() = default;
field_infos(field_infos&&) = default;
field_infos& operator = (field_infos&&) = default;
field_infos(const field_infos& s) = delete;
field_infos& operator = (const field_infos& s) = delete;
inline field_infos(field_infos&&) = default;
inline field_infos& operator = (field_infos&&) = default;
inline field_infos(const field_infos& s) = delete;
inline field_infos& operator = (const field_infos& s) = delete;

inline uintptr_t id() const
{
return m_defs_id;
}

/**
* @brief Adds metadata for a new field to the list. An exception is
Expand All @@ -210,8 +224,8 @@ class dynamic_struct
template<typename T>
inline const field_info& add_field(const std::string& name)
{
auto field = field_info::build<T>(name, m_definitions.size(), this);
return add_field(field);
auto field = field_info::build<T>(name, m_definitions.size(), id());
return add_field_info(field);
}

virtual const std::unordered_map<std::string, field_info>& fields()
Expand All @@ -220,7 +234,7 @@ class dynamic_struct
}

protected:
virtual const field_info& add_field(const field_info& field)
virtual const field_info& add_field_info(const field_info& field)
{
const auto &it = m_definitions.find(field.name());
if (it != m_definitions.end())
Expand All @@ -239,17 +253,18 @@ class dynamic_struct
return def;
}

uintptr_t m_defs_id;
std::unordered_map<std::string, field_info> m_definitions;
std::vector<const field_info*> m_definitions_ordered;
friend class dynamic_struct;
};

dynamic_struct(const std::shared_ptr<field_infos>& dynamic_fields)
inline explicit dynamic_struct(const std::shared_ptr<field_infos>& dynamic_fields)
: m_fields_len(0), m_fields(), m_dynamic_fields(dynamic_fields) { }
dynamic_struct(dynamic_struct&&) = default;
dynamic_struct& operator = (dynamic_struct&&) = default;
dynamic_struct(const dynamic_struct& s) = default;
dynamic_struct& operator = (const dynamic_struct& s) = default;
inline dynamic_struct(dynamic_struct&&) = default;
inline dynamic_struct& operator = (dynamic_struct&&) = default;
inline dynamic_struct(const dynamic_struct& s) = default;
inline dynamic_struct& operator = (const dynamic_struct& s) = default;
virtual ~dynamic_struct()
{
if (m_dynamic_fields)
Expand Down Expand Up @@ -301,7 +316,7 @@ class dynamic_struct
*/
virtual void set_dynamic_fields(const std::shared_ptr<field_infos>& defs)
{
if (m_dynamic_fields)
if (m_dynamic_fields && m_dynamic_fields.use_count() > 1)
{
throw sinsp_exception("dynamic struct defintions set twice");
}
Expand Down Expand Up @@ -358,7 +373,7 @@ class dynamic_struct
{
throw sinsp_exception("can't set invalid field in dynamic struct");
}
if (m_dynamic_fields.get() != i.m_defsptr)
if (m_dynamic_fields->id() != i.m_defs_id)
{
throw sinsp_exception("using dynamic field accessor on struct it was not created from");
}
Expand Down
Loading

0 comments on commit 24696e0

Please sign in to comment.