Merge remote-tracking branch 'origin/topic/awelzel/random-perf-things'

* origin/topic/awelzel/random-perf-things:
  SegmentProfiler: Do not initialize initial_rusage
  EventMgr: Remove queue_flare, use GetNextTimeout() instead
  UpdateConnVal: Avoid FieldOffset() calls

CHANGES

@@ -1,3 +1,66 @@
+6.2.0-dev.238 | 2023-12-05 16:00:56 +0100
+
+  * SegmentProfiler: Do not initialize initial_rusage (Arne Welzel, Corelight)
+
+    We use the SegmentProfiler in quite a few hot places and the memset of
+    the rusage structure (144bytes here) can show up significantly even if
+    the segment profiler itself isn't used.
+
+    Relates to #3485.
+
+  * EventMgr: Remove queue_flare, use GetNextTimeout() instead (Arne Welzel, Corelight)
+
+    It can be visible overhead to call write() on the underlying pipe of the
+    EventMgr's flare whenever the first event is enqueued during an IO loop
+    iteration. Particularly in scenarios where there's about 1 event per packet
+    for long lived connections and script-side event processing is fast.
+
+    Given the event manager is drained anyhow at the end of the main loop, this
+    shouldn't be needed. In fact, the EventMgr.Process() method is basically
+    a stub. The one reason it is needed is when more events are enqueued during
+    a drain. That, however, can be dealt with by implementing GetNextTimeout()
+    to return 0.0 when there's more events queued. This way the main-loop's poll
+    timeout is 0.0 and it'll continue immediately.
+
+    This also allows to removes some extra code and drop the recently introduced
+    InitPostFork() addition: Without a pipe, there's no need to recreate it.
+
+  * UpdateConnVal: Avoid FieldOffset() calls (Arne Welzel, Corelight)
+
+    These can be significant if a lot of new connections and or events
+    are created for which an existing conn val needs updating and otherwise
+    things are very fast.
+
+  * RuleActionMIME: Switch to std::string (Arne Welzel, Corelight)
+
+    And return const std::string& from GetMIME(). Probably not at all performance
+    relevant, but while I'm already here.
+
+  * signatures: Support custom event via [event_name] syntax (Arne Welzel, Corelight)
+
+    This change allows to specify a per signature specific event, overriding
+    the default signature_match event. It further removes the message
+    parameter from such events if not provided in the signature.
+
+    This also tracks the message as StringValPtr directly to avoid
+    allocating the same StringVal for every DoAction() call.
+
+    Closes #3403
+
+  * zeek-setup: Exit when rule loading tickles reporter errors (Arne Welzel, Corelight)
+
+    With custom events for signatures, Reporter::error() may be invoked
+    while loading them. Early exit in case that happens. We could continue
+    and either disable the signatures or fallback to the default
+    signature_match() event, but not sure that would be obviously better.
+
+  * rule-scan: Copy yytext strings (Arne Welzel, Corelight)
+
+    When trying to use TOK_IDENT and TOK_STRING in a single rule, that
+    resulted in "corrupt" strings.
+
+    https://www.gnu.org/software/bison/manual/html_node/Strings-are-Destroyed.html
+
 6.2.0-dev.229 | 2023-12-04 12:32:17 -0800
 
   * Add facade types to avoid using raw Broker types (Dominik Charousset, Corelight)

VERSION

@@ -1 +1 @@
-6.2.0-dev.229
+6.2.0-dev.238

src/Event.cc

@@ -99,7 +99,6 @@ void EventMgr::QueueEvent(Event* event) {
 
     if ( ! head ) {
         head = tail = event;
-        queue_flare.Fire();
     }
     else {
         tail->SetNext(event);
@@ -177,25 +176,12 @@ void EventMgr::Describe(ODesc* d) const {
 }
 
 void EventMgr::Process() {
-    queue_flare.Extinguish();
-
     // While it semes like the most logical thing to do, we dont want
     // to call Drain() as part of this method. It will get called at
-    // the end of net_run after all of the sources have been processed
+    // the end of run_loop after all of the sources have been processed
     // and had the opportunity to spawn new events.
 }
 
-void EventMgr::InitPostScript() {
-    iosource_mgr->Register(this, true, false);
-    if ( ! iosource_mgr->RegisterFd(queue_flare.FD(), this) )
-        reporter->FatalError("Failed to register event manager FD with iosource_mgr");
-}
-
-void EventMgr::InitPostFork() {
-    // Re-initialize the flare, closing and re-opening the underlying
-    // pipe FDs. This is needed so that each Zeek process in a supervisor
-    // setup has its own pipe instead of them all sharing a single pipe.
-    queue_flare = zeek::detail::Flare{};
-}
+void EventMgr::InitPostScript() { iosource_mgr->Register(this, true, false); }
 
 } // namespace zeek

src/Event.h

@@ -106,14 +106,14 @@ public:
 
     void Describe(ODesc* d) const override;
 
-    double GetNextTimeout() override { return -1; }
+    // Let the IO loop know when there's more events to process
+    // by returning a zero-timeout.
+    double GetNextTimeout() override { return head ? 0.0 : -1.0; }
+
     void Process() override;
     const char* Tag() override { return "EventManager"; }
     void InitPostScript();
 
-    // Initialization to be done after a fork() happened.
-    void InitPostFork();
-
     uint64_t num_events_queued = 0;
     uint64_t num_events_dispatched = 0;
 
@@ -127,7 +127,6 @@ protected:
     double current_ts;
     RecordVal* src_val;
     bool draining;
-    detail::Flare queue_flare;
 };
 
 extern EventMgr event_mgr;

src/Stats.h

@@ -39,13 +39,13 @@ class SegmentProfiler {
 public:
     // The constructor takes some way of identifying the segment.
     SegmentProfiler(std::shared_ptr<SegmentStatsReporter> arg_reporter, const char* arg_name)
-        : reporter(std::move(arg_reporter)), name(arg_name), loc(), initial_rusage() {
+        : reporter(std::move(arg_reporter)), name(arg_name), loc() {
         if ( reporter )
             Init();
     }
 
     SegmentProfiler(std::shared_ptr<SegmentStatsReporter> arg_reporter, const Location* arg_loc)
-        : reporter(std::move(arg_reporter)), name(), loc(arg_loc), initial_rusage() {
+        : reporter(std::move(arg_reporter)), name(), loc(arg_loc) {
         if ( reporter )
             Init();
     }

src/analyzer/protocol/conn-size/ConnSize.cc

@@ -139,20 +139,15 @@ void ConnSize_Analyzer::SetDurationThreshold(double duration) {
 }
 
 void ConnSize_Analyzer::UpdateConnVal(RecordVal* conn_val) {
-    // RecordType *connection_type is declared in NetVar.h
-    RecordVal* orig_endp = conn_val->GetFieldAs<RecordVal>("orig");
-    RecordVal* resp_endp = conn_val->GetFieldAs<RecordVal>("resp");
-
-    // endpoint is the RecordType from NetVar.h
-    int pktidx = id::endpoint->FieldOffset("num_pkts");
-    int bytesidx = id::endpoint->FieldOffset("num_bytes_ip");
-
-    if ( pktidx < 0 )
-        reporter->InternalError("'endpoint' record missing 'num_pkts' field");
-
-    if ( bytesidx < 0 )
-        reporter->InternalError("'endpoint' record missing 'num_bytes_ip' field");
+    static const auto& conn_type = zeek::id::find_type<zeek::RecordType>("connection");
+    static const int origidx = conn_type->FieldOffset("orig");
+    static const int respidx = conn_type->FieldOffset("resp");
+    static const auto& endpoint_type = zeek::id::find_type<zeek::RecordType>("endpoint");
+    static const int pktidx = endpoint_type->FieldOffset("num_pkts");
+    static const int bytesidx = endpoint_type->FieldOffset("num_bytes_ip");
 
+    auto* orig_endp = conn_val->GetFieldAs<RecordVal>(origidx);
+    auto* resp_endp = conn_val->GetFieldAs<RecordVal>(respidx);
     orig_endp->Assign(pktidx, orig_pkts);
     orig_endp->Assign(bytesidx, orig_bytes);
     resp_endp->Assign(pktidx, resp_pkts);

src/packet_analysis/protocol/tcp/TCPSessionAdapter.cc

@@ -1027,8 +1027,11 @@ void TCPSessionAdapter::FlipRoles() {
 }
 
 void TCPSessionAdapter::UpdateConnVal(RecordVal* conn_val) {
-    auto orig_endp_val = conn_val->GetFieldAs<RecordVal>("orig");
-    auto resp_endp_val = conn_val->GetFieldAs<RecordVal>("resp");
+    static const auto& conn_type = zeek::id::find_type<zeek::RecordType>("connection");
+    static const int origidx = conn_type->FieldOffset("orig");
+    static const int respidx = conn_type->FieldOffset("resp");
+    auto* orig_endp_val = conn_val->GetFieldAs<RecordVal>(origidx);
+    auto* resp_endp_val = conn_val->GetFieldAs<RecordVal>(respidx);
 
     orig_endp_val->Assign(0, orig->Size());
     orig_endp_val->Assign(1, orig->state);

src/zeek-setup.cc

@@ -506,8 +506,6 @@ SetupResult setup(int argc, char** argv, Options* zopts) {
         // If we get here, we're a supervised node that just returned
         // from CreateStem() after being forked from the stem.
         Supervisor::ThisNode()->Init(&options);
-
-        event_mgr.InitPostFork();
     }
 
     script_coverage_mgr.ReadStats();