Merge remote-tracking branch 'origin/topic/timw/enum-val-lookup-speedup'

* origin/topic/timw/enum-val-lookup-speedup:
  Some minor c++ modernization in EnumType methods
  Avoid O(n) search in EnumType::Lookup
This commit is contained in:
Tim Wojtulewicz 2025-04-02 16:36:00 -07:00
commit 8d71420d09
4 changed files with 28 additions and 35 deletions

View file

@ -1,3 +1,9 @@
7.2.0-dev.469 | 2025-04-02 16:36:00 -0700
* Some minor c++ modernization in EnumType methods (Tim Wojtulewicz, Corelight)
* Avoid O(n) search in EnumType::Lookup (Tim Wojtulewicz, Corelight)
7.2.0-dev.466 | 2025-04-02 18:49:50 +0200 7.2.0-dev.466 | 2025-04-02 18:49:50 +0200
* Spicy: Rework code for converting Spicy values to Zeek values. (Robin Sommer, Corelight) * Spicy: Rework code for converting Spicy values to Zeek values. (Robin Sommer, Corelight)

View file

@ -1 +1 @@
7.2.0-dev.466 7.2.0-dev.469

View file

@ -1498,7 +1498,7 @@ EnumType::EnumType(const string& name) : Type(TYPE_ENUM) {
SetName(name); SetName(name);
} }
EnumType::EnumType(const EnumType* e) : Type(TYPE_ENUM), names(e->names), vals(e->vals) { EnumType::EnumType(const EnumType* e) : Type(TYPE_ENUM), names(e->names), rev_names(e->rev_names), vals(e->vals) {
counter = e->counter; counter = e->counter;
SetName(e->GetName()); SetName(e->GetName());
} }
@ -1601,10 +1601,12 @@ void EnumType::CheckAndAddName(const string& module_name, const char* name, zeek
void EnumType::AddNameInternal(const string& module_name, const char* name, zeek_int_t val, bool is_export) { void EnumType::AddNameInternal(const string& module_name, const char* name, zeek_int_t val, bool is_export) {
string fullname = detail::make_full_var_name(module_name.c_str(), name); string fullname = detail::make_full_var_name(module_name.c_str(), name);
names[fullname] = val; names[fullname] = val;
rev_names[val] = fullname;
} }
void EnumType::AddNameInternal(const string& full_name, zeek_int_t val) { void EnumType::AddNameInternal(const string& full_name, zeek_int_t val) {
names[full_name] = val; names[full_name] = val;
rev_names[val] = full_name;
if ( vals.find(val) == vals.end() ) if ( vals.find(val) == vals.end() )
vals[val] = make_intrusive<EnumVal>(IntrusivePtr{NewRef{}, this}, val); vals[val] = make_intrusive<EnumVal>(IntrusivePtr{NewRef{}, this}, val);
@ -1615,39 +1617,33 @@ zeek_int_t EnumType::Lookup(const string& module_name, const char* name) const {
} }
zeek_int_t EnumType::Lookup(const string& full_name) const { zeek_int_t EnumType::Lookup(const string& full_name) const {
NameMap::const_iterator pos = names.find(full_name.c_str()); if ( auto pos = names.find(full_name.c_str()); pos != names.end() )
if ( pos == names.end() )
return -1;
else
return pos->second; return pos->second;
return -1;
} }
const char* EnumType::Lookup(zeek_int_t value) const { const char* EnumType::Lookup(zeek_int_t value) const {
for ( NameMap::const_iterator iter = names.begin(); iter != names.end(); ++iter ) if ( auto it = rev_names.find(value); it != rev_names.end() )
if ( iter->second == value ) return it->second.c_str();
return iter->first.c_str();
return nullptr; return nullptr;
} }
EnumType::enum_name_list EnumType::Names() const { EnumType::enum_name_list EnumType::Names() const {
enum_name_list n; enum_name_list n;
for ( NameMap::const_iterator iter = names.begin(); iter != names.end(); ++iter ) for ( auto iter = names.begin(); iter != names.end(); ++iter )
n.emplace_back(iter->first, iter->second); n.emplace_back(iter->first, iter->second);
return n; return n;
} }
const EnumValPtr& EnumType::GetEnumVal(zeek_int_t i) { const EnumValPtr& EnumType::GetEnumVal(zeek_int_t i) {
auto it = vals.find(i); if ( auto it = vals.find(i); it != vals.end() )
return it->second;
if ( it == vals.end() ) { auto ev = make_intrusive<EnumVal>(IntrusivePtr{NewRef{}, this}, i);
auto ev = make_intrusive<EnumVal>(IntrusivePtr{NewRef{}, this}, i); return vals.emplace(i, std::move(ev)).first->second;
return vals.emplace(i, std::move(ev)).first->second;
}
return it->second;
} }
void EnumType::DoDescribe(ODesc* d) const { void EnumType::DoDescribe(ODesc* d) const {
@ -1670,27 +1666,19 @@ void EnumType::DoDescribe(ODesc* d) const {
void EnumType::DescribeReST(ODesc* d, bool roles_only) const { void EnumType::DescribeReST(ODesc* d, bool roles_only) const {
d->Add(":zeek:type:`enum`"); d->Add(":zeek:type:`enum`");
// Create temporary, reverse name map so that enums can be documented for ( const auto& [_, enum_name] : rev_names ) {
// in ascending order of their actual integral value instead of by name.
using RevNameMap = std::map<zeek_int_t, std::string>;
RevNameMap rev;
for ( NameMap::const_iterator it = names.begin(); it != names.end(); ++it )
rev[it->second] = it->first;
for ( RevNameMap::const_iterator it = rev.begin(); it != rev.end(); ++it ) {
d->NL(); d->NL();
d->PushIndent(); d->PushIndent();
if ( roles_only ) if ( roles_only )
d->Add(util::fmt(":zeek:enum:`%s`", it->second.c_str())); d->Add(util::fmt(":zeek:enum:`%s`", enum_name.c_str()));
else else
d->Add(util::fmt(".. zeek:enum:: %s %s", it->second.c_str(), GetName().c_str())); d->Add(util::fmt(".. zeek:enum:: %s %s", enum_name.c_str(), GetName().c_str()));
zeekygen::detail::IdentifierInfo* doc = detail::zeekygen_mgr->GetIdentifierInfo(it->second); zeekygen::detail::IdentifierInfo* doc = detail::zeekygen_mgr->GetIdentifierInfo(enum_name);
if ( ! doc ) { if ( ! doc ) {
reporter->InternalWarning("Enum %s documentation lookup failure", it->second.c_str()); reporter->InternalWarning("Enum %s documentation lookup failure", enum_name.c_str());
continue; continue;
} }

View file

@ -863,14 +863,13 @@ protected:
void DoDescribe(ODesc* d) const override; void DoDescribe(ODesc* d) const override;
using NameMap = std::map<std::string, zeek_int_t>; std::map<std::string, zeek_int_t> names;
NameMap names; std::map<zeek_int_t, std::string> rev_names;
// Whether any of the elements of the enum were added via redef's. // Whether any of the elements of the enum were added via redef's.
bool has_redefs = false; bool has_redefs = false;
using ValMap = std::unordered_map<zeek_int_t, EnumValPtr>; std::unordered_map<zeek_int_t, EnumValPtr> vals;
ValMap vals;
// The counter is initialized to 0 and incremented on every implicit // The counter is initialized to 0 and incremented on every implicit
// auto-increment name that gets added (thus its > 0 if // auto-increment name that gets added (thus its > 0 if