Merge remote-tracking branch 'max/optimize'

* max/optimize:
  plugin/Manager: migrate to std::string_view
  util: optimize the normal_path() common case
  util: pass string_view to without_bropath_component()
  module_util: make GLOBAL_MODULE_NAME constexpr
  Scope: convert Scope::Lookup() and others to template
  Scope: Vars() returns const reference
  Anon: remove unnecessary {map,vector}::clear() calls
  Dict: make the destructor non-virtual
  Obj: make the Location constructors `constexpr`
  Obj: remove unused fields Location::{timestamp,text}
  Obj: remove Location::delete_data, nobody ever sets it
  Obj: make the Location destructor non-virtual

CHANGES

@@ -1,4 +1,53 @@
 
+3.1.0-dev.597 | 2020-02-07 15:25:56 -0700
+
+  * plugin/Manager: migrate to std::string_view (Max Kellermann)
+
+  * util: optimize the normal_path() common case
+
+    Speeds up Zeek startup by 2%. (Max Kellermann)
+
+  * util: pass string_view to without_bropath_component() (Max Kellermann)
+
+  * 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. (Max Kellermann)
+
+  * 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%. (Max Kellermann)
+
+  * Scope: Vars() returns const reference
+
+    No caller wants to modify the container. (Max Kellermann)
+
+  * Anon: remove unnecessary {map,vector}::clear() calls
+
+    The destructor will do this automatically. (Max Kellermann)
+
+  * Dict: make the destructor non-virtual (Max Kellermann)
+
+  * Obj: make the Location constructors `constexpr` (Max Kellermann)
+
+  * Obj: remove unused fields Location::{timestamp,text} (Max Kellermann)
+
+  * Obj: remove Location::delete_data, nobody ever sets it
+
+    This allows removing the destructor completely, making the class
+    trivially-destructible. (Max Kellermann)
+
+  * 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%. (Max Kellermann)
+
+
 3.1.0-dev.584 | 2020-02-07 14:12:17 -0800
 
   * Improve kerberos analyzer address and event handling

VERSION

@@ -1 +1 @@
-3.1.0-dev.584
+3.1.0-dev.597

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()

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);
 

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

src/Obj.h

@@ -6,35 +6,14 @@
 
 class ODesc;
 
-class Location {
+class Location final {
 public:
-	Location(const char* fname, int line_f, int line_l, int col_f, int col_l)
+	constexpr Location(const char* fname, int line_f, int line_l,
-		{
+			   int col_f, int col_l) noexcept
-		filename = fname;
+		:filename(fname), first_line(line_f), last_line(line_l),
-		first_line = line_f;
+		 first_column(col_f), last_column(col_l) {}
-		last_line = line_l;
-		first_column = col_f;
-		last_column = col_l;
-		delete_data = false;
 
-		timestamp = 0;
+	Location() = default;
-		text = 0;
-		}
-
-	Location()
-		{
-		filename = 0;
-		first_line = last_line = first_column = last_column = 0;
-		delete_data = false;
-		timestamp = 0;
-		text = 0;
-		}
-
-	virtual ~Location()
-		{
-		if ( delete_data )
-			delete [] filename;
-		}
 
 	void Describe(ODesc* d) const;
 
@@ -42,14 +21,9 @@ public:
 	bool operator!=(const Location& l) const
 		{ return ! (*this == l); }
 
-	const char* filename;
+	const char* filename = nullptr;
-	int first_line, last_line;
+	int first_line = 0, last_line = 0;
-	int first_column, last_column;
+	int first_column = 0, last_column = 0;
-	bool delete_data;
-
-	// Timestamp and text for compatibility with Bison's default yyltype.
-	int timestamp;
-	char* text;
 };
 
 #define YYLTYPE yyltype

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;

src/Scope.h

@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include <utility>
 #include <string>
 #include <map>
 
@@ -18,18 +19,23 @@ public:
 	explicit Scope(ID* id, attr_list* al);
 	~Scope() override;
 
-	ID* Lookup(const std::string& name) const
+	template<typename N>
+	ID* Lookup(N &&name) const
 		{
-		const auto& entry = local.find(name);
+		const auto& entry = local.find(std::forward<N>(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<typename N>
+	void Insert(N &&name, ID* id) { local[std::forward<N>(name)] = id; }
+
+	template<typename N>
+	ID* Remove(N &&name)
 		{
-		const auto& entry = local.find(name);
+		const auto& entry = local.find(std::forward<N>(name));
 		if ( entry != local.end() )
 			{
 			ID* id = entry->second;
@@ -45,7 +51,7 @@ public:
 	BroType* ReturnType() const	{ return return_type; }
 
 	size_t Length() const		{ return local.size(); }
-	std::map<std::string, ID*>& Vars()	{ return local; }
+	const std::map<std::string, ID*>& Vars()	{ return local; }
 
 	ID* GenerateTemporary(const char* name);
 

src/module_util.h

@@ -6,7 +6,7 @@
 
 #include <string>
 
-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);

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;

src/plugin/Manager.h

@@ -4,6 +4,8 @@
 
 #include <utility>
 #include <map>
+#include <string_view>
+
 
 #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

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<std::string_view> final_components;
 	string new_path;
@@ -1669,7 +1678,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 +1692,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] == '/' )
+		while ( !v.empty() && v.front() == '/' )
-			rval.erase(0, 1);
+			v.remove_prefix(1);
 
-		return rval;
+		return std::string(v);
 		}
 
 	return rval;

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.