Merge pull request #1556 in SNORT/snort3 from ~RUCOMBS/snort3:profile_short_circuit...

Squashed commit of the following:

commit d2741170286a40b9455cbf3933938c6b05215e42
Author: russ <rucombs@cisco.com>
Date:   Tue Mar 19 19:24:05 2019 -0400

    profiler: add quick exit if not configured to minimize overhead

diff --git a/src/ips_options/test/ips_regex_test.cc b/src/ips_options/test/ips_regex_test.cc
index c5b15f676efb64fdba9b0a394ade2b2ef8564353..8ea290f752c6662b3d26dd08c7a32f5ad3da59a5 100644 (file)
--- a/src/ips_options/test/ips_regex_test.cc
+++ b/src/ips_options/test/ips_regex_test.cc
@@ -28,7 +28,7 @@
 #include "framework/ips_option.h"
 #include "framework/module.h"
 #include "main/snort_config.h"
-#include "profiler/memory_profiler_defs.h"
+#include "profiler/profiler_defs.h"
 #include "protocols/packet.h"
 
 // must appear after snort_config.h to avoid broken c++ map include
@@ -91,6 +91,8 @@ char* snort_strdup(const char* s)
 
 MemoryContext::MemoryContext(MemoryTracker&) { }
 MemoryContext::~MemoryContext() = default;
+
+bool TimeProfilerStats::enabled = false;
 }
 
 extern const BaseApi* ips_regex;

diff --git a/src/main/modules.cc b/src/main/modules.cc
index 766600f893d4026d84d102e449cdc990d9ca9923..b4cc4602731fe2c857ff3afc1a5a3a63ce8f770d 100755 (executable)
--- a/src/main/modules.cc
+++ b/src/main/modules.cc
@@ -484,7 +484,9 @@ class ProfilerModule : public Module
 {
 public:
     ProfilerModule() : Module("profiler", profiler_help, profiler_params) { }
+
     bool set(const char*, Value&, SnortConfig*) override;
+    bool end(const char*, int, SnortConfig*) override;
 
     Usage get_usage() const override
     { return GLOBAL; }
@@ -508,6 +510,12 @@ bool ProfilerModule::set(const char* fqn, Value& v, SnortConfig* sc)
     return false;
 }
 
+bool ProfilerModule::end(const char*, int, SnortConfig* sc)
+{
+    TimeProfilerStats::set_enabled(sc->profiler->time.show);
+    return true;
+}
+
 //-------------------------------------------------------------------------
 // classification module
 //-------------------------------------------------------------------------

diff --git a/src/main/snort_config.h b/src/main/snort_config.h
index 9420729a79c845b05e05268dc814529d0c5c903c..e95f3fc1489064b89db747b90f2e93fb232a6c02 100644 (file)
--- a/src/main/snort_config.h
+++ b/src/main/snort_config.h
@@ -357,7 +357,6 @@ public:
 
     //------------------------------------------------------
     ProfilerConfig* profiler = nullptr;
-
     LatencyConfig* latency = nullptr;
 
     unsigned remote_control_port = 0;

diff --git a/src/profiler/time_profiler.cc b/src/profiler/time_profiler.cc
index fd7ea3c4adc9a5305e0c66e7764527b9ef919683..d09d372330a62ff97dcbe61e1e431e4804b1982c 100644 (file)
--- a/src/profiler/time_profiler.cc
+++ b/src/profiler/time_profiler.cc
@@ -38,6 +38,11 @@ using namespace snort;
 
 #define s_time_table_title "module profile"
 
+// enabled is not in SnortConfig to avoid that ugly dependency
+// enabled is not in TimeContext because declaring it SO_PUBLIC made TimeContext visible
+// putting enabled in TimeProfilerStats seems to be the best solution
+bool TimeProfilerStats::enabled = false;
+
 namespace time_stats
 {
 
@@ -463,10 +468,72 @@ TEST_CASE( "time profiler sorting", "[profiler][time_profiler]" )
     }
 }
 
+TEST_CASE( "time profiler time context disabled", "[profiler][time_profiler]" )
+{
+    TimeProfilerStats stats;
+    REQUIRE_FALSE( stats );
+    TimeProfilerStats::set_enabled(false);
+
+    SECTION( "lifetime" )
+    {
+        {
+            TimeContext ctx(stats);
+            CHECK( ctx.active() );
+            CHECK( stats.ref_count == 0 );
+        }
+
+        CHECK( stats.ref_count == 0 );
+    }
+
+    SECTION( "manually managed lifetime" )
+    {
+        {
+            TimeContext ctx(stats);
+            CHECK( ctx.active() );
+            CHECK( stats.ref_count == 0 );
+            ctx.stop();
+            CHECK( ctx.active() );
+            CHECK( stats.ref_count == 0 );
+        }
+
+        CHECK( stats.ref_count == 0 );
+    }
+
+    SECTION( "updates stats" )
+    {
+        TimeContext ctx(stats);
+        avoid_optimization();
+        ctx.stop();
+
+        CHECK( !stats );
+    }
+
+    SECTION( "reentrance" )
+    {
+        {
+            TimeContext ctx1(stats);
+
+            CHECK( stats.ref_count == 0 );
+
+            {
+                TimeContext ctx2(stats);
+
+                CHECK( (stats.ref_count == 0) );
+            }
+
+            CHECK( stats.ref_count == 0 );
+        }
+
+        CHECK( stats.ref_count == 0 ); // ref_count restored
+        CHECK( stats.checks == 0 ); // only updated once
+    }
+}
+
 TEST_CASE( "time profiler time context", "[profiler][time_profiler]" )
 {
     TimeProfilerStats stats;
     REQUIRE_FALSE( stats );
+    TimeProfilerStats::set_enabled(true);
 
     SECTION( "lifetime" )
     {

diff --git a/src/profiler/time_profiler_defs.h b/src/profiler/time_profiler_defs.h
index d0adbeaaa2950de7cbcc714e8f6b7f32b46fcef3..7e810ef13f5ddc5f41215dd64ef9c58b0aed71bf 100644 (file)
--- a/src/profiler/time_profiler_defs.h
+++ b/src/profiler/time_profiler_defs.h
@@ -47,6 +47,13 @@ struct SO_PUBLIC TimeProfilerStats
     hr_duration elapsed;
     uint64_t checks;
     mutable unsigned int ref_count;
+    static bool enabled;
+
+    static void set_enabled(bool b)
+    { enabled = b; }
+
+    static bool is_enabled()
+    { return enabled; }
 
     void update(hr_duration delta)
     { elapsed += delta; ++checks; }
@@ -90,17 +97,20 @@ public:
     TimeContext(TimeProfilerStats& stats) :
         stats(stats)
     {
-        if ( stats.enter() )
+        if ( stats.is_enabled() and stats.enter() )
             sw.start();
     }
 
     ~TimeContext()
-    { stop(); }
+    {
+        if ( stats.is_enabled() )
+            stop();
+    }
 
     // Use this for finer grained control of the TimeContext "lifetime"
     void stop()
     {
-        if ( stopped_once )
+        if ( !stats.is_enabled() or stopped_once )
             return; // stop() should only be executed once per context
 
         stopped_once = true;
@@ -130,6 +140,8 @@ public:
 
     ~TimeExclude()
     {
+        if ( !TimeProfilerStats::is_enabled() )
+            return;
         ctx.stop();
         stats.elapsed -= tmp.elapsed;
     }