Access Journal::Stream using member functions (RIPD-1087):

Replace Journal public data members with member function accessors
in order to make Journal lighter weight.  The change makes a
Journal cheaper to pass by value.

Also add missing stream checks (e.g., calls to JLOG) to avoid
text processing that ultimately will not be stored in the log.

beast/insight/impl/StatsDCollector.cpp

@@ -326,8 +326,8 @@ public:
 
         if (ec)
         {
-            m_journal.error <<
+            if (auto stream = m_journal.error())
-                "async_send failed: " << ec.message();
+                stream << "async_send failed: " << ec.message();
             return;
         }
     }
@@ -414,8 +414,8 @@ public:
 
         if (ec)
         {
-            m_journal.error <<
+            if (auto stream = m_journal.error())
-                "on_timer failed: " << ec.message();
+                stream << "on_timer failed: " << ec.message();
             return;
         }
 
@@ -435,8 +435,8 @@ public:
 
         if (m_socket.connect (to_endpoint (m_address), ec))
         {
-            m_journal.error <<
+            if (auto stream = m_journal.error())
-                "Connect failed: " << ec.message();
+                stream << "Connect failed: " << ec.message();
             return;
         }
 

beast/threads/impl/Stoppable.cpp

@@ -117,7 +117,8 @@ void Stoppable::stopAsyncRecursive (Journal j)
 
 #ifdef NDEBUG
     if (ms >= 10)
-        j.fatal << m_name << "::onStop took " << ms << "ms";
+        if (auto stream = j.fatal())
+            stream << m_name << "::onStop took " << ms << "ms";
 #else
     (void)ms;
 #endif
@@ -145,7 +146,8 @@ void Stoppable::stopRecursive (Journal j)
     bool const timedOut (! m_stoppedEvent.wait (1 * 1000)); // milliseconds
     if (timedOut)
     {
-        j.error << "Waiting for '" << m_name << "' to stop";
+        if (auto stream = j.error())
+            stream << "Waiting for '" << m_name << "' to stop";
         m_stoppedEvent.wait ();
     }
 
@@ -193,7 +195,8 @@ void RootStoppable::stop (Journal j)
         std::lock_guard<std::mutex> lock(m_);
         if (m_calledStop)
         {
-            j.warning << "Stoppable::stop called again";
+            if (auto stream = j.warn())
+                stream << "Stoppable::stop called again";
             return;
         }
         m_calledStop = true;

beast/utility/Journal.h

@@ -20,10 +20,31 @@
 #ifndef BEAST_UTILITY_JOURNAL_H_INCLUDED
 #define BEAST_UTILITY_JOURNAL_H_INCLUDED
 
+#include <cassert>
 #include <sstream>
 
 namespace beast {
 
+/** A namespace for easy access to logging severity values. */
+namespace severities
+{
+    /** Severity level / threshold of a Journal message. */
+    enum Severity
+    {
+        kAll = 0,
+
+        kTrace = kAll,
+        kDebug,
+        kInfo,
+        kWarning,
+        kError,
+        kFatal,
+
+        kDisabled,
+        kNone = kDisabled
+    };
+}
+
 /** A generic endpoint for log messages.
 
     The Journal has a few simple goals:
@@ -39,26 +60,13 @@ namespace beast {
 class Journal
 {
 public:
-    /** Severity level of the message. */
-    enum Severity
-    {
-        kAll = 0,
-
-        kTrace = kAll,
-        kDebug,
-        kInfo,
-        kWarning,
-        kError,
-        kFatal,
-
-        kDisabled,
-        kNone = kDisabled
-    };
-
     class Sink;
 
 private:
-    Sink* m_sink;
+    // Severity level / threshold of a Journal message.
+    using Severity = severities::Severity;
+
+    Sink& m_sink;
 
 public:
     //--------------------------------------------------------------------------
@@ -70,6 +78,7 @@ public:
         Sink () = delete;
         explicit Sink(Sink const& sink) = default;
         Sink (Severity thresh, bool console);
+        Sink& operator= (Sink const& lhs) = delete;
 
     public:
         virtual ~Sink () = 0;
@@ -100,6 +109,13 @@ public:
         bool m_console;
     };
 
+static_assert(std::is_default_constructible<Sink>::value == false, "");
+static_assert(std::is_copy_constructible<Sink>::value == false, "");
+static_assert(std::is_move_constructible<Sink>::value == false, "");
+static_assert(std::is_copy_assignable<Sink>::value == false, "");
+static_assert(std::is_move_assignable<Sink>::value == false, "");
+static_assert(std::is_nothrow_destructible<Sink>::value == true, "");
+
     /** Returns a Sink which does nothing. */
     static Sink& getNullSink ();
 
@@ -107,76 +123,98 @@ public:
 
     class Stream;
 
-    /** Scoped ostream-based container for writing messages to a Journal. */
+private:
+    /* Scoped ostream-based container for writing messages to a Journal. */
     class ScopedStream
     {
     public:
-        explicit ScopedStream (Stream const& stream);
+        ScopedStream (ScopedStream const& other)
-        ScopedStream (ScopedStream const& other);
+            : ScopedStream (other.m_sink, other.m_level)
+        { }
+
+        ScopedStream (Sink& sink, Severity level);
 
         template <typename T>
-        ScopedStream (Stream const& stream, T const& t)
+        ScopedStream (Stream const& stream, T const& t);
-            : m_sink (stream.sink())
-            , m_level (stream.severity())
-        {
-            m_ostream << t;
-        }
 
-        ScopedStream (Stream const& stream,
+        ScopedStream (
-            std::ostream& manip (std::ostream&));
+            Stream const& stream, std::ostream& manip (std::ostream&));
+
+        ScopedStream& operator= (ScopedStream const&) = delete;
 
         ~ScopedStream ();
 
-        std::ostringstream& ostream () const;
+        std::ostringstream& ostream () const
+        {
+            return m_ostream;
+        }
 
         std::ostream& operator<< (
             std::ostream& manip (std::ostream&)) const;
 
         template <typename T>
-        std::ostream& operator<< (T const& t) const
+        std::ostream& operator<< (T const& t) const;
-        {
-            m_ostream << t;
-            return m_ostream;
-        }
 
     private:
-        void init ();
-
-        ScopedStream& operator= (ScopedStream const&); // disallowed
-
         Sink& m_sink;
-        Severity const m_level; // cached from Stream for call to m_sink.write
+        Severity const m_level;
         std::ostringstream mutable m_ostream;
     };
 
-    //--------------------------------------------------------------------------
+static_assert(std::is_default_constructible<ScopedStream>::value == false, "");
+static_assert(std::is_copy_constructible<ScopedStream>::value == true, "");
+static_assert(std::is_move_constructible<ScopedStream>::value == true, "");
+static_assert(std::is_copy_assignable<ScopedStream>::value == false, "");
+static_assert(std::is_move_assignable<ScopedStream>::value == false, "");
+static_assert(std::is_nothrow_destructible<ScopedStream>::value == true, "");
 
+    //--------------------------------------------------------------------------
+public:
+    /** Provide a light-weight way to check active() before string formatting */
     class Stream
     {
     public:
         /** Create a stream which produces no output. */
-        Stream ();
+        Stream ()
+        : m_sink (getNullSink())
+        , m_level (severities::kDisabled)
+        { }
 
-        /** Create stream that writes at the given level. */
+        /** Create stream that writes at the given level.
-        Stream (Sink& sink, Severity level);
+
+            Constructor is inlined so checking active() very inexpensive.
+        */
+        Stream (Sink& sink, Severity level)
+            : m_sink (sink)
+            , m_level (level)
+        {
+            assert (m_level < severities::kDisabled);
+        }
 
         /** Construct or copy another Stream. */
-        /** @{ */
+        Stream (Stream const& other)
-        Stream (Stream const& other);
+            : Stream (other.m_sink, other.m_level)
-        Stream& operator= (Stream const& other);
+        {  }
-        /** @} */
+
+        Stream& operator= (Stream const& other) = delete;
 
         /** Returns the Sink that this Stream writes to. */
-        Sink& sink() const;
+        Sink& sink() const
+        {
+            return m_sink;
+        }
 
-        /** Returns the Severity of messages this Stream reports. */
+        /** Returns the Severity level of messages this Stream reports. */
-        Severity severity() const;
+        Severity level() const
+        {
+            return m_level;
+        }
 
-        /** Returns `true` if sink logs anything at this stream's severity. */
+        /** Returns `true` if sink logs anything at this stream's level. */
         /** @{ */
         bool active() const
         {
-            return m_sink->active (m_level);
+            return m_sink.active (m_level);
         }
 
         explicit
@@ -191,55 +229,132 @@ public:
         ScopedStream operator<< (std::ostream& manip (std::ostream&)) const;
 
         template <typename T>
-        ScopedStream operator<< (T const& t) const
+        ScopedStream operator<< (T const& t) const;
-        {
-            return ScopedStream (*this, t);
-        }
         /** @} */
 
     private:
-        Sink* m_sink;
+        Sink& m_sink;
         Severity m_level;
     };
 
+static_assert(std::is_default_constructible<Stream>::value == true, "");
+static_assert(std::is_copy_constructible<Stream>::value == true, "");
+static_assert(std::is_move_constructible<Stream>::value == true, "");
+static_assert(std::is_copy_assignable<Stream>::value == false, "");
+static_assert(std::is_move_assignable<Stream>::value == false, "");
+static_assert(std::is_nothrow_destructible<Stream>::value == true, "");
+
     //--------------------------------------------------------------------------
 
     /** Create a journal that writes to the null sink. */
-    Journal ();
+    Journal ()
+    : Journal (getNullSink())
+    { }
 
     /** Create a journal that writes to the specified sink. */
-    explicit Journal (Sink& sink);
+    explicit Journal (Sink& sink)
+    : m_sink (sink)
+    { }
 
     /** Create a journal from another journal. */
-    Journal (Journal const& other);
+    Journal (Journal const& other)
+    : Journal (other.m_sink)
+    { }
 
     // Disallowed.
     Journal& operator= (Journal const& other) = delete;
 
     /** Destroy the journal. */
-    ~Journal ();
+    ~Journal () = default;
 
     /** Returns the Sink associated with this Journal. */
-    Sink& sink() const;
+    Sink& sink() const
+    {
+        return m_sink;
+    }
 
-    /** Returns a stream for this sink, with the specified severity. */
+    /** Returns a stream for this sink, with the specified severity level. */
-    Stream stream (Severity level) const;
+    Stream stream (Severity level) const
+    {
+        return Stream (m_sink, level);
+    }
 
     /** Returns `true` if any message would be logged at this severity level.
         For a message to be logged, the severity must be at or above the
         sink's severity threshold.
     */
-    bool active (Severity level) const;
+    bool active (Severity level) const
+    {
+        return m_sink.active (level);
+    }
 
-    /** Convenience sink streams for each severity level. */
+    /** Severity stream access functions. */
-    Stream const trace;
+    /** @{ */
-    Stream const debug;
+    Stream trace() const
-    Stream const info;
+    {
-    Stream const warning;
+        return { m_sink, severities::kTrace };
-    Stream const error;
+    }
-    Stream const fatal;
+
+    Stream debug() const
+    {
+        return { m_sink, severities::kDebug };
+    }
+
+    Stream info() const
+    {
+        return { m_sink, severities::kInfo };
+    }
+
+    Stream warn() const
+    {
+        return { m_sink, severities::kWarning };
+    }
+
+    Stream error() const
+    {
+        return { m_sink, severities::kError };
+    }
+
+    Stream fatal() const
+    {
+        return { m_sink, severities::kFatal };
+    }
+    /** @} */
 };
 
+static_assert(std::is_default_constructible<Journal>::value == true, "");
+static_assert(std::is_copy_constructible<Journal>::value == true, "");
+static_assert(std::is_move_constructible<Journal>::value == true, "");
+static_assert(std::is_copy_assignable<Journal>::value == false, "");
+static_assert(std::is_move_assignable<Journal>::value == false, "");
+static_assert(std::is_nothrow_destructible<Journal>::value == true, "");
+
+//------------------------------------------------------------------------------
+
+template <typename T>
+Journal::ScopedStream::ScopedStream (Journal::Stream const& stream, T const& t)
+   : ScopedStream (stream.sink(), stream.level())
+{
+    m_ostream << t;
 }
 
+template <typename T>
+std::ostream&
+Journal::ScopedStream::operator<< (T const& t) const
+{
+    m_ostream << t;
+    return m_ostream;
+}
+
+//------------------------------------------------------------------------------
+
+template <typename T>
+Journal::ScopedStream
+Journal::Stream::operator<< (T const& t) const
+{
+    return ScopedStream (*this, t);
+}
+
+} // beast
+
 #endif

beast/utility/WrappedSink.h

@@ -57,7 +57,7 @@ public:
     }
 
     bool
-    active (beast::Journal::Severity level) const override
+    active (beast::severities::Severity level) const override
     {
         return sink_.active (level);
     }
@@ -73,18 +73,18 @@ public:
         sink_.console (output);
     }
 
-    beast::Journal::Severity
+    beast::severities::Severity
     threshold() const override
     {
         return sink_.threshold();
     }
 
-    void threshold (beast::Journal::Severity thresh) override
+    void threshold (beast::severities::Severity thresh) override
     {
         sink_.threshold (thresh);
     }
 
-    void write (beast::Journal::Severity level, std::string const& text) override
+    void write (beast::severities::Severity level, std::string const& text) override
     {
         using beast::Journal;
         sink_.write (level, prefix_ + text);

beast/utility/impl/Journal.cpp

@@ -29,12 +29,12 @@ class NullJournalSink : public Journal::Sink
 {
 public:
     NullJournalSink ()
-    : Sink (Journal::kDisabled, false)
+    : Sink (severities::kDisabled, false)
     { }
 
     ~NullJournalSink() override = default;
 
-    bool active (Journal::Severity) const override
+    bool active (severities::Severity) const override
     {
         return false;
     }
@@ -48,16 +48,16 @@ public:
     {
     }
 
-    Journal::Severity threshold() const override
+    severities::Severity threshold() const override
     {
-        return Journal::kDisabled;
+        return severities::kDisabled;
     }
 
-    void threshold (Journal::Severity) override
+    void threshold (severities::Severity) override
     {
     }
 
-    void write (Journal::Severity, std::string const&) override
+    void write (severities::Severity, std::string const&) override
     {
     }
 };
@@ -97,7 +97,7 @@ void Journal::Sink::console (bool output)
     m_console = output;
 }
 
-Journal::Severity Journal::Sink::threshold () const
+severities::Severity Journal::Sink::threshold () const
 {
     return thresh_;
 }
@@ -109,26 +109,20 @@ void Journal::Sink::threshold (Severity thresh)
 
 //------------------------------------------------------------------------------
 
-Journal::ScopedStream::ScopedStream (Stream const& stream)
+Journal::ScopedStream::ScopedStream (Sink& sink, Severity level)
-    : m_sink (stream.sink ())
+    : m_sink (sink)
-    , m_level (stream.severity ())
+    , m_level (level)
 {
-    init ();
+    // Modifiers applied from all ctors
-}
+    m_ostream
-
+        << std::boolalpha
-Journal::ScopedStream::ScopedStream (ScopedStream const& other)
+        << std::showbase;
-    : m_sink (other.m_sink)
-    , m_level (other.m_level)
-{
-    init ();
 }
 
 Journal::ScopedStream::ScopedStream (
     Stream const& stream, std::ostream& manip (std::ostream&))
-    : m_sink (stream.sink ())
+    : ScopedStream (stream.sink(), stream.level())
-    , m_level (stream.severity ())
 {
-    init ();
     m_ostream << manip;
 }
 
@@ -144,114 +138,18 @@ Journal::ScopedStream::~ScopedStream ()
     }
 }
 
-void Journal::ScopedStream::init ()
-{
-    // Modifiers applied from all ctors
-    m_ostream
-        << std::boolalpha
-        << std::showbase;
-}
-
 std::ostream& Journal::ScopedStream::operator<< (std::ostream& manip (std::ostream&)) const
 {
     return m_ostream << manip;
 }
 
-std::ostringstream& Journal::ScopedStream::ostream () const
-{
-    return m_ostream;
-}
-
 //------------------------------------------------------------------------------
 
-Journal::Stream::Stream ()
-    : m_sink (&getNullSink ())
-    , m_level (kDisabled)
-{
-}
-
-Journal::Stream::Stream (Sink& sink, Severity level)
-    : m_sink (&sink)
-    , m_level (level)
-{
-    assert (level != kDisabled);
-}
-
-Journal::Stream::Stream (Stream const& other)
-    : m_sink (other.m_sink)
-    , m_level (other.m_level)
-{
-}
-
-Journal::Sink& Journal::Stream::sink () const
-{
-    return *m_sink;
-}
-
-Journal::Severity Journal::Stream::severity () const
-{
-    return m_level;
-}
-
-Journal::Stream& Journal::Stream::operator= (Stream const& other)
-{
-    m_sink = other.m_sink;
-    m_level = other.m_level;
-    return *this;
-}
-
 Journal::ScopedStream Journal::Stream::operator<< (
     std::ostream& manip (std::ostream&)) const
 {
     return ScopedStream (*this, manip);
 }
 
-//------------------------------------------------------------------------------
+} // beast
 
-Journal::Journal ()
-    : m_sink  (&getNullSink())
-    , trace   (stream (kTrace))
-    , debug   (stream (kDebug))
-    , info    (stream (kInfo))
-    , warning (stream (kWarning))
-    , error   (stream (kError))
-    , fatal   (stream (kFatal))
-{
-}
-
-Journal::Journal (Sink& sink)
-    : m_sink  (&sink)
-    , trace   (stream (kTrace))
-    , debug   (stream (kDebug))
-    , info    (stream (kInfo))
-    , warning (stream (kWarning))
-    , error   (stream (kError))
-    , fatal   (stream (kFatal))
-{
-}
-
-Journal::Journal (Journal const& other)
-    : Journal  (*other.m_sink)
-{
-}
-
-Journal::~Journal ()
-{
-}
-
-Journal::Sink& Journal::sink() const
-{
-    return *m_sink;
-}
-
-Journal::Stream Journal::stream (Severity level) const
-{
-    return Stream (*m_sink, level);
-}
-
-bool Journal::active (Severity level) const
-{
-    return m_sink->active (level);
-}
-
-}

beast/utility/tests/Journal.test.cpp

@@ -32,7 +32,7 @@ public:
 
     public:
         TestSink()
-            : Sink (Journal::kWarning, false)
+            : Sink (severities::kWarning, false)
             , m_count(0)
         {
         }
@@ -50,7 +50,7 @@ public:
         }
 
         void
-        write (Journal::Severity level, std::string const&) override
+        write (severities::Severity level, std::string const&) override
         {
             if (level >= threshold())
                 ++m_count;
@@ -61,38 +61,39 @@ public:
     {
         TestSink sink;
 
-        sink.threshold(Journal::kInfo);
+        using namespace beast::severities;
+        sink.threshold(kInfo);
 
         Journal j(sink);
 
-        j.trace << " ";
+        j.trace() << " ";
         expect(sink.count() == 0);
-        j.debug << " ";
+        j.debug() << " ";
         expect(sink.count() == 0);
-        j.info << " ";
+        j.info() << " ";
         expect(sink.count() == 1);
-        j.warning << " ";
+        j.warn() << " ";
         expect(sink.count() == 2);
-        j.error << " ";
+        j.error() << " ";
         expect(sink.count() == 3);
-        j.fatal << " ";
+        j.fatal() << " ";
         expect(sink.count() == 4);
 
         sink.reset();
 
-        sink.threshold(Journal::kDebug);
+        sink.threshold(kDebug);
 
-        j.trace << " ";
+        j.trace() << " ";
         expect(sink.count() == 0);
-        j.debug << " ";
+        j.debug() << " ";
         expect(sink.count() == 1);
-        j.info << " ";
+        j.info() << " ";
         expect(sink.count() == 2);
-        j.warning << " ";
+        j.warn() << " ";
         expect(sink.count() == 3);
-        j.error << " ";
+        j.error() << " ";
         expect(sink.count() == 4);
-        j.fatal << " ";
+        j.fatal() << " ";
         expect(sink.count() == 5);
     }
 };