From 5a58469862c92716a3d4c1d7368d572d9a88ec69 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Wed, 22 Jan 2020 14:15:33 -0800 Subject: [PATCH] quic: further refinement to StatsDebug PR-URL: https://github.com/nodejs/quic/pull/294 Reviewed-By: Anna Henningsen --- src/quic/node_quic_buffer-inl.h | 4 +-- src/quic/node_quic_http3_application.cc | 1 - src/quic/node_quic_session.cc | 20 +++-------- src/quic/node_quic_session.h | 19 +++++----- src/quic/node_quic_socket.cc | 20 +++-------- src/quic/node_quic_socket.h | 18 +++++----- src/quic/node_quic_stream-inl.h | 2 +- src/quic/node_quic_stream.cc | 22 ++++-------- src/quic/node_quic_stream.h | 25 +++++++------- src/quic/node_quic_util-inl.h | 40 ++++++++++++++++----- src/quic/node_quic_util.h | 46 +++++++++++++++++++------ test/cctest/test_quic_cid.cc | 6 ++-- 12 files changed, 119 insertions(+), 104 deletions(-) diff --git a/src/quic/node_quic_buffer-inl.h b/src/quic/node_quic_buffer-inl.h index 20d5ab5fe0..cc68817205 100644 --- a/src/quic/node_quic_buffer-inl.h +++ b/src/quic/node_quic_buffer-inl.h @@ -54,8 +54,8 @@ void QuicBufferChunk::Done(int status) { QuicBuffer::QuicBuffer(QuicBuffer&& src) noexcept : head_(src.head_), tail_(src.tail_), - length_(src.length_), - ended_(src.ended_) { + ended_(src.ended_), + length_(src.length_) { root_ = std::move(src.root_); src.head_ = nullptr; src.tail_ = nullptr; diff --git a/src/quic/node_quic_http3_application.cc b/src/quic/node_quic_http3_application.cc index 76314c48bf..0cf36c310b 100644 --- a/src/quic/node_quic_http3_application.cc +++ b/src/quic/node_quic_http3_application.cc @@ -82,7 +82,6 @@ MaybeLocal Http3Header::GetName(QuicApplication* app) const { } MaybeLocal Http3Header::GetValue(QuicApplication* app) const { - Environment* env = app->env(); return Http3RcBufferPointer::External::New( static_cast(app), value_); diff --git a/src/quic/node_quic_session.cc b/src/quic/node_quic_session.cc index a8a00a29aa..272b865501 100644 --- a/src/quic/node_quic_session.cc +++ b/src/quic/node_quic_session.cc @@ -1347,9 +1347,6 @@ QuicSession::QuicSession( QuicSession::~QuicSession() { CHECK(!Ngtcp2CallbackScope::InNgtcp2CallbackScope(this)); - StatsDebug stats_debug(this); - Debug(this, "Destroyed. %s", stats_debug.ToString().c_str()); - crypto_context_->Cancel(); connection_.reset(); @@ -1359,18 +1356,11 @@ QuicSession::~QuicSession() { RemoveListener(listener_); } -std::string QuicSession::StatsDebug::ToString() { -#define V(_, name, label) \ - " "## label + ": " + \ - std::to_string(session_->GetStat(&QuicSessionStats::name)) + "\n" - - std::string out = "Statistics:\n"; - out += " Duration: " + - std::to_string(uv_hrtime() - - session_->GetStat(&QuicSessionStats::created_at)) + "\n" + - SESSION_STATS(V); - return out; - +template +void QuicSessionStatsTraits::ToString(const QuicSession& ptr, Fn&& add_field) { +#define V(_n, name, label) \ + add_field(label, ptr.GetStat(&QuicSessionStats::name)); + SESSION_STATS(V) #undef V } diff --git a/src/quic/node_quic_session.h b/src/quic/node_quic_session.h index 91c510968e..9b10ecde67 100644 --- a/src/quic/node_quic_session.h +++ b/src/quic/node_quic_session.h @@ -234,6 +234,14 @@ struct QuicSessionStats { }; #undef V +struct QuicSessionStatsTraits { + using Stats = QuicSessionStats; + using Base = QuicSession; + + template + static void ToString(const QuicSession& ptr, Fn&& add_field); +}; + class QuicSessionListener { public: virtual ~QuicSessionListener(); @@ -613,7 +621,7 @@ class QuicApplication : public MemoryRetainer { // a QuicSession object. class QuicSession : public AsyncWrap, public mem::NgLibMemoryManager, - public StatsBase { + public StatsBase { public: // The default preferred address strategy is to ignore it static void IgnorePreferredAddressStrategy( @@ -1423,15 +1431,6 @@ class QuicSession : public AsyncWrap, static const ngtcp2_conn_callbacks callbacks[2]; - class StatsDebug { - public: - StatsDebug(QuicSession* session) : session_(session) {} - std::string ToString(); - private: - QuicSession* session_; - }; - - friend class QuicCryptoContext; friend class QuicSessionListener; friend class JSQuicSessionListener; diff --git a/src/quic/node_quic_socket.cc b/src/quic/node_quic_socket.cc index ae20d31c01..82fe214bdc 100644 --- a/src/quic/node_quic_socket.cc +++ b/src/quic/node_quic_socket.cc @@ -284,27 +284,17 @@ QuicSocket::QuicSocket( } QuicSocket::~QuicSocket() { - uint64_t now = uv_hrtime(); - StatsDebug stats_debug(this); - Debug(this, "Destroyed. %s", stats_debug.ToString().c_str()); QuicSocketListener* listener = listener_; listener_->OnDestroy(); if (listener == listener_) RemoveListener(listener_); } -std::string QuicSocket::StatsDebug::ToString() { -#define V(_, name, label) \ - " "## label + ": " + \ - std::to_string(socket_->GetStat(&QuicSocketStats::name)) + "\n" - - std::string out = "Statistics:\n"; - out += " Duration: " + - std::to_string(uv_hrtime() - - socket_->GetStat(&QuicSocketStats::created_at)) + "\n" + - SOCKET_STATS(V); - return out; - +template +void QuicSocketStatsTraits::ToString(const QuicSocket& ptr, Fn&& add_field) { +#define V(_n, name, label) \ + add_field(label, ptr.GetStat(&QuicSocketStats::name)); + SOCKET_STATS(V) #undef V } diff --git a/src/quic/node_quic_socket.h b/src/quic/node_quic_socket.h index 9720030db6..aaa7aa91f8 100644 --- a/src/quic/node_quic_socket.h +++ b/src/quic/node_quic_socket.h @@ -70,6 +70,14 @@ struct QuicSocketStats { }; #undef V +struct QuicSocketStatsTraits { + using Stats = QuicSocketStats; + using Base = QuicSocket; + + template + static void ToString(const QuicSocket& ptr, Fn&& add_field); +}; + class QuicSocket; class QuicEndpoint; @@ -244,7 +252,7 @@ class QuicEndpoint : public BaseObject, class QuicSocket : public AsyncWrap, public QuicEndpointListener, public mem::NgLibMemoryManager, - public StatsBase { + public StatsBase { public: static void Initialize( Environment* env, @@ -531,14 +539,6 @@ class QuicSocket : public AsyncWrap, SendWrap* last_created_send_wrap_ = nullptr; - class StatsDebug { - public: - StatsDebug(QuicSocket* socket) : socket_(socket) {} - std::string ToString(); - private: - QuicSocket* socket_; - }; - friend class QuicSocketListener; }; diff --git a/src/quic/node_quic_stream-inl.h b/src/quic/node_quic_stream-inl.h index 1f5d68eba5..6130834b25 100644 --- a/src/quic/node_quic_stream-inl.h +++ b/src/quic/node_quic_stream-inl.h @@ -148,7 +148,7 @@ void QuicStream::Unschedule() { stream_queue_.Remove(); } -std::string QuicHeader::ToString() { +std::string QuicHeader::ToString() const { return name() + " = " + value(); } diff --git a/src/quic/node_quic_stream.cc b/src/quic/node_quic_stream.cc index aff7484cfc..24d9e953e6 100644 --- a/src/quic/node_quic_stream.cc +++ b/src/quic/node_quic_stream.cc @@ -55,23 +55,13 @@ QuicStream::QuicStream( IncrementStat(&QuicStreamStats::max_offset, params.initial_max_data); } -QuicStream::~QuicStream() { - StatsDebug stats_debug(this); - Debug(this, "Destroyed. %s", stats_debug.ToString().c_str()); -} - -std::string QuicStream::StatsDebug::ToString() { -#define V(_, name, label) \ - " "## label + ": " + \ - std::to_string(stream_->GetStat(&QuicStreamStats::name)) + "\n" - - std::string out = "Statistics:\n"; - out += " Duration: " + - std::to_string(uv_hrtime() - - stream_->GetStat(&QuicStreamStats::created_at)) + "\n" + - STREAM_STATS(V); - return out; +QuicStream::~QuicStream() {} +template +void QuicStreamStatsTraits::ToString(const QuicStream& ptr, Fn&& add_field) { +#define V(_n, name, label) \ + add_field(label, ptr.GetStat(&QuicStreamStats::name)); + STREAM_STATS(V) #undef V } diff --git a/src/quic/node_quic_stream.h b/src/quic/node_quic_stream.h index 47e0f078d6..197eef3257 100644 --- a/src/quic/node_quic_stream.h +++ b/src/quic/node_quic_stream.h @@ -18,6 +18,7 @@ namespace node { namespace quic { class QuicSession; +class QuicStream; class QuicApplication; enum QuicStreamHeaderFlags : uint32_t { @@ -65,6 +66,14 @@ struct QuicStreamStats { }; #undef V +struct QuicStreamStatsTraits { + using Stats = QuicStreamStats; + using Base = QuicStream; + + template + static void ToString(const QuicStream& ptr, Fn&& add_field); +}; + // QuicHeader is a base class for implementing QUIC application // specific headers. Each type of QUIC application may have // different internal representations for a header name+value @@ -72,8 +81,7 @@ struct QuicStreamStats { // per stream must create a specialization of the Header class. class QuicHeader : public MemoryRetainer { public: - QuicHeader() {} - + QuicHeader() = default; virtual ~QuicHeader() {} virtual v8::MaybeLocal GetName(QuicApplication* app) const = 0; virtual v8::MaybeLocal GetValue(QuicApplication* app) const = 0; @@ -84,7 +92,7 @@ class QuicHeader : public MemoryRetainer { // (including the name and value) virtual size_t length() const = 0; - inline std::string ToString(); + inline std::string ToString() const; }; enum QuicStreamStates : uint32_t { @@ -198,7 +206,7 @@ enum QuicStreamOrigin { class QuicStream : public AsyncWrap, public bob::SourceImpl, public StreamBase, - public StatsBase { + public StatsBase { public: static void Initialize( Environment* env, @@ -380,15 +388,6 @@ class QuicStream : public AsyncWrap, size_t current_headers_length_ = 0; ListNode stream_queue_; - - class StatsDebug { - public: - StatsDebug(QuicStream* stream) : stream_(stream) {} - std::string ToString(); - private: - QuicStream* stream_; - }; - public: // Linked List of QuicStream objects using Queue = ListHead; diff --git a/src/quic/node_quic_util-inl.h b/src/quic/node_quic_util-inl.h index f4a6f2d527..3511416e9a 100644 --- a/src/quic/node_quic_util-inl.h +++ b/src/quic/node_quic_util-inl.h @@ -1,7 +1,9 @@ #ifndef SRC_QUIC_NODE_QUIC_UTIL_INL_H_ #define SRC_QUIC_NODE_QUIC_UTIL_INL_H_ +#include "debug_utils.h" #include "node_internals.h" +#include "node_quic_crypto.h" #include "node_quic_util.h" #include "memory_tracker-inl.h" #include "env-inl.h" @@ -324,7 +326,7 @@ StatsBase::StatsBase( int options) : stats_buffer_( env->isolate(), - sizeof(T) / sizeof(uint64_t), + sizeof(typename T::Stats) / sizeof(uint64_t), reinterpret_cast(&stats_)) { static constexpr uint64_t kMax = std::numeric_limits::max(); stats_.created_at = uv_hrtime(); @@ -368,30 +370,29 @@ StatsBase::StatsBase( } template -void StatsBase::IncrementStat(uint64_t T::*member, uint64_t amount) { +void StatsBase::IncrementStat(uint64_t T::Stats::*member, uint64_t amount) { static constexpr uint64_t kMax = std::numeric_limits::max(); stats_.*member += std::min(amount, kMax - stats_.*member); } template -void StatsBase::SetStat(uint64_t T::*member, uint64_t value) { +void StatsBase::SetStat(uint64_t T::Stats::*member, uint64_t value) { stats_.*member = value; } template -void StatsBase::RecordTimestamp(uint64_t T::*member) { +void StatsBase::RecordTimestamp(uint64_t T::Stats::*member) { stats_.*member = uv_hrtime(); } template -uint64_t StatsBase::GetStat(uint64_t T::*member) const { +uint64_t StatsBase::GetStat(uint64_t T::Stats::*member) const { return stats_.*member; } template -inline void StatsBase::RecordRate(uint64_t T::*member) { +inline void StatsBase::RecordRate(uint64_t T::Stats::*member) { CHECK(rate_); - uint64_t received_at = GetStat(member); uint64_t now = uv_hrtime(); if (received_at > 0) @@ -406,7 +407,7 @@ inline void StatsBase::RecordSize(uint64_t val) { } template -inline void StatsBase::RecordAck(uint64_t T::*member) { +inline void StatsBase::RecordAck(uint64_t T::Stats::*member) { CHECK(ack_); uint64_t acked_at = GetStat(member); uint64_t now = uv_hrtime(); @@ -423,6 +424,29 @@ void StatsBase::StatsMemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("ack_histogram", ack_); } +template +StatsBase::~StatsBase() { + StatsBase::StatsDebug stats_debug(static_cast(this)); + Debug(static_cast(this), + "Destroyed. %s", + stats_debug.ToString().c_str()); +} + +template +std::string StatsBase::StatsDebug::ToString() const { + std::string out = "Statistics:\n"; + auto add_field = [&out](const char* name, uint64_t val) { + out += " "; + out += std::string(name); + out += ": "; + out += std::to_string(val); + out += "\n"; + }; + add_field("Duration", uv_hrtime() - ptr->GetStat(&T::Stats::created_at)); + T::ToString(*ptr, add_field); + return out; +} + template size_t get_length(const T* vec, size_t count) { CHECK_NOT_NULL(vec); diff --git a/src/quic/node_quic_util.h b/src/quic/node_quic_util.h index 0b06e573f8..dc101242d4 100644 --- a/src/quic/node_quic_util.h +++ b/src/quic/node_quic_util.h @@ -71,8 +71,23 @@ enum QuicErrorFamily : int32_t { QUIC_ERROR_APPLICATION }; +template class StatsBase; + +template +struct StatsTraits { + using Stats = T; + using Base = Q; + + template + static void ToString(const Q& ptr, Fn&& add_field); +}; + // StatsBase is a utility help for classes (like QuicSession) -// that record performance statistics +// that record performance statistics. The template takes a +// single Traits argument (see QuicStreamStatsTraits in +// node_quic_stream.h as an example). When the StatsBase +// is deconstructed, collected statistics are output to +// Debug automatically. template class StatsBase { public: @@ -82,28 +97,38 @@ class StatsBase { SIZE = 2, ACK = 4 }; + inline StatsBase( Environment* env, v8::Local wrap, int options = HistogramOptions::NONE); - protected: + inline ~StatsBase(); + + // The StatsDebug utility is used when StatsBase is destroyed + // to output statistical information. + struct StatsDebug { + typename T::Base* ptr; + explicit StatsDebug(typename T::Base* ptr_) : ptr(ptr_) {} + std::string ToString() const; + }; + // Increments the given stat field by the given amount - inline void IncrementStat(uint64_t T::*member, uint64_t amount = 1); + inline void IncrementStat(uint64_t T::Stats::*member, uint64_t amount = 1); // Sets an entirely new value for the given stat field - inline void SetStat(uint64_t T::*member, uint64_t value); + inline void SetStat(uint64_t T::Stats::*member, uint64_t value); // Sets the given stat field to the current uv_hrtime() - inline void RecordTimestamp(uint64_t T::*member); + inline void RecordTimestamp(uint64_t T::Stats::*member); // Gets the current value of the given stat field - inline uint64_t GetStat(uint64_t T::*member) const; + inline uint64_t GetStat(uint64_t T::Stats::*member) const; // If the rate histogram is used, records the time elapsed // between now and the timestamp specified by the member // field. - inline void RecordRate(uint64_t T::*member); + inline void RecordRate(uint64_t T::Stats::*member); // If the size histogram is used, records the given size. inline void RecordSize(uint64_t val); @@ -111,13 +136,12 @@ class StatsBase { // If the ack rate histogram is used, records the time // elapsed between now and the timestamp specified by // the member field. - inline void RecordAck(uint64_t T::*member); + inline void RecordAck(uint64_t T::Stats::*member); - protected: inline void StatsMemoryInfo(MemoryTracker* tracker) const; private: - T stats_{}; + typename T::Stats stats_{}; BaseObjectPtr rate_; BaseObjectPtr size_; BaseObjectPtr ack_; @@ -336,7 +360,7 @@ class StatelessResetToken : public MemoryRetainer { StatelessResetToken::Compare>; private: - uint8_t buf_[NGTCP2_STATELESS_RESET_TOKENLEN]; + uint8_t buf_[NGTCP2_STATELESS_RESET_TOKENLEN]{}; const uint8_t* token_; }; diff --git a/test/cctest/test_quic_cid.cc b/test/cctest/test_quic_cid.cc index 37e9e00e11..eb1a9c5331 100644 --- a/test/cctest/test_quic_cid.cc +++ b/test/cctest/test_quic_cid.cc @@ -19,13 +19,13 @@ TEST(QuicCID, Simple) { QuicCID qcid1(cid1); CHECK(qcid1); CHECK_EQ(qcid1.length(), 3); - CHECK_EQ(qcid1.ToHex(), "616263"); + CHECK_EQ(qcid1.ToString(), "616263"); QuicCID qcid2(cid2); qcid1 = qcid2; - CHECK_EQ(qcid1.ToHex(), qcid2.ToHex()); + CHECK_EQ(qcid1.ToString(), qcid2.ToString()); qcid1.set_length(5); memset(qcid1.data(), 1, 5); - CHECK_EQ(qcid1.ToHex(), "0101010101"); + CHECK_EQ(qcid1.ToString(), "0101010101"); }