From 2964093e5dba20c603771b1a87e9e2fbfa1bf606 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Sat, 4 Apr 2020 21:18:50 -0400 Subject: [PATCH 1/4] Reorder some class variables to fill in gaps in structure packing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The big hitters: Dict: Fills in four 4-byte holes in the structure. This shrinks Dictionary from 136 bytes to 114 bytes. Desc: Fills in a 6-byte hole in the structure. This shrinks ODesc from 152 bytes to 144 bytes. Frame: Moves and combines 4 bool variables from a few places into one single 4-byte block. This resolves all of the holes at once. This shrinks Frame from 216 bytes to 192 bytes and removes one cache line. Func: Moves one int32_t variable to fill in a 4-byte hole. This shrinks Func from 112 bytes to 104 bytes. ID: Moves two bool variables to fill in a 3-byte hole. This leaves behind a 1-byte hole, but removes a 6-byte pad from the end of the structure. This shrinks ID from 144 bytes to 136 bytes. Other changes: RuleHdrTest: Fills in one 4-byte hole in the structure. This shrinks RuleHdrTest from 248 bytes to 240 bytes. RuleEndpointState: Moves one bool variable down in the structure to reduce a 7-byte hole. This unfortunately causes a 3-byte hole later in the structure but there’s no easy way to filll it in. This does shrink RuleEndpointState from 128 bytes to 120 bytes though. ScannedFile: Moves two bool values to reduce a 4-byte hole by 2 bytes. This shrinks ScannedFile from 64 bytes to 56 bytes. Brofiler: Moves one char value to reduce a 4-byte hole by 1 byte. This shrinks Brofiler from 96 bytes to 88 bytes and removes one cache line. DbgBreakpoint: Moves some values around to fill in a 4-byte hole and reduce a second. A 2-byte hole still exists, but the structure shrinks from 632 bytes to 624 bytes. It’s possible on this one that one of the int32_t values could be an int16_t and remove the last 2-byte gap. ParseLocationRec: Moves one int to fill in a 4-byte hole. This shrinks ParseLocationRec from 32 bytes to 24 bytes. DebugCmdInfo: Moves one bool variable to shift a few others up. This results in a 6-byte pad at the end of the structure but removes a 7-byte hole in the middle. This shrinks DebugCmdInfo from 56 bytes to 48 bytes. FragReassembler: Moves one variable down to fill in a 4-byte hole. This shrinks FragReassembler from 272 bytes to 264 bytes. nb_dns_result: Moves ones uint32_t variable to fill in a 4-byte hole, also removing a 4-byte pad from the end of the structure. This shrinks nb_dns_result from 32 bytes to 24 bytes. nb_dns_entry: Moves one short value to fill in a 2-byte hole, also removing a 6-byte hole. This shrinks nb_dns_entry from 1064 bytes to 1056 bytes. --- src/Brofiler.h | 12 ++++++------ src/DNS_Mgr.h | 4 ++-- src/DbgBreakpoint.h | 12 ++++++------ src/Debug.h | 2 +- src/DebugCmds.h | 5 ++--- src/Desc.h | 10 ++++++---- src/Dict.h | 5 +++-- src/Frag.h | 4 ++-- src/Frame.h | 10 +++++----- src/Func.h | 2 +- src/ID.h | 4 ++-- src/Net.h | 7 ++++--- src/RuleMatcher.h | 5 ++--- src/Val.h | 2 +- src/nb_dns.c | 2 +- src/nb_dns.h | 2 +- 16 files changed, 45 insertions(+), 43 deletions(-) diff --git a/src/Brofiler.h b/src/Brofiler.h index ee8d528baf..0587d6e5c6 100644 --- a/src/Brofiler.h +++ b/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, 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. diff --git a/src/DNS_Mgr.h b/src/DNS_Mgr.h index 8155375e2d..f82b4cfa77 100644 --- a/src/DNS_Mgr.h +++ b/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) { } diff --git a/src/DbgBreakpoint.h b/src/DbgBreakpoint.h index e27fabe2c1..ae86e83b69 100644 --- a/src/DbgBreakpoint.h +++ b/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 }; diff --git a/src/Debug.h b/src/Debug.h index 676c587635..ae3114ebc4 100644 --- a/src/Debug.h +++ b/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; diff --git a/src/DebugCmds.h b/src/DebugCmds.h index 470e4ae993..2e4e424888 100644 --- a/src/DebugCmds.h +++ b/src/DebugCmds.h @@ -30,14 +30,13 @@ public: protected: DebugCmd cmd; - int num_names; + int32_t num_names; std::vector 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; }; diff --git a/src/Desc.h b/src/Desc.h index 2a29c75a92..bbb365abd5 100644 --- a/src/Desc.h +++ b/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 escape_set; + bool is_short; + bool want_quotes; + + int indent_with_spaces; + + using escape_set = std::set; 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; diff --git a/src/Dict.h b/src/Dict.h index b5edeba76b..48e538c3af 100644 --- a/src/Dict.h +++ b/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** 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; diff --git a/src/Frag.h b/src/Frag.h index 31d5ec5bd0..a7cf5a60ec 100644 --- a/src/Frag.h +++ b/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; }; diff --git a/src/Frame.h b/src/Frame.h index 0961290ab1..6df91b7afa 100644 --- a/src/Frame.h +++ b/src/Frame.h @@ -261,6 +261,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 +275,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; @@ -289,12 +293,8 @@ 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; const CallExpr* call; - bool delayed; std::vector functions_with_closure_frame_reference; }; diff --git a/src/Func.h b/src/Func.h index c414e3ac85..ac707fcb52 100644 --- a/src/Func.h +++ b/src/Func.h @@ -111,9 +111,9 @@ protected: vector bodies; IntrusivePtr scope; Kind kind; + uint32_t unique_id; IntrusivePtr type; string name; - uint32_t unique_id; static vector unique_ids; }; diff --git a/src/ID.h b/src/ID.h index ee96ea7845..9f56b1c20e 100644 --- a/src/ID.h +++ b/src/ID.h @@ -122,6 +122,8 @@ protected: const char* name; IDScope scope; bool is_export; + bool infer_return_type; + bool weak_ref; IntrusivePtr 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> option_handlers; - bool infer_return_type; - bool weak_ref; }; diff --git a/src/Net.h b/src/Net.h index e7b7f3bd7f..8dd643c399 100644 --- a/src/Net.h +++ b/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) { } }; diff --git a/src/RuleMatcher.h b/src/RuleMatcher.h index 0650081000..d5f958c2a5 100644 --- a/src/RuleMatcher.h +++ b/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 rule_hdr_test_list; @@ -173,7 +172,6 @@ private: typedef PList 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 }; diff --git a/src/Val.h b/src/Val.h index 96b610892d..2f33c4b60e 100644 --- a/src/Val.h +++ b/src/Val.h @@ -691,7 +691,7 @@ class CompositeHash; class HashKey; class Frame; -class TableVal : public Val, public notifier::Modifiable { +class TableVal final : public Val, public notifier::Modifiable { public: explicit TableVal(IntrusivePtr t, IntrusivePtr attrs = nullptr); ~TableVal() override; diff --git a/src/nb_dns.c b/src/nb_dns.c index 106740bb61..7cc6e80a19 100644 --- a/src/nb_dns.c +++ b/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; }; diff --git a/src/nb_dns.h b/src/nb_dns.h index 3505239f91..1b96a8d426 100644 --- a/src/nb_dns.h +++ b/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; From 471cf8587b31632e5886751432b6c246706738e6 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Sat, 4 Apr 2020 21:19:22 -0400 Subject: [PATCH 2/4] Mark constants in List constexpr so they don't actually take up space in created objects This resizes List from 24 bytes to 16 bytes. --- src/List.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/List.h b/src/List.h index 58624c7f0b..d4b9798e4d 100644 --- a/src/List.h +++ b/src/List.h @@ -33,8 +33,8 @@ template 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) From 6f8bbadcf9c92396008bb976210656f1b416ffde Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Sat, 4 Apr 2020 21:22:46 -0400 Subject: [PATCH 3/4] Set InternalHashTag to a uint16_t so CompositeHash doesn't have a gap in it. Resizes CompositeHash from 32 bytes to 24 bytes. --- src/Type.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Type.h b/src/Type.h index 9686bcdf9f..8cf7f58387 100644 --- a/src/Type.h +++ b/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, From f5865b6b97982f612ccfc888c22f008248b871ae Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 6 Apr 2020 15:32:39 -0700 Subject: [PATCH 4/4] Lazy-initalize some of the fields in Frame to reduce the size of all Frames when they're not used --- src/Frame.cc | 60 +++++++++++++++++++++++++++++++++++----------------- src/Frame.h | 8 ++++--- 2 files changed, 46 insertions(+), 22 deletions(-) diff --git a/src/Frame.cc b/src/Frame.cc index 1c093398dc..3ddab79c56 100644 --- a/src/Frame.cc +++ b/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>(); + + 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(*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,13 @@ 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(*offset_map); + else + *(other->offset_map) = *offset_map; + } return other; } @@ -281,7 +297,9 @@ broker::expected Frame::Serialize(const Frame* target, const id_li // and id_list them; - std::unordered_map new_map(target->offset_map); + std::unordered_map new_map; + if ( target->offset_map ) + new_map = *(target->offset_map); for (const auto& we : selection) { @@ -358,7 +376,7 @@ std::pair> Frame::Unserialize(const broker::vector& da return std::make_pair(true, nullptr); id_list outer_ids; - std::unordered_map offset_map; + OffsetMap offset_map; IntrusivePtr closure; auto where = data.begin(); @@ -442,7 +460,8 @@ std::pair> Frame::Unserialize(const broker::vector& da // We'll associate this frame with a function later. auto rf = make_intrusive(frame_size, nullptr, nullptr); - rf->offset_map = std::move(offset_map); + rf->offset_map = make_unique(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 +496,10 @@ std::pair> 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(); + + std::transform(ids.begin(), ids.end(), std::inserter(*offset_map, offset_map->end()), [] (const ID* id) -> std::pair { return std::make_pair(std::string(id->Name()), id->Offset()); diff --git a/src/Frame.h b/src/Frame.h index 6df91b7afa..e5ebd22724 100644 --- a/src/Frame.h +++ b/src/Frame.h @@ -234,6 +234,8 @@ public: private: + using OffsetMap = std::unordered_map; + /** * 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 - SerializeOffsetMap(const std::unordered_map& in); + SerializeOffsetMap(const OffsetMap& in); /** Serializes an id_list */ static broker::expected @@ -283,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 offset_map; + std::unique_ptr offset_map; /** The function this frame is associated with. */ const BroFunc* function; @@ -296,7 +298,7 @@ private: IntrusivePtr trigger; const CallExpr* call; - std::vector functions_with_closure_frame_reference; + std::unique_ptr> functions_with_closure_frame_reference; }; /**