Merge remote-tracking branch 'origin/topic/timw/structure-packing'

During merge, changed a bit of how Frame OffsetMap
assignments/contruction were handled to keep parity with old version.

* origin/topic/timw/structure-packing:
  Lazy-initalize some of the fields in Frame to reduce the size of all Frames when they're not used
  Set InternalHashTag to a uint16_t so CompositeHash doesn't have a gap in it.
  Mark constants in List constexpr so they don't actually take up space in created objects
  Reorder some class variables to fill in gaps in structure packing

src/Brofiler.h

@@ -56,7 +56,12 @@ private:
 	 * Indicates whether new statments will not be considered as part of
 	 * coverage statistics because it was marked with the @no-test tag.
 	 */
-	unsigned int ignoring;
+	uint32_t ignoring;
+
+	/**
+	 * The character to use to delimit Brofiler output files.  Default is '\t'.
+	 */
+	char delim;
 
 	/**
 	 * This maps Stmt location-desc pairs to the total number of times that
@@ -66,11 +71,6 @@ private:
 	 */
 	map<pair<string, string>, uint64_t> usage_map;
 
-	/**
-	 * The character to use to delimit Brofiler output files.  Default is '\t'.
-	 */
-	char delim;
-
 	/**
 	 * A canonicalization routine for Stmt descriptions containing characters
 	 * that don't agree with the output format of Brofiler.

src/DNS_Mgr.h

@@ -163,11 +163,11 @@ protected:
 
 	struct AsyncRequest {
 		double time;
-		bool is_txt;
-		bool processed;
 		IPAddr host;
 		string name;
 		CallbackList callbacks;
+		bool is_txt;
+		bool processed;
 
 		AsyncRequest() : time(0.0), is_txt(false), processed(false) { }
 

src/DbgBreakpoint.h

@@ -63,21 +63,21 @@ protected:
 	void PrintHitMsg();	// display reason when the breakpoint hits
 
 	Kind kind;
-	bool enabled;	// ### comment this and next
-	bool temporary;
-	int BPID;
+	int32_t BPID;
 
 	char description[512];
 	string function_name;	// location
 	const char* source_filename;
-	int source_line;
+	int32_t source_line;
+	bool enabled;	// ### comment this and next
+	bool temporary;
 
 	Stmt* at_stmt;
 	double at_time;	// break when the virtual time is this
 
 	// Support for conditional and N'th time breakpoints.
-	int repeat_count;	// if positive, break after this many hits
-	int hit_count;	// how many times it's been hit (w/o breaking)
+	int32_t repeat_count;	// if positive, break after this many hits
+	int32_t hit_count;	// how many times it's been hit (w/o breaking)
 
 	string condition;	// condition to evaluate; nil for none
 };

src/Debug.h

@@ -18,9 +18,9 @@ class Stmt;
 enum ParseLocationRecType { plrUnknown, plrFileAndLine, plrFunction };
 struct ParseLocationRec {
 	ParseLocationRecType type;
+	int32_t line;
 	Stmt* stmt;
 	const char* filename;
-	int line;
 };
 
 class StmtLocMapping;

src/DebugCmds.h

@@ -30,14 +30,13 @@ public:
 protected:
 	DebugCmd cmd;
 
-	int num_names;
+	int32_t num_names;
 	std::vector<const char*> names;
+	const char* const helpstring;
 
 	// Whether executing this should restart execution of the script.
 	bool resume_execution;
 
-	const char* const helpstring;
-
 	// Does entering a blank line repeat this command?
 	bool repeatable;
 };

src/Desc.h

@@ -190,15 +190,17 @@ protected:
 
 	bool utf8; // whether valid utf-8 sequences may pass through unescaped
 	bool escape;	// escape unprintable characters in output?
-	typedef std::set<std::string> escape_set;
+	bool is_short;
+	bool want_quotes;
+
+	int indent_with_spaces;
+
+	using escape_set = std::set<std::string>;
 	escape_set escape_sequences; // additional sequences of chars to escape
 
 	BroFile* f;	// or the file we're using.
 
 	int indent_level;
-	int indent_with_spaces;
-	bool is_short;
-	bool want_quotes;
 	bool do_flush;
 	bool include_stats;
 

src/Dict.h

@@ -162,17 +162,18 @@ private:
 	int num_buckets = 0;
 	int num_entries = 0;
 	int max_num_entries = 0;
+	int thresh_entries = 0;
 	uint64_t cumulative_entries = 0;
 	double den_thresh = 0.0;
-	int thresh_entries = 0;
 
 	// Resizing table (replicates tbl above).
 	PList<DictEntry>** tbl2 = nullptr;
 	int num_buckets2 = 0;
 	int num_entries2 = 0;
 	int max_num_entries2 = 0;
-	double den_thresh2 = 0;
+
 	int thresh_entries2 = 0;
+	double den_thresh2 = 0;
 
 	hash_t tbl_next_ind = 0;
 

src/Frag.h

@@ -44,11 +44,11 @@ protected:
 
 	u_char* proto_hdr;
 	IP_Hdr* reassembled_pkt;
-	uint16_t proto_hdr_len;
 	NetSessions* s;
 	uint64_t frag_size;	// size of fully reassembled fragment
-	uint16_t next_proto; // first IPv6 fragment header's next proto field
 	FragReassemblerKey key;
+	uint16_t next_proto; // first IPv6 fragment header's next proto field
+	uint16_t proto_hdr_len;
 
 	FragTimer* expire_timer;
 };

src/Frame.cc

@@ -36,10 +36,13 @@ Frame::Frame(int arg_size, const BroFunc* func, const zeek::Args* fn_args)
 
 Frame::~Frame()
 	{
-	for ( auto& func : functions_with_closure_frame_reference )
+	if ( functions_with_closure_frame_reference )
 		{
-		func->StrengthenClosureReference(this);
-		Unref(func);
+		for ( auto& func : *functions_with_closure_frame_reference )
+			{
+			func->StrengthenClosureReference(this);
+			Unref(func);
+			}
 		}
 
 	if ( ! weak_closure_ref )
@@ -56,7 +59,11 @@ Frame::~Frame()
 void Frame::AddFunctionWithClosureRef(BroFunc* func)
 	{
 	::Ref(func);
-	functions_with_closure_frame_reference.emplace_back(func);
+
+	if ( ! functions_with_closure_frame_reference )
+		functions_with_closure_frame_reference = make_unique<std::vector<BroFunc*>>();
+
+	functions_with_closure_frame_reference->emplace_back(func);
 	}
 
 void Frame::SetElement(int n, Val* v, bool weak_ref)
@@ -95,11 +102,11 @@ void Frame::SetElement(const ID* id, Val* v)
 		}
 
 	// do we have an offset for it?
-	if ( offset_map.size() )
+	if ( offset_map && ! offset_map->empty() )
 		{
-		auto where = offset_map.find(std::string(id->Name()));
+		auto where = offset_map->find(std::string(id->Name()));
 
-		if ( where != offset_map.end() )
+		if ( where != offset_map->end() )
 			{
 			// Need to add a Ref to 'v' since the SetElement() for
 			// id->Offset() below is otherwise responsible for keeping track
@@ -121,10 +128,10 @@ Val* Frame::GetElement(const ID* id) const
 		}
 
 	// do we have an offset for it?
-	if ( offset_map.size() )
+	if ( offset_map && ! offset_map->empty() )
 		{
-		auto where = offset_map.find(std::string(id->Name()));
-		if ( where != offset_map.end() )
+		auto where = offset_map->find(std::string(id->Name()));
+		if ( where != offset_map->end() )
 			return frame[where->second];
 		}
 
@@ -174,7 +181,10 @@ void Frame::Describe(ODesc* d) const
 Frame* Frame::Clone() const
 	{
 	Frame* other = new Frame(size, function, func_args);
-	other->offset_map = offset_map;
+
+	if ( offset_map )
+		other->offset_map = make_unique<OffsetMap>(*offset_map);
+
 	other->CaptureClosure(closure, outer_ids);
 
 	other->call = call;
@@ -233,10 +243,10 @@ Frame* Frame::SelectiveClone(const id_list& selection, BroFunc* func) const
 
 	for ( const auto& id : us )
 		{
-		if ( offset_map.size() )
+		if ( offset_map && ! offset_map->empty() )
 			{
-			auto where = offset_map.find(std::string(id->Name()));
-			if ( where != offset_map.end() )
+			auto where = offset_map->find(std::string(id->Name()));
+			if ( where != offset_map->end() )
 				{
 				clone_if_not_func(frame, where->second, func, other);
 				continue;
@@ -265,7 +275,15 @@ Frame* Frame::SelectiveClone(const id_list& selection, BroFunc* func) const
 	if( closure )
 		other->CaptureClosure(closure, outer_ids);
 
-	other->offset_map = offset_map;
+	if ( offset_map )
+		{
+		if ( ! other->offset_map )
+			other->offset_map = make_unique<OffsetMap>(*offset_map);
+		else
+			*(other->offset_map) = *offset_map;
+		}
+	else
+		other->offset_map.reset();
 
 	return other;
 	}
@@ -281,7 +299,9 @@ broker::expected<broker::data> Frame::Serialize(const Frame* target, const id_li
 	// and
 	id_list them;
 
-	std::unordered_map<std::string, int> new_map(target->offset_map);
+	std::unordered_map<std::string, int> new_map;
+	if ( target->offset_map )
+		new_map = *(target->offset_map);
 
 	for (const auto& we : selection)
 		{
@@ -358,7 +378,7 @@ std::pair<bool, IntrusivePtr<Frame>> Frame::Unserialize(const broker::vector& da
 		return std::make_pair(true, nullptr);
 
 	id_list outer_ids;
-	std::unordered_map<std::string, int> offset_map;
+	OffsetMap offset_map;
 	IntrusivePtr<Frame> closure;
 
 	auto where = data.begin();
@@ -442,7 +462,8 @@ std::pair<bool, IntrusivePtr<Frame>> Frame::Unserialize(const broker::vector& da
 
 	// We'll associate this frame with a function later.
 	auto rf = make_intrusive<Frame>(frame_size, nullptr, nullptr);
-	rf->offset_map = std::move(offset_map);
+	rf->offset_map = make_unique<OffsetMap>(std::move(offset_map));
+
 	// Frame takes ownership of unref'ing elements in outer_ids
 	rf->outer_ids = std::move(outer_ids);
 	rf->closure = closure.release();
@@ -477,7 +498,10 @@ std::pair<bool, IntrusivePtr<Frame>> Frame::Unserialize(const broker::vector& da
 
 void Frame::AddKnownOffsets(const id_list& ids)
 	{
-	std::transform(ids.begin(), ids.end(), std::inserter(offset_map, offset_map.end()),
+	if ( ! offset_map )
+		offset_map = make_unique<OffsetMap>();
+
+	std::transform(ids.begin(), ids.end(), std::inserter(*offset_map, offset_map->end()),
 		       [] (const ID* id) -> std::pair<std::string, int>
 		       {
 		       return std::make_pair(std::string(id->Name()), id->Offset());

src/Frame.h

@@ -234,6 +234,8 @@ public:
 
 private:
 
+	using OffsetMap = std::unordered_map<std::string, int>;
+
 	/**
 	 * Unrefs the value at offset 'n' frame unless it's a weak reference.
 	 */
@@ -244,7 +246,7 @@ private:
 
 	/** Serializes an offset_map */
 	static broker::expected<broker::data>
-	SerializeOffsetMap(const std::unordered_map<std::string, int>& in);
+	SerializeOffsetMap(const OffsetMap& in);
 
 	/** Serializes an id_list */
 	static broker::expected<broker::data>
@@ -261,6 +263,11 @@ private:
 	/** The number of vals that can be stored in this frame. */
 	int size;
 
+	bool weak_closure_ref = false;
+	bool break_before_next_stmt;
+	bool break_on_return;
+	bool delayed;
+
 	/** Associates ID's offsets with values. */
 	Val** frame;
 
@@ -270,7 +277,6 @@ private:
 
 	/** The enclosing frame of this frame. */
 	Frame* closure;
-	bool weak_closure_ref = false;
 
 	/** ID's used in this frame from the enclosing frame. */
 	id_list outer_ids;
@@ -279,7 +285,7 @@ private:
 	 * Maps ID names to offsets. Used if this frame is  serialized
 	 * to maintain proper offsets after being sent elsewhere.
 	 */
-	std::unordered_map<std::string, int> offset_map;
+	std::unique_ptr<OffsetMap> offset_map;
 
 	/** The function this frame is associated with. */
 	const BroFunc* function;
@@ -289,14 +295,10 @@ private:
 	/** The next statement to be evaluted in the context of this frame. */
 	Stmt* next_stmt;
 
-	bool break_before_next_stmt;
-	bool break_on_return;
-
 	IntrusivePtr<trigger::Trigger> trigger;
 	const CallExpr* call;
-	bool delayed;
 
-	std::vector<BroFunc*> functions_with_closure_frame_reference;
+	std::unique_ptr<std::vector<BroFunc*>> functions_with_closure_frame_reference;
 };
 
 /**

src/Func.h

@@ -111,9 +111,9 @@ protected:
 	vector<Body> bodies;
 	IntrusivePtr<Scope> scope;
 	Kind kind;
+	uint32_t unique_id;
 	IntrusivePtr<BroType> type;
 	string name;
-	uint32_t unique_id;
 	static vector<Func*> unique_ids;
 };
 

src/ID.h

@@ -122,6 +122,8 @@ protected:
 	const char* name;
 	IDScope scope;
 	bool is_export;
+	bool infer_return_type;
+	bool weak_ref;
 	IntrusivePtr<BroType> type;
 	bool is_const, is_enum_const, is_type, is_option;
 	int offset;
@@ -130,6 +132,4 @@ protected:
 	// contains list of functions that are called when an option changes
 	std::multimap<int, IntrusivePtr<Func>> option_handlers;
 
-	bool infer_return_type;
-	bool weak_ref;
 };

src/List.h

@@ -33,8 +33,8 @@ template<typename T>
 class List {
 public:
 
-	const int DEFAULT_LIST_SIZE = 10;
-	const int LIST_GROWTH_FACTOR = 2;
+	constexpr static int DEFAULT_LIST_SIZE = 10;
+	constexpr static int LIST_GROWTH_FACTOR = 2;
 
 	~List()		{ free(entries); }
 	explicit List(int size = 0)

src/Net.h

@@ -93,17 +93,18 @@ struct ScannedFile {
 	dev_t dev;
 	ino_t inode;
 	int include_level;
-	string name;
 	bool skipped;		// This ScannedFile was @unload'd.
 	bool prefixes_checked;	// If loading prefixes for this file has been tried.
+	string name;
 
 	ScannedFile(dev_t arg_dev, ino_t arg_inode, int arg_include_level,
 	            const string& arg_name, bool arg_skipped = false,
 	            bool arg_prefixes_checked = false)
 		: dev(arg_dev), inode(arg_inode),
 		  include_level(arg_include_level),
-		  name(arg_name), skipped(arg_skipped),
-		  prefixes_checked(arg_prefixes_checked)
+		  skipped(arg_skipped),
+		  prefixes_checked(arg_prefixes_checked),
+		  name(arg_name)
 		{ }
 };
 

src/RuleMatcher.h

@@ -102,6 +102,7 @@ private:
 
 	uint32_t id;	// For debugging, each HdrTest gets an unique ID
 	static uint32_t idcounter;
+	int32_t level;	// level within the tree
 
 	// The following are all set by RuleMatcher::BuildRulesTree().
 	friend class RuleMatcher;
@@ -132,8 +133,6 @@ private:
 
 	RuleHdrTest* sibling;	// linkage within HdrTest tree
 	RuleHdrTest* child;
-
-	int level;	// level within the tree
 };
 
 typedef PList<RuleHdrTest> rule_hdr_test_list;
@@ -173,7 +172,6 @@ private:
 
 	typedef PList<Matcher> matcher_list;
 
-	bool is_orig;
 	analyzer::Analyzer* analyzer;
 	RuleEndpointState* opposite;
 	analyzer::pia::PIA* pia;
@@ -188,6 +186,7 @@ private:
 	bstr_list matched_text;
 
 	int payload_size;
+	bool is_orig;
 
 	int_list matched_rules;		// Rules for which all conditions have matched
 };

src/Type.h

@@ -56,7 +56,7 @@ enum function_flavor {
 	FUNC_FLAVOR_HOOK
 };
 
-enum InternalTypeTag {
+enum InternalTypeTag : uint16_t {
 	TYPE_INTERNAL_VOID,
 	TYPE_INTERNAL_INT, TYPE_INTERNAL_UNSIGNED, TYPE_INTERNAL_DOUBLE,
 	TYPE_INTERNAL_STRING, TYPE_INTERNAL_ADDR, TYPE_INTERNAL_SUBNET,

src/nb_dns.c

@@ -61,10 +61,10 @@ extern int recvfrom(int, void *, int, int, struct sockaddr *, int *);
 struct nb_dns_entry {
 	struct nb_dns_entry *next;
 	char name[NS_MAXDNAME + 1];
+	u_short id;
 	int qtype;			/* query type */
 	int atype;			/* address family */
 	int asize;			/* address size */
-	u_short id;
 	void *cookie;
 };
 

src/nb_dns.h

@@ -11,8 +11,8 @@ struct nb_dns_info;
 struct nb_dns_result {
 	void *cookie;
 	int host_errno;
-	struct hostent *hostent;
 	uint32_t ttl;
+	struct hostent *hostent;
 };
 
 typedef unsigned int nb_uint32_t;