From 2041921fcbaedf2f32ff92f6f38422f149258f46 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 10:35:04 +0100 Subject: [PATCH 01/12] Obj: make the Location destructor non-virtual Nobody ever derives from this class. This removes an indirect call and allows inlining the destructor. This speeds up Zeek startup by 1-2%. --- src/Obj.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Obj.h b/src/Obj.h index 0c96013620..df5088179e 100644 --- a/src/Obj.h +++ b/src/Obj.h @@ -6,7 +6,7 @@ class ODesc; -class Location { +class Location final { public: Location(const char* fname, int line_f, int line_l, int col_f, int col_l) { @@ -30,7 +30,7 @@ public: text = 0; } - virtual ~Location() + ~Location() { if ( delete_data ) delete [] filename; From 91e0860cd4746e1f5a07f423f4dacdafa6be5e61 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 10:36:47 +0100 Subject: [PATCH 02/12] Obj: remove Location::delete_data, nobody ever sets it This allows removing the destructor completely, making the class trivially-destructible. --- src/Obj.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Obj.h b/src/Obj.h index df5088179e..35b9574c3b 100644 --- a/src/Obj.h +++ b/src/Obj.h @@ -15,7 +15,6 @@ public: last_line = line_l; first_column = col_f; last_column = col_l; - delete_data = false; timestamp = 0; text = 0; @@ -25,17 +24,10 @@ public: { filename = 0; first_line = last_line = first_column = last_column = 0; - delete_data = false; timestamp = 0; text = 0; } - ~Location() - { - if ( delete_data ) - delete [] filename; - } - void Describe(ODesc* d) const; bool operator==(const Location& l) const; @@ -45,7 +37,6 @@ public: const char* filename; int first_line, last_line; int first_column, last_column; - bool delete_data; // Timestamp and text for compatibility with Bison's default yyltype. int timestamp; From ba445d36f97250fa1d17ca0014d7ea5dca7f6a47 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 13:37:40 +0100 Subject: [PATCH 03/12] Obj: remove unused fields Location::{timestamp,text} --- src/Obj.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Obj.h b/src/Obj.h index 35b9574c3b..bba40d2428 100644 --- a/src/Obj.h +++ b/src/Obj.h @@ -15,17 +15,12 @@ public: last_line = line_l; first_column = col_f; last_column = col_l; - - timestamp = 0; - text = 0; } Location() { filename = 0; first_line = last_line = first_column = last_column = 0; - timestamp = 0; - text = 0; } void Describe(ODesc* d) const; @@ -37,10 +32,6 @@ public: const char* filename; int first_line, last_line; int first_column, last_column; - - // Timestamp and text for compatibility with Bison's default yyltype. - int timestamp; - char* text; }; #define YYLTYPE yyltype From b4966858ce5fa9d44ccffa5d06690f5e4cb57b79 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 13:38:08 +0100 Subject: [PATCH 04/12] Obj: make the Location constructors `constexpr` --- src/Obj.h | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/Obj.h b/src/Obj.h index bba40d2428..cc66274598 100644 --- a/src/Obj.h +++ b/src/Obj.h @@ -8,20 +8,12 @@ class ODesc; class Location final { public: - Location(const char* fname, int line_f, int line_l, int col_f, int col_l) - { - filename = fname; - first_line = line_f; - last_line = line_l; - first_column = col_f; - last_column = col_l; - } + constexpr Location(const char* fname, int line_f, int line_l, + int col_f, int col_l) noexcept + :filename(fname), first_line(line_f), last_line(line_l), + first_column(col_f), last_column(col_l) {} - Location() - { - filename = 0; - first_line = last_line = first_column = last_column = 0; - } + Location() = default; void Describe(ODesc* d) const; @@ -29,9 +21,9 @@ public: bool operator!=(const Location& l) const { return ! (*this == l); } - const char* filename; - int first_line, last_line; - int first_column, last_column; + const char* filename = nullptr; + int first_line = 0, last_line = 0; + int first_column = 0, last_column = 0; }; #define YYLTYPE yyltype From 05f692995d5e04c95fa6b70c055ce8db584c0e9c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 10:47:25 +0100 Subject: [PATCH 05/12] Dict: make the destructor non-virtual --- src/Dict.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Dict.h b/src/Dict.h index 92a124486d..dc9ee9982c 100644 --- a/src/Dict.h +++ b/src/Dict.h @@ -23,7 +23,7 @@ class Dictionary { public: explicit Dictionary(dict_order ordering = UNORDERED, int initial_size = 0); - virtual ~Dictionary(); + ~Dictionary(); // Member functions for looking up a key, inserting/changing its // contents, and deleting it. These come in two flavors: one From acdfd5706edfdba2ec5980d83b841c360cb93c6d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 2 Feb 2020 16:48:18 +0100 Subject: [PATCH 06/12] Anon: remove unnecessary {map,vector}::clear() calls The destructor will do this automatically. --- src/Anon.cc | 2 -- src/Anon.h | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Anon.cc b/src/Anon.cc index 0c511ee158..691cf7f2d5 100644 --- a/src/Anon.cc +++ b/src/Anon.cc @@ -144,8 +144,6 @@ AnonymizeIPAddr_A50::~AnonymizeIPAddr_A50() { for ( unsigned int i = 0; i < blocks.size(); ++i ) delete [] blocks[i]; - - blocks.clear(); } void AnonymizeIPAddr_A50::init() diff --git a/src/Anon.h b/src/Anon.h index 6c3125f94c..8bd9861e2a 100644 --- a/src/Anon.h +++ b/src/Anon.h @@ -40,7 +40,7 @@ typedef uint32_t ipaddr32_t; class AnonymizeIPAddr { public: - virtual ~AnonymizeIPAddr() { mapping.clear(); } + virtual ~AnonymizeIPAddr() = default; ipaddr32_t Anonymize(ipaddr32_t addr); From f8e9cc0fc559fc9de0c3196e38c609ab38373c92 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 11:53:43 +0100 Subject: [PATCH 07/12] Scope: Vars() returns const reference No caller wants to modify the container. --- src/Scope.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Scope.h b/src/Scope.h index 7485347669..0cc70b7751 100644 --- a/src/Scope.h +++ b/src/Scope.h @@ -45,7 +45,7 @@ public: BroType* ReturnType() const { return return_type; } size_t Length() const { return local.size(); } - std::map& Vars() { return local; } + const std::map& Vars() { return local; } ID* GenerateTemporary(const char* name); From f1908b6212ea3abd18e35d99a957f4995c48400e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 11:53:33 +0100 Subject: [PATCH 08/12] Scope: convert Scope::Lookup() and others to template Allows passing rvalue references which eliminates unnecessary std::string copies. This speeds up Zeek startup by 1-2%. --- src/Scope.cc | 4 ++-- src/Scope.h | 16 +++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/Scope.cc b/src/Scope.cc index 509f686594..f53d4e0ba4 100644 --- a/src/Scope.cc +++ b/src/Scope.cc @@ -177,11 +177,11 @@ ID* install_ID(const char* name, const char* module_name, ID* id = new ID(full_name.data(), scope, is_export); if ( SCOPE_FUNCTION != scope ) - global_scope()->Insert(full_name, id); + global_scope()->Insert(std::move(full_name), id); else { id->SetOffset(top_scope->Length()); - top_scope->Insert(full_name, id); + top_scope->Insert(std::move(full_name), id); } return id; diff --git a/src/Scope.h b/src/Scope.h index 0cc70b7751..942d38ae1c 100644 --- a/src/Scope.h +++ b/src/Scope.h @@ -2,6 +2,7 @@ #pragma once +#include #include #include @@ -18,18 +19,23 @@ public: explicit Scope(ID* id, attr_list* al); ~Scope() override; - ID* Lookup(const std::string& name) const + template + ID* Lookup(N &&name) const { - const auto& entry = local.find(name); + const auto& entry = local.find(std::forward(name)); if ( entry != local.end() ) return entry->second; return nullptr; } - void Insert(const std::string& name, ID* id) { local[name] = id; } - ID* Remove(const std::string& name) + + template + void Insert(N &&name, ID* id) { local[std::forward(name)] = id; } + + template + ID* Remove(N &&name) { - const auto& entry = local.find(name); + const auto& entry = local.find(std::forward(name)); if ( entry != local.end() ) { ID* id = entry->second; From 0548e1255f5c88782e809f16c4fbf97438e8e1cd Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 12:19:24 +0100 Subject: [PATCH 09/12] module_util: make GLOBAL_MODULE_NAME constexpr This allows the compiler to hard-code pointers to the string without looking up a global variable at runtime. --- src/module_util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/module_util.h b/src/module_util.h index 5e1597a987..1186ddcb12 100644 --- a/src/module_util.h +++ b/src/module_util.h @@ -6,7 +6,7 @@ #include -static const char* GLOBAL_MODULE_NAME = "GLOBAL"; +static constexpr const char* GLOBAL_MODULE_NAME = "GLOBAL"; extern std::string extract_module_name(const char* name); extern std::string extract_var_name(const char* name); From 98241bbc600e61ac633b2c6e02893bdbd242caf1 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 12:49:41 +0100 Subject: [PATCH 10/12] util: pass string_view to without_bropath_component() --- src/util.cc | 11 ++++++----- src/util.h | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/util.cc b/src/util.cc index 1a9b01d5bb..37832d089d 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1669,7 +1669,7 @@ string normalize_path(std::string_view path) return new_path; } -string without_bropath_component(const string& path) +string without_bropath_component(std::string_view path) { string rval = normalize_path(path); @@ -1683,13 +1683,14 @@ string without_bropath_component(const string& path) continue; // Found the containing directory. - rval.erase(0, common.size()); + std::string_view v(rval); + v.remove_prefix(common.size()); // Remove leading path separators. - while ( rval.size() && rval[0] == '/' ) - rval.erase(0, 1); + while ( !v.empty() && v.front() == '/' ) + v.remove_prefix(1); - return rval; + return std::string(v); } return rval; diff --git a/src/util.h b/src/util.h index a8e23f39ef..cb5e1fc2eb 100644 --- a/src/util.h +++ b/src/util.h @@ -353,7 +353,7 @@ std::string normalize_path(std::string_view path); * @param path A file/directory path that may be within a ZEEKPATH component. * @return *path* minus the common ZEEKPATH component (if any) removed. */ -std::string without_bropath_component(const std::string& path); +std::string without_bropath_component(std::string_view path); /** * Locate a file within a given search path. From 95e646fca72627acd41b2b4fb5add2210cbe5cfd Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 12:50:52 +0100 Subject: [PATCH 11/12] util: optimize the normal_path() common case Speeds up Zeek startup by 2%. --- src/util.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/util.cc b/src/util.cc index 37832d089d..b143d0a4ef 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1612,6 +1612,15 @@ TEST_CASE("util normalize_path") string normalize_path(std::string_view path) { + if ( path.find("/.") == std::string_view::npos && + path.find("//") == std::string_view::npos ) + { + // no need to normalize anything + if ( path.size() > 1 && path.back() == '/' ) + path.remove_suffix(1); + return std::string(path); + } + size_t n; vector final_components; string new_path; From 298fd125ae4faabe80504a14910ba649bd0ed19f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 4 Feb 2020 12:54:08 +0100 Subject: [PATCH 12/12] plugin/Manager: migrate to std::string_view --- src/plugin/Manager.cc | 4 ++-- src/plugin/Manager.h | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/plugin/Manager.cc b/src/plugin/Manager.cc index 12c13ac3c3..39ac0568ba 100644 --- a/src/plugin/Manager.cc +++ b/src/plugin/Manager.cc @@ -478,9 +478,9 @@ Manager::bif_init_func_map* Manager::BifFilesInternal() return bifs; } -Plugin* Manager::LookupPluginByPath(std::string path) +Plugin* Manager::LookupPluginByPath(std::string_view _path) { - path = normalize_path(path); + auto path = normalize_path(_path); if ( is_file(path) ) path = SafeDirname(path).result; diff --git a/src/plugin/Manager.h b/src/plugin/Manager.h index 11fbcab74f..8127a77369 100644 --- a/src/plugin/Manager.h +++ b/src/plugin/Manager.h @@ -4,6 +4,8 @@ #include #include +#include + #include "Plugin.h" #include "Component.h" @@ -153,7 +155,7 @@ public: * path. The path can be the plugin directory itself, or any path * inside it. */ - Plugin* LookupPluginByPath(std::string path); + Plugin* LookupPluginByPath(std::string_view path); /** * Returns true if there's at least one plugin interested in a given