JException has a JBacktrace instead of a string

Previously, JException would stringify the JBacktrace in the constructor. This is fine as long as JException represents an unrecoverable error. If used for flow control, however, this imposes a steep performance penalty. The penalty will become even steeper when we enable `addr2line`. Now the JException only stringifies the backtrace upon printing, and only if the backtrace is requested.
JeffersonLab · Dec 12, 2024 · 04bc4bf · 04bc4bf
1 parent 7cb8655
commit 04bc4bf
Show file tree

Hide file tree

Showing 5 changed files with 63 additions and 34 deletions.
diff --git a/src/libraries/JANA/Engine/JExecutionEngine.cc b/src/libraries/JANA/Engine/JExecutionEngine.cc
@@ -317,7 +317,7 @@ void JExecutionEngine::HandleFailures() {
         if (worker->is_timed_out) {
             GetApplication()->SetExitCode((int) JApplication::ExitCode::Timeout);
             auto ex = JException("Timeout in worker thread");
-            ex.stacktrace = worker->backtrace.ToString();
+            ex.backtrace = worker->backtrace;
             throw ex;
         }
     }

diff --git a/src/libraries/JANA/JException.h b/src/libraries/JANA/JException.h
@@ -15,11 +15,8 @@ struct JException : public std::exception {
 public:
 
     /// Basic constructor
-    explicit JException(std::string message = "Unknown exception") : message(std::move(message))
-    {
-        JBacktrace backtrace;
+    explicit JException(std::string message = "Unknown exception") : message(std::move(message)) {
         backtrace.Capture(2);
-        stacktrace = backtrace.ToString();
     }
 
     virtual ~JException() = default;
@@ -35,7 +32,7 @@ struct JException : public std::exception {
     }
 
     std::string GetStackTrace() {
-        return stacktrace;
+        return backtrace.ToString();
     }
 
     const char* what() const noexcept {
@@ -63,8 +60,10 @@ struct JException : public std::exception {
         if (ex.plugin_name.length() != 0) {
             os << "  Plugin:   " << ex.plugin_name << std::endl;
         }
-        if (ex.stacktrace.length() != 0 && ex.show_stacktrace) {
-            os << "  Backtrace:" << std::endl << std::endl << ex.stacktrace;
+        if (ex.show_stacktrace) {
+            ex.backtrace.WaitForCapture();
+            ex.backtrace.ToString();
+            os << "  Backtrace:" << std::endl << std::endl << ex.backtrace.ToString();
         }
         return os;
     }
@@ -75,7 +74,7 @@ struct JException : public std::exception {
     std::string type_name;
     std::string function_name;
     std::string instance_name;
-    std::string stacktrace;
+    JBacktrace backtrace;
     std::exception_ptr nested_exception;
     bool show_stacktrace=true;
 
@@ -89,10 +88,7 @@ JException::JException(std::string format_str, Args... args) {
     char cmess[1024];
     snprintf(cmess, 1024, format_str.c_str(), args...);
     message = cmess;
-
-    JBacktrace backtrace;
     backtrace.Capture(2);
-    stacktrace = backtrace.ToString();
 }
 
 
diff --git a/src/libraries/JANA/Utils/JBacktrace.cc b/src/libraries/JANA/Utils/JBacktrace.cc
@@ -15,6 +15,28 @@
 #include <iomanip>
 
 
+JBacktrace::JBacktrace(const JBacktrace& other) {
+    m_ready = other.m_ready.load();
+    m_frame_count = other.m_frame_count;
+    m_frames_to_omit = other.m_frames_to_omit;
+    m_buffer = other.m_buffer;
+}
+
+JBacktrace& JBacktrace::operator=(const JBacktrace& other) {
+    m_ready = other.m_ready.load();
+    m_frame_count = other.m_frame_count;
+    m_frames_to_omit = other.m_frames_to_omit;
+    m_buffer = other.m_buffer;
+    return *this;
+}
+
+JBacktrace::JBacktrace(JBacktrace&& other) {
+    m_ready = other.m_ready.load();
+    m_frame_count = other.m_frame_count;
+    m_frames_to_omit = other.m_frames_to_omit;
+    m_buffer = std::move(other.m_buffer);
+}
+
 void JBacktrace::Reset() {
     m_ready = false;
 }
@@ -26,13 +48,13 @@ void JBacktrace::WaitForCapture() const {
 }
 
 void JBacktrace::Capture(int frames_to_omit) {
-    m_frame_count = backtrace(m_buffer, MAX_FRAMES);
+    m_frame_count = backtrace(m_buffer.data(), MAX_FRAMES);
     m_frames_to_omit = frames_to_omit;
     m_ready.store(true, std::memory_order_release);
 }
 
-void JBacktrace::Format(std::ostream& os) {
-    char** symbols = backtrace_symbols(m_buffer, m_frame_count);
+void JBacktrace::Format(std::ostream& os) const {
+    char** symbols = backtrace_symbols(m_buffer.data(), m_frame_count);
     // Skip the first few frames, which are inevitably signal handlers, JBacktrace ctors, or JException ctors
     for (int i=m_frames_to_omit; i<m_frame_count; ++i) {
 
@@ -68,7 +90,7 @@ void JBacktrace::Format(std::ostream& os) {
     free(symbols);
 }
 
-std::string JBacktrace::AddrToLineInfo(const char* filename, size_t offset) {
+std::string JBacktrace::AddrToLineInfo(const char* filename, size_t offset) const {
 
     char backup_line_info[256];
     std::snprintf(backup_line_info, sizeof(backup_line_info), "%s:%zx\n", filename, offset);
@@ -102,7 +124,7 @@ std::string JBacktrace::AddrToLineInfo(const char* filename, size_t offset) {
     return std::string(backup_line_info);
 }
 
-std::string JBacktrace::ToString() {
+std::string JBacktrace::ToString() const {
     std::ostringstream oss;
     Format(oss);
     return oss.str();

diff --git a/src/libraries/JANA/Utils/JBacktrace.h b/src/libraries/JANA/Utils/JBacktrace.h
@@ -7,22 +7,30 @@
 #include <ostream>
 #include <string>
 #include <atomic>
+#include <array>
 
 
 class JBacktrace {
     static const int MAX_FRAMES = 100;
-    void* m_buffer[MAX_FRAMES];
+    std::array<void*, MAX_FRAMES> m_buffer;
     int m_frame_count = 0;
     int m_frames_to_omit = 0;
     std::atomic_bool m_ready {false};
 
 public:
+
+    JBacktrace() = default;
+    ~JBacktrace() = default;
+    JBacktrace(const JBacktrace& other);
+    JBacktrace(JBacktrace&& other);
+    JBacktrace& operator=(const JBacktrace&);
+
     void WaitForCapture() const;
     void Capture(int frames_to_omit=0);
     void Reset();
-    std::string ToString();
-    void Format(std::ostream& os);
-    std::string AddrToLineInfo(const char* filename, size_t offset);
+    std::string ToString() const;
+    void Format(std::ostream& os) const;
+    std::string AddrToLineInfo(const char* filename, size_t offset) const;
 
 };
 

diff --git a/src/plugins/JTest/JTestTracker.h b/src/plugins/JTest/JTestTracker.h
@@ -13,8 +13,9 @@
 
 class JTestTracker : public JFactoryT<JTestTrackData> {
 
+    Parameter<bool> m_except {this, "except", false, "Event #7 always excepts" };
     Parameter<bool> m_segfault {this, "segfault", false, "Event #7 always segfaults" };
-    Parameter<bool> m_timeout {this, "timeout", false, "Event #22 always times out" };
+    Parameter<bool> m_timeout {this, "timeout", false, "Event #7 always times out" };
     Parameter<size_t> m_cputime_ms {this, "cputime_ms", 200, "Time spent during tracking" };
     Parameter<size_t> m_write_bytes {this, "bytes", 1000, "Bytes written during tracking"};
     Parameter<double> m_cputime_spread {this, "cputime_spread", 0.25, "Spread of time spent during tracking"};
@@ -36,21 +37,23 @@ class JTestTracker : public JFactoryT<JTestTrackData> {
         m_bench_utils.read_memory(ed->buffer);
 
         // Do lots of computation
-        if (*m_timeout) {
-            if (aEvent->GetEventNumber() == 22) {
+        m_bench_utils.consume_cpu_ms(*m_cputime_ms, *m_cputime_spread);
+
+        // Optionally trigger failure scenarios
+        if(aEvent->GetEventNumber() == 7) {
+            // Only one of these can happen, so in principle the param should be an enum
+            if (*m_except) {
+                throw std::runtime_error("Something went wrong");
+            }
+            if (*m_segfault) {
+                // Trigger a segfault on purpose
+                JTestTrackData* d = nullptr;
+                d->buffer[0] = 22;
+            }
+            if (*m_timeout) {
                 m_bench_utils.consume_cpu_ms(1000000);
             }
         }
-        else {
-            m_bench_utils.consume_cpu_ms(*m_cputime_ms, *m_cputime_spread);
-        }
-
-
-        if(*m_segfault && aEvent->GetEventNumber() == 7) {
-            // Trigger a segfault on purpose
-            JTestTrackData* d = nullptr;
-            d->buffer[0] = 22;
-        }
 
         // Write (small) track data
         auto td = new JTestTrackData;