Merge pull request #51 from Flow-IPC/async_file_logger_improvement2

Prevent unlimited growing of log buffer - Log memory limit

src/flow/log/log.cpp

@@ -18,6 +18,7 @@
 /// @file
 #include "flow/log/log.hpp"
 #include "flow/error/error.hpp"
+#include "flow/util/util_fwd.hpp"
 
 namespace flow::log
 {
@@ -284,4 +285,16 @@ void beautify_chrono_logger_this_thread(Logger* logger_ptr)
   }
 }
 
+size_t deep_size(const Msg_metadata& val)
+{
+  // We're following the loose pattern explained at the end of Async_file_logger::mem_cost().
+
+  using util::deep_size;
+
+  /* Reminder: exclude sizeof(val); include only non-shallow memory used on val's behalf; so
+   * sum of deep_size(X), for each X that is (1) a member of Msg_metadata; and (2) has a deep_size(X) free function.
+   * As of this writing there is just one: */
+  return deep_size(val.m_call_thread_nickname);
+}
+
 } // namespace flow::log

src/flow/log/log.hpp

@@ -1036,6 +1036,9 @@ class Component
  * User only need to worry about this when dealing with the internals of a Logger implementation.  Copying is to be
  * avoided, as there are some non-trivial data stored here; though it is not too bad.
  *
+ * @warning If changing the insides of Msg_metadata, ensure free function `deep_copy(const Msg_metadata&)`
+ *          remains accurate.
+ *
  * @todo Add support in Msg_metadata for a message ID which could more straightforwardly help the human log reader
  * to map a log line to the originating log call site in source code.  One approach, then, might be to output
  * that message ID when available; else output #m_msg_src_file, #m_msg_src_line, #m_msg_src_function; or maybe
@@ -1147,7 +1150,7 @@ struct Msg_metadata
    * thread ID if the thread has a nickname, meaning `!m_call_thread_nickname.empty()`.  The working assumption is
    * that (1) both members are met for direct log output only and no other logic; and (2) the nickname is preferable
    * when set, the thread ID being the fallback.  (If this sounds meh, consider that it's entirely reasonable to make
-   * the nickname contain some nice info *and* the original thread ID as well in string form.  However, might
+   * the nickname contain some nice info *and* the original thread ID as well in string form.  However, mind
    * the length -- the Performance Note in #m_call_thread_nickname doc header.)
    */
   util::Thread_id m_call_thread_id;
@@ -1734,8 +1737,6 @@ class Log_context
 
 // Template implementations.
 
-// Template implementations.
-
 template<typename Payload>
 Component::Component(Payload payload)
 {

src/flow/log/log_fwd.hpp

@@ -377,4 +377,14 @@ void swap(Log_context& val1, Log_context& val2);
  */
 void beautify_chrono_logger_this_thread(Logger* logger_ptr);
 
+/**
+ * Estimate of memory footprint of the given value, including memory allocated on its behalf -- but
+ * excluding its shallow `sizeof`! -- in bytes.
+ *
+ * @param val
+ *        Value.
+ * @return See above.
+ */
+size_t deep_size(const Msg_metadata& val);
+
 } // namespace flow::log

src/flow/util/basic_blob.hpp

@@ -223,6 +223,11 @@ namespace flow::util
  * simply represent the same area of memory; and resize() and co. can turn a non-overlapping range into an overlapping
  * one (encroaching on someone else's "territory" within the pool).
  *
+ * @todo Write a class template, perhaps `Tight_blob<Allocator, bool>`, which would be identical to Basic_blob
+ * but forego the framing features, namely size() and start(), thus storing only the RAII array pointer data()
+ * and capacity(); rewrite Basic_blob in terms of this `Tight_blob`.  This simple container type has had some demand
+ * in practice, and Basic_blob can and should be cleanly built on top of it (perhaps even as an IS-A subclass).
+ *
  * @tparam Allocator
  *         An allocator, with `value_type` equal to our #value_type, per the standard C++1x `Allocator` concept.
  *         In most uses this shall be left at the default `std::allocator<value_type>` which allocates in

src/flow/util/util.cpp

@@ -183,4 +183,22 @@ void beautify_chrono_ostream(std::ostream* os_ptr)
   os << symbol_format;
 }
 
+size_t deep_size(const std::string& val)
+{
+  // We're following the loose pattern explained at the end of Async_file_logger::mem_cost().
+
+#if (!defined(__GNUC__)) || (!defined(__x86_64__))
+#  error "An estimation trick below has only been checked with x64 gcc and clang.  Revisit code for other envs."
+#endif
+
+  /* If it is long enough to not fit inside the std::string object itself
+   * (common optimization in STL: Small String Optimization), then it'll allocate a buffer in heap.
+   * We could even determine whether it actually happened here at runtime, but that wastes cycles
+   * (original use case is in log::Async_file_logger specifically where every cycle counts).
+   * Instead we've established experimentally that with default STL and clangs 4-17 and gccs 5-13
+   * SSO is active for .capacity() <= 15.  @todo Check LLVM libc++ too.  Prob same thing... SSO is well established. */
+  const auto sz = val.capacity();
+  return (sz <= 15) ? 0 : sz;
+}
+
 } // namespace flow::util

src/flow/util/util_fwd.hpp

@@ -918,6 +918,16 @@ Enum istream_to_enum(std::istream* is_ptr, Enum enum_default, Enum enum_sentinel
  */
 void beautify_chrono_ostream(std::ostream* os);
 
+/**
+ * Estimate of memory footprint of the given value, including memory allocated on its behalf -- but
+ * excluding its shallow `sizeof`! -- in bytes.
+ *
+ * @param val
+ *        Value.
+ * @return See above.
+ */
+size_t deep_size(const std::string& val);
+
 // Macros.
 
 /**