From 63d1edd0f21d72405a7221cb51d659685bc395ca Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 12 Jan 2023 09:35:39 -0800 Subject: [PATCH] [hpack] Add stat for received hpack metadata size (#32062) * [hpack] Add stat for received hpack metadata size * Automated change: Fix sanity tests Co-authored-by: ctiller --- BUILD | 2 + .../chttp2/transport/hpack_parser.cc | 3 + src/core/lib/debug/stats_data.cc | 67 ++++++++++--------- src/core/lib/debug/stats_data.h | 30 +++++---- src/core/lib/debug/stats_data.yaml | 8 ++- tools/codegen/core/gen_stats_data.py | 2 +- 6 files changed, 67 insertions(+), 45 deletions(-) diff --git a/BUILD b/BUILD index a397252262c..989d485487e 100644 --- a/BUILD +++ b/BUILD @@ -3445,12 +3445,14 @@ grpc_cc_library( "grpc_public_hdrs", "grpc_trace", "hpack_parser_table", + "stats", "//src/core:decode_huff", "//src/core:error", "//src/core:experiments", "//src/core:hpack_constants", "//src/core:slice", "//src/core:slice_refcount", + "//src/core:stats_data", "//src/core:status_helper", ], ) diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index f8916ca2372..afe40cd9243 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -43,6 +43,8 @@ #include "src/core/ext/transport/chttp2/transport/decode_huff.h" #include "src/core/ext/transport/chttp2/transport/hpack_constants.h" +#include "src/core/lib/debug/stats.h" +#include "src/core/lib/debug/stats_data.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/experiments/experiments.h" #include "src/core/lib/gprpp/status_helper.h" @@ -1309,6 +1311,7 @@ grpc_error_handle HPackParser::ParseInput(Input input, bool is_last) { unparsed_bytes_ = std::vector(input.frontier(), input.end_ptr()); return absl::OkStatus(); } + global_stats().IncrementHttp2MetadataSize(frame_length_); return input.TakeError(); } diff --git a/src/core/lib/debug/stats_data.cc b/src/core/lib/debug/stats_data.cc index 7b6c992f466..da351f939d3 100644 --- a/src/core/lib/debug/stats_data.cc +++ b/src/core/lib/debug/stats_data.cc @@ -31,15 +31,15 @@ union DblUint { uint64_t uint; }; } // namespace -void HistogramCollector_32768_24::Collect(Histogram_32768_24* result) const { - for (int i = 0; i < 24; i++) { +void HistogramCollector_65536_26::Collect(Histogram_65536_26* result) const { + for (int i = 0; i < 26; i++) { result->buckets_[i] += buckets_[i].load(std::memory_order_relaxed); } } -Histogram_32768_24 operator-(const Histogram_32768_24& left, - const Histogram_32768_24& right) { - Histogram_32768_24 result; - for (int i = 0; i < 24; i++) { +Histogram_65536_26 operator-(const Histogram_65536_26& left, + const Histogram_65536_26& right) { + Histogram_65536_26 result; + for (int i = 0; i < 26; i++) { result.buckets_[i] = left.buckets_[i] - right.buckets_[i]; } return result; @@ -117,29 +117,31 @@ const absl::string_view GlobalStats::counter_doc[static_cast( "Number of completion queues created for cq_callback (indicates callback " "api usage)", }; -const absl::string_view - GlobalStats::histogram_name[static_cast(Histogram::COUNT)] = { - "call_initial_size", "tcp_write_size", "tcp_write_iov_size", - "tcp_read_size", "tcp_read_offer", "tcp_read_offer_iov_size", - "http2_send_message_size", +const absl::string_view GlobalStats::histogram_name[static_cast( + Histogram::COUNT)] = { + "call_initial_size", "tcp_write_size", "tcp_write_iov_size", + "tcp_read_size", "tcp_read_offer", "tcp_read_offer_iov_size", + "http2_send_message_size", "http2_metadata_size", }; -const absl::string_view - GlobalStats::histogram_doc[static_cast(Histogram::COUNT)] = { - "Initial size of the grpc_call arena created at call start", - "Number of bytes offered to each syscall_write", - "Number of byte segments offered to each syscall_write", - "Number of bytes received by each syscall_read", - "Number of bytes offered to each syscall_read", - "Number of byte segments offered to each syscall_read", - "Size of messages received by HTTP2 transport", +const absl::string_view GlobalStats::histogram_doc[static_cast( + Histogram::COUNT)] = { + "Initial size of the grpc_call arena created at call start", + "Number of bytes offered to each syscall_write", + "Number of byte segments offered to each syscall_write", + "Number of bytes received by each syscall_read", + "Number of bytes offered to each syscall_read", + "Number of byte segments offered to each syscall_read", + "Size of messages received by HTTP2 transport", + "Number of bytes consumed by metadata, according to HPACK accounting rules", }; namespace { -const int kStatsTable0[25] = { - 0, 1, 2, 4, 7, 11, 17, 26, 40, 61, 93, 142, 216, - 329, 500, 760, 1155, 1755, 2667, 4052, 6155, 9350, 14203, 21574, 32768}; -const uint8_t kStatsTable1[27] = {3, 3, 4, 5, 6, 6, 7, 8, 9, - 10, 11, 11, 12, 13, 14, 15, 16, 16, - 17, 18, 19, 20, 20, 21, 22, 23, 24}; +const int kStatsTable0[27] = {0, 1, 2, 4, 7, 11, 17, + 26, 40, 61, 92, 139, 210, 317, + 478, 721, 1087, 1638, 2468, 3719, 5604, + 8443, 12721, 19166, 28875, 43502, 65536}; +const uint8_t kStatsTable1[29] = {3, 3, 4, 5, 6, 6, 7, 8, 9, 10, + 11, 11, 12, 13, 14, 15, 16, 16, 17, 18, + 19, 20, 21, 21, 22, 23, 24, 25, 26}; const int kStatsTable2[21] = { 0, 1, 3, 8, 19, 45, 106, 250, 588, 1383, 3252, 7646, 17976, 42262, @@ -150,7 +152,7 @@ const uint8_t kStatsTable3[23] = {2, 3, 3, 4, 5, 6, 7, 8, const int kStatsTable4[11] = {0, 1, 2, 4, 7, 11, 17, 26, 38, 56, 80}; const uint8_t kStatsTable5[9] = {3, 3, 4, 5, 6, 6, 7, 8, 9}; } // namespace -int Histogram_32768_24::BucketFor(int value) { +int Histogram_65536_26::BucketFor(int value) { if (value < 3) { if (value < 0) { return 0; @@ -158,14 +160,14 @@ int Histogram_32768_24::BucketFor(int value) { return value; } } else { - if (value < 24577) { + if (value < 49153) { DblUint val; val.dbl = value; const int bucket = kStatsTable1[((val.uint - 4613937818241073152ull) >> 51)]; return bucket - (value < kStatsTable0[bucket]); } else { - return 23; + return 25; } } } @@ -234,7 +236,7 @@ HistogramView GlobalStats::histogram(Histogram which) const { default: GPR_UNREACHABLE_CODE(return HistogramView()); case Histogram::kCallInitialSize: - return HistogramView{&Histogram_32768_24::BucketFor, kStatsTable0, 24, + return HistogramView{&Histogram_65536_26::BucketFor, kStatsTable0, 26, call_initial_size.buckets()}; case Histogram::kTcpWriteSize: return HistogramView{&Histogram_16777216_20::BucketFor, kStatsTable2, 20, @@ -254,6 +256,9 @@ HistogramView GlobalStats::histogram(Histogram which) const { case Histogram::kHttp2SendMessageSize: return HistogramView{&Histogram_16777216_20::BucketFor, kStatsTable2, 20, http2_send_message_size.buckets()}; + case Histogram::kHttp2MetadataSize: + return HistogramView{&Histogram_65536_26::BucketFor, kStatsTable0, 26, + http2_metadata_size.buckets()}; } } std::unique_ptr GlobalStatsCollector::Collect() const { @@ -298,6 +303,7 @@ std::unique_ptr GlobalStatsCollector::Collect() const { data.tcp_read_offer.Collect(&result->tcp_read_offer); data.tcp_read_offer_iov_size.Collect(&result->tcp_read_offer_iov_size); data.http2_send_message_size.Collect(&result->http2_send_message_size); + data.http2_metadata_size.Collect(&result->http2_metadata_size); } return result; } @@ -336,6 +342,7 @@ std::unique_ptr GlobalStats::Diff(const GlobalStats& other) const { tcp_read_offer_iov_size - other.tcp_read_offer_iov_size; result->http2_send_message_size = http2_send_message_size - other.http2_send_message_size; + result->http2_metadata_size = http2_metadata_size - other.http2_metadata_size; return result; } } // namespace grpc_core diff --git a/src/core/lib/debug/stats_data.h b/src/core/lib/debug/stats_data.h index 33a30ed15fe..bcb16d0bf5b 100644 --- a/src/core/lib/debug/stats_data.h +++ b/src/core/lib/debug/stats_data.h @@ -34,28 +34,28 @@ #include "src/core/lib/gprpp/per_cpu.h" namespace grpc_core { -class HistogramCollector_32768_24; -class Histogram_32768_24 { +class HistogramCollector_65536_26; +class Histogram_65536_26 { public: static int BucketFor(int value); const uint64_t* buckets() const { return buckets_; } - friend Histogram_32768_24 operator-(const Histogram_32768_24& left, - const Histogram_32768_24& right); + friend Histogram_65536_26 operator-(const Histogram_65536_26& left, + const Histogram_65536_26& right); private: - friend class HistogramCollector_32768_24; - uint64_t buckets_[24]{}; + friend class HistogramCollector_65536_26; + uint64_t buckets_[26]{}; }; -class HistogramCollector_32768_24 { +class HistogramCollector_65536_26 { public: void Increment(int value) { - buckets_[Histogram_32768_24::BucketFor(value)].fetch_add( + buckets_[Histogram_65536_26::BucketFor(value)].fetch_add( 1, std::memory_order_relaxed); } - void Collect(Histogram_32768_24* result) const; + void Collect(Histogram_65536_26* result) const; private: - std::atomic buckets_[24]{}; + std::atomic buckets_[26]{}; }; class HistogramCollector_16777216_20; class Histogram_16777216_20 { @@ -132,6 +132,7 @@ struct GlobalStats { kTcpReadOffer, kTcpReadOfferIovSize, kHttp2SendMessageSize, + kHttp2MetadataSize, COUNT }; GlobalStats(); @@ -163,13 +164,14 @@ struct GlobalStats { }; uint64_t counters[static_cast(Counter::COUNT)]; }; - Histogram_32768_24 call_initial_size; + Histogram_65536_26 call_initial_size; Histogram_16777216_20 tcp_write_size; Histogram_80_10 tcp_write_iov_size; Histogram_16777216_20 tcp_read_size; Histogram_16777216_20 tcp_read_offer; Histogram_80_10 tcp_read_offer_iov_size; Histogram_16777216_20 http2_send_message_size; + Histogram_65536_26 http2_metadata_size; HistogramView histogram(Histogram which) const; std::unique_ptr Diff(const GlobalStats& other) const; }; @@ -257,6 +259,9 @@ class GlobalStatsCollector { void IncrementHttp2SendMessageSize(int value) { data_.this_cpu().http2_send_message_size.Increment(value); } + void IncrementHttp2MetadataSize(int value) { + data_.this_cpu().http2_metadata_size.Increment(value); + } private: struct Data { @@ -277,13 +282,14 @@ class GlobalStatsCollector { std::atomic cq_pluck_creates{0}; std::atomic cq_next_creates{0}; std::atomic cq_callback_creates{0}; - HistogramCollector_32768_24 call_initial_size; + HistogramCollector_65536_26 call_initial_size; HistogramCollector_16777216_20 tcp_write_size; HistogramCollector_80_10 tcp_write_iov_size; HistogramCollector_16777216_20 tcp_read_size; HistogramCollector_16777216_20 tcp_read_offer; HistogramCollector_80_10 tcp_read_offer_iov_size; HistogramCollector_16777216_20 http2_send_message_size; + HistogramCollector_65536_26 http2_metadata_size; }; PerCpu data_; }; diff --git a/src/core/lib/debug/stats_data.yaml b/src/core/lib/debug/stats_data.yaml index c330e80a764..c253fd9b506 100644 --- a/src/core/lib/debug/stats_data.yaml +++ b/src/core/lib/debug/stats_data.yaml @@ -21,8 +21,8 @@ - counter: server_calls_created doc: Number of server side calls created by this process - histogram: call_initial_size - max: 32768 - buckets: 24 + max: 65536 + buckets: 26 doc: Initial size of the grpc_call arena created at call start - counter: client_channels_created doc: Number of client channels created @@ -74,6 +74,10 @@ doc: Number of times sending was completely stalled by the transport flow control window - counter: http2_stream_stalls doc: Number of times sending was completely stalled by the stream flow control window +- histogram: http2_metadata_size + max: 65536 + buckets: 26 + doc: Number of bytes consumed by metadata, according to HPACK accounting rules # completion queues - counter: cq_pluck_creates doc: Number of completion queues created for cq_pluck (indicates sync api usage) diff --git a/tools/codegen/core/gen_stats_data.py b/tools/codegen/core/gen_stats_data.py index 4f05686dbfa..4957c73548e 100755 --- a/tools/codegen/core/gen_stats_data.py +++ b/tools/codegen/core/gen_stats_data.py @@ -25,7 +25,7 @@ import sys import yaml with open('src/core/lib/debug/stats_data.yaml') as f: - attrs = yaml.load(f.read()) + attrs = yaml.load(f.read(), Loader=yaml.Loader) REQUIRED_FIELDS = ['name', 'doc']