From 064db44e3bf7b7264f6a380796caab3f5d379135 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 20 Oct 2016 09:34:58 -0700 Subject: Initial sketch of bdp estimator --- src/core/lib/transport/bdp_estimator.c | 65 ++++++++++++++++++++++++++++++++++ src/core/lib/transport/bdp_estimator.h | 62 ++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 src/core/lib/transport/bdp_estimator.c create mode 100644 src/core/lib/transport/bdp_estimator.h (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/bdp_estimator.c b/src/core/lib/transport/bdp_estimator.c new file mode 100644 index 0000000000..038054d4e4 --- /dev/null +++ b/src/core/lib/transport/bdp_estimator.c @@ -0,0 +1,65 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "src/core/lib/transport/bdp_estimator.h" + +#include + +void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator) { + estimator->num_samples = 0; + estimator->first_sample_idx = 0; + estimator->sampling = false; +} + +static int compare_samples(const void *a, const void *b) { + return GPR_ICMP(*(int64_t *)a, *(int64_t *)b); +} + +bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, + int64_t *estimate) { + if (estimator->num_samples < GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE) { + return false; + } + + int64_t samples[GRPC_BDP_SAMPLES]; + for (uint8_t i = 0; i < estimator->num_samples; i++) { + samples[i] = + estimator + ->samples[(estimator->first_sample_idx + i) % GRPC_BDP_SAMPLES]; + } + qsort(samples, estimator->num_samples, sizeof(*samples), compare_samples); + + if (estimator->num_samples & 1) { + } else { + } +} diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h new file mode 100644 index 0000000000..9bbb3fffb5 --- /dev/null +++ b/src/core/lib/transport/bdp_estimator.h @@ -0,0 +1,62 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#ifndef GRPC_CORE_LIB_TRANSPORT_BDP_ESTIMATOR_H +#define GRPC_CORE_LIB_TRANSPORT_BDP_ESTIMATOR_H + +#include +#include + +#define GRPC_BDP_SAMPLES 16 +#define GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE 3 + +typedef struct grpc_bdp_estimator { + int64_t samples[GRPC_BDP_SAMPLES]; + uint8_t num_samples; + uint8_t first_sample_idx; + bool sampling; +} grpc_bdp_estimator; + +void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator); +void grpc_bdp_estimator_destroy(grpc_bdp_estimator *estimator); + +// Returns true if a reasonable estimate could be obtained +bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, + int64_t *estimate); +// Returns true if the user should start a ping +bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, + int64_t num_bytes); +// Completes a previously started ping +void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator); + +#endif -- cgit v1.2.3 From 56331f7f6598a773b7f46bc2b9b85cfb66685c41 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 20 Oct 2016 13:43:07 -0700 Subject: Complete code (no tests yet) --- src/core/lib/transport/bdp_estimator.c | 40 ++++++++++++++++++++++++++++++++++ src/core/lib/transport/bdp_estimator.h | 4 +++- 2 files changed, 43 insertions(+), 1 deletion(-) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/bdp_estimator.c b/src/core/lib/transport/bdp_estimator.c index 038054d4e4..f397440cab 100644 --- a/src/core/lib/transport/bdp_estimator.c +++ b/src/core/lib/transport/bdp_estimator.c @@ -33,6 +33,9 @@ #include "src/core/lib/transport/bdp_estimator.h" +#include + +#include #include void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator) { @@ -60,6 +63,43 @@ bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, qsort(samples, estimator->num_samples, sizeof(*samples), compare_samples); if (estimator->num_samples & 1) { + *estimate = samples[estimator->num_samples / 2]; + } else { + *estimate = (samples[estimator->num_samples / 2] + + samples[estimator->num_samples / 2 + 1]) / + 2; + } + return true; +} + +static int64_t *sampling(grpc_bdp_estimator *estimator) { + return &estimator + ->samples[(estimator->first_sample_idx + estimator->num_samples) % + GRPC_BDP_SAMPLES]; +} + +bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, + int64_t num_bytes) { + if (estimator->sampling) { + *sampling(estimator) += num_bytes; + return false; } else { + return true; + } +} + +void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator) { + GPR_ASSERT(!estimator->sampling); + estimator->sampling = true; + if (estimator->num_samples == GRPC_BDP_SAMPLES) { + estimator->first_sample_idx++; + estimator->num_samples--; } + *sampling(estimator) = 0; +} + +void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator) { + GPR_ASSERT(estimator->sampling); + estimator->num_samples++; + estimator->sampling = false; } diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index 9bbb3fffb5..3149486c87 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -41,10 +41,10 @@ #define GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE 3 typedef struct grpc_bdp_estimator { - int64_t samples[GRPC_BDP_SAMPLES]; uint8_t num_samples; uint8_t first_sample_idx; bool sampling; + int64_t samples[GRPC_BDP_SAMPLES]; } grpc_bdp_estimator; void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator); @@ -56,6 +56,8 @@ bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, // Returns true if the user should start a ping bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, int64_t num_bytes); +// Note that a ping is starting +void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator); // Completes a previously started ping void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator); -- cgit v1.2.3 From 9e0066b0c9a7e71e9b2ba14cf0049ca0dbfccc96 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 20 Oct 2016 14:04:18 -0700 Subject: Add estimator test --- BUILD | 8 + CMakeLists.txt | 3 + Makefile | 40 +++ binding.gyp | 1 + build.yaml | 12 + config.m4 | 1 + gRPC-Core.podspec | 3 + grpc.gemspec | 2 + package.xml | 2 + src/core/lib/transport/bdp_estimator.h | 1 - src/python/grpcio/grpc_core_dependencies.py | 1 + tools/doxygen/Doxyfile.core.internal | 2 + tools/run_tests/sources_and_headers.json | 20 ++ tools/run_tests/tests.json | 333 +++++++++++---------- vsprojects/buildtests_c.sln | 27 ++ vsprojects/vcxproj/grpc/grpc.vcxproj | 3 + vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 6 + .../vcxproj/grpc_test_util/grpc_test_util.vcxproj | 3 + .../grpc_test_util/grpc_test_util.vcxproj.filters | 6 + .../vcxproj/grpc_unsecure/grpc_unsecure.vcxproj | 3 + .../grpc_unsecure/grpc_unsecure.vcxproj.filters | 6 + 21 files changed, 326 insertions(+), 157 deletions(-) (limited to 'src/core/lib/transport') diff --git a/BUILD b/BUILD index 5c4333463c..76ce21c064 100644 --- a/BUILD +++ b/BUILD @@ -237,6 +237,7 @@ cc_library( "src/core/lib/surface/init.h", "src/core/lib/surface/lame_client.h", "src/core/lib/surface/server.h", + "src/core/lib/transport/bdp_estimator.h", "src/core/lib/transport/byte_stream.h", "src/core/lib/transport/connectivity_state.h", "src/core/lib/transport/mdstr_hash_table.h", @@ -409,6 +410,7 @@ cc_library( "src/core/lib/surface/server.c", "src/core/lib/surface/validate_metadata.c", "src/core/lib/surface/version.c", + "src/core/lib/transport/bdp_estimator.c", "src/core/lib/transport/byte_stream.c", "src/core/lib/transport/connectivity_state.c", "src/core/lib/transport/mdstr_hash_table.c", @@ -642,6 +644,7 @@ cc_library( "src/core/lib/surface/init.h", "src/core/lib/surface/lame_client.h", "src/core/lib/surface/server.h", + "src/core/lib/transport/bdp_estimator.h", "src/core/lib/transport/byte_stream.h", "src/core/lib/transport/connectivity_state.h", "src/core/lib/transport/mdstr_hash_table.h", @@ -799,6 +802,7 @@ cc_library( "src/core/lib/surface/server.c", "src/core/lib/surface/validate_metadata.c", "src/core/lib/surface/version.c", + "src/core/lib/transport/bdp_estimator.c", "src/core/lib/transport/byte_stream.c", "src/core/lib/transport/connectivity_state.c", "src/core/lib/transport/mdstr_hash_table.c", @@ -1002,6 +1006,7 @@ cc_library( "src/core/lib/surface/init.h", "src/core/lib/surface/lame_client.h", "src/core/lib/surface/server.h", + "src/core/lib/transport/bdp_estimator.h", "src/core/lib/transport/byte_stream.h", "src/core/lib/transport/connectivity_state.h", "src/core/lib/transport/mdstr_hash_table.h", @@ -1151,6 +1156,7 @@ cc_library( "src/core/lib/surface/server.c", "src/core/lib/surface/validate_metadata.c", "src/core/lib/surface/version.c", + "src/core/lib/transport/bdp_estimator.c", "src/core/lib/transport/byte_stream.c", "src/core/lib/transport/connectivity_state.c", "src/core/lib/transport/mdstr_hash_table.c", @@ -1916,6 +1922,7 @@ objc_library( "src/core/lib/surface/server.c", "src/core/lib/surface/validate_metadata.c", "src/core/lib/surface/version.c", + "src/core/lib/transport/bdp_estimator.c", "src/core/lib/transport/byte_stream.c", "src/core/lib/transport/connectivity_state.c", "src/core/lib/transport/mdstr_hash_table.c", @@ -2128,6 +2135,7 @@ objc_library( "src/core/lib/surface/init.h", "src/core/lib/surface/lame_client.h", "src/core/lib/surface/server.h", + "src/core/lib/transport/bdp_estimator.h", "src/core/lib/transport/byte_stream.h", "src/core/lib/transport/connectivity_state.h", "src/core/lib/transport/mdstr_hash_table.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index 893aac36fb..78a4852bed 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -374,6 +374,7 @@ add_library(grpc src/core/lib/surface/server.c src/core/lib/surface/validate_metadata.c src/core/lib/surface/version.c + src/core/lib/transport/bdp_estimator.c src/core/lib/transport/byte_stream.c src/core/lib/transport/connectivity_state.c src/core/lib/transport/mdstr_hash_table.c @@ -635,6 +636,7 @@ add_library(grpc_cronet src/core/lib/surface/server.c src/core/lib/surface/validate_metadata.c src/core/lib/surface/version.c + src/core/lib/transport/bdp_estimator.c src/core/lib/transport/byte_stream.c src/core/lib/transport/connectivity_state.c src/core/lib/transport/mdstr_hash_table.c @@ -868,6 +870,7 @@ add_library(grpc_unsecure src/core/lib/surface/server.c src/core/lib/surface/validate_metadata.c src/core/lib/surface/version.c + src/core/lib/transport/bdp_estimator.c src/core/lib/transport/byte_stream.c src/core/lib/transport/connectivity_state.c src/core/lib/transport/mdstr_hash_table.c diff --git a/Makefile b/Makefile index 38be9e658c..5c96a07477 100644 --- a/Makefile +++ b/Makefile @@ -905,6 +905,7 @@ alloc_test: $(BINDIR)/$(CONFIG)/alloc_test alpn_test: $(BINDIR)/$(CONFIG)/alpn_test api_fuzzer: $(BINDIR)/$(CONFIG)/api_fuzzer bad_server_response_test: $(BINDIR)/$(CONFIG)/bad_server_response_test +bdp_estimator_test: $(BINDIR)/$(CONFIG)/bdp_estimator_test bin_decoder_test: $(BINDIR)/$(CONFIG)/bin_decoder_test bin_encoder_test: $(BINDIR)/$(CONFIG)/bin_encoder_test census_context_test: $(BINDIR)/$(CONFIG)/census_context_test @@ -1242,6 +1243,7 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/alloc_test \ $(BINDIR)/$(CONFIG)/alpn_test \ $(BINDIR)/$(CONFIG)/bad_server_response_test \ + $(BINDIR)/$(CONFIG)/bdp_estimator_test \ $(BINDIR)/$(CONFIG)/bin_decoder_test \ $(BINDIR)/$(CONFIG)/bin_encoder_test \ $(BINDIR)/$(CONFIG)/census_context_test \ @@ -1561,6 +1563,8 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/alpn_test || ( echo test alpn_test failed ; exit 1 ) $(E) "[RUN] Testing bad_server_response_test" $(Q) $(BINDIR)/$(CONFIG)/bad_server_response_test || ( echo test bad_server_response_test failed ; exit 1 ) + $(E) "[RUN] Testing bdp_estimator_test" + $(Q) $(BINDIR)/$(CONFIG)/bdp_estimator_test || ( echo test bdp_estimator_test failed ; exit 1 ) $(E) "[RUN] Testing bin_decoder_test" $(Q) $(BINDIR)/$(CONFIG)/bin_decoder_test || ( echo test bin_decoder_test failed ; exit 1 ) $(E) "[RUN] Testing bin_encoder_test" @@ -2631,6 +2635,7 @@ LIBGRPC_SRC = \ src/core/lib/surface/server.c \ src/core/lib/surface/validate_metadata.c \ src/core/lib/surface/version.c \ + src/core/lib/transport/bdp_estimator.c \ src/core/lib/transport/byte_stream.c \ src/core/lib/transport/connectivity_state.c \ src/core/lib/transport/mdstr_hash_table.c \ @@ -2910,6 +2915,7 @@ LIBGRPC_CRONET_SRC = \ src/core/lib/surface/server.c \ src/core/lib/surface/validate_metadata.c \ src/core/lib/surface/version.c \ + src/core/lib/transport/bdp_estimator.c \ src/core/lib/transport/byte_stream.c \ src/core/lib/transport/connectivity_state.c \ src/core/lib/transport/mdstr_hash_table.c \ @@ -3179,6 +3185,7 @@ LIBGRPC_TEST_UTIL_SRC = \ src/core/lib/surface/server.c \ src/core/lib/surface/validate_metadata.c \ src/core/lib/surface/version.c \ + src/core/lib/transport/bdp_estimator.c \ src/core/lib/transport/byte_stream.c \ src/core/lib/transport/connectivity_state.c \ src/core/lib/transport/mdstr_hash_table.c \ @@ -3374,6 +3381,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/surface/server.c \ src/core/lib/surface/validate_metadata.c \ src/core/lib/surface/version.c \ + src/core/lib/transport/bdp_estimator.c \ src/core/lib/transport/byte_stream.c \ src/core/lib/transport/connectivity_state.c \ src/core/lib/transport/mdstr_hash_table.c \ @@ -6986,6 +6994,38 @@ endif endif +BDP_ESTIMATOR_TEST_SRC = \ + test/core/transport/bdp_estimator_test.c \ + +BDP_ESTIMATOR_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(BDP_ESTIMATOR_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/bdp_estimator_test: openssl_dep_error + +else + + + +$(BINDIR)/$(CONFIG)/bdp_estimator_test: $(BDP_ESTIMATOR_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LD) $(LDFLAGS) $(BDP_ESTIMATOR_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBS) $(LDLIBS_SECURE) -o $(BINDIR)/$(CONFIG)/bdp_estimator_test + +endif + +$(OBJDIR)/$(CONFIG)/test/core/transport/bdp_estimator_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_bdp_estimator_test: $(BDP_ESTIMATOR_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(BDP_ESTIMATOR_TEST_OBJS:.o=.dep) +endif +endif + + BIN_DECODER_TEST_SRC = \ test/core/transport/chttp2/bin_decoder_test.c \ diff --git a/binding.gyp b/binding.gyp index 397bb1b639..d94f2fe2b4 100644 --- a/binding.gyp +++ b/binding.gyp @@ -649,6 +649,7 @@ 'src/core/lib/surface/server.c', 'src/core/lib/surface/validate_metadata.c', 'src/core/lib/surface/version.c', + 'src/core/lib/transport/bdp_estimator.c', 'src/core/lib/transport/byte_stream.c', 'src/core/lib/transport/connectivity_state.c', 'src/core/lib/transport/mdstr_hash_table.c', diff --git a/build.yaml b/build.yaml index 2a06653103..4237876d72 100644 --- a/build.yaml +++ b/build.yaml @@ -241,6 +241,7 @@ filegroups: - src/core/lib/surface/init.h - src/core/lib/surface/lame_client.h - src/core/lib/surface/server.h + - src/core/lib/transport/bdp_estimator.h - src/core/lib/transport/byte_stream.h - src/core/lib/transport/connectivity_state.h - src/core/lib/transport/mdstr_hash_table.h @@ -335,6 +336,7 @@ filegroups: - src/core/lib/surface/server.c - src/core/lib/surface/validate_metadata.c - src/core/lib/surface/version.c + - src/core/lib/transport/bdp_estimator.c - src/core/lib/transport/byte_stream.c - src/core/lib/transport/connectivity_state.c - src/core/lib/transport/mdstr_hash_table.c @@ -1337,6 +1339,16 @@ targets: - grpc - gpr_test_util - gpr +- name: bdp_estimator_test + build: test + language: c + src: + - test/core/transport/bdp_estimator_test.c + deps: + - grpc_test_util + - grpc + - gpr_test_util + - gpr - name: bin_decoder_test build: test language: c diff --git a/config.m4 b/config.m4 index d8716753b6..a2d480aead 100644 --- a/config.m4 +++ b/config.m4 @@ -168,6 +168,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/surface/server.c \ src/core/lib/surface/validate_metadata.c \ src/core/lib/surface/version.c \ + src/core/lib/transport/bdp_estimator.c \ src/core/lib/transport/byte_stream.c \ src/core/lib/transport/connectivity_state.c \ src/core/lib/transport/mdstr_hash_table.c \ diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index bb1bbc5f0e..cf13b495cd 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -324,6 +324,7 @@ Pod::Spec.new do |s| 'src/core/lib/surface/init.h', 'src/core/lib/surface/lame_client.h', 'src/core/lib/surface/server.h', + 'src/core/lib/transport/bdp_estimator.h', 'src/core/lib/transport/byte_stream.h', 'src/core/lib/transport/connectivity_state.h', 'src/core/lib/transport/mdstr_hash_table.h', @@ -500,6 +501,7 @@ Pod::Spec.new do |s| 'src/core/lib/surface/server.c', 'src/core/lib/surface/validate_metadata.c', 'src/core/lib/surface/version.c', + 'src/core/lib/transport/bdp_estimator.c', 'src/core/lib/transport/byte_stream.c', 'src/core/lib/transport/connectivity_state.c', 'src/core/lib/transport/mdstr_hash_table.c', @@ -701,6 +703,7 @@ Pod::Spec.new do |s| 'src/core/lib/surface/init.h', 'src/core/lib/surface/lame_client.h', 'src/core/lib/surface/server.h', + 'src/core/lib/transport/bdp_estimator.h', 'src/core/lib/transport/byte_stream.h', 'src/core/lib/transport/connectivity_state.h', 'src/core/lib/transport/mdstr_hash_table.h', diff --git a/grpc.gemspec b/grpc.gemspec index 85172922cc..1698024065 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -244,6 +244,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/surface/init.h ) s.files += %w( src/core/lib/surface/lame_client.h ) s.files += %w( src/core/lib/surface/server.h ) + s.files += %w( src/core/lib/transport/bdp_estimator.h ) s.files += %w( src/core/lib/transport/byte_stream.h ) s.files += %w( src/core/lib/transport/connectivity_state.h ) s.files += %w( src/core/lib/transport/mdstr_hash_table.h ) @@ -420,6 +421,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/surface/server.c ) s.files += %w( src/core/lib/surface/validate_metadata.c ) s.files += %w( src/core/lib/surface/version.c ) + s.files += %w( src/core/lib/transport/bdp_estimator.c ) s.files += %w( src/core/lib/transport/byte_stream.c ) s.files += %w( src/core/lib/transport/connectivity_state.c ) s.files += %w( src/core/lib/transport/mdstr_hash_table.c ) diff --git a/package.xml b/package.xml index 31a2822a75..430be87f2d 100644 --- a/package.xml +++ b/package.xml @@ -251,6 +251,7 @@ + @@ -427,6 +428,7 @@ + diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index 3149486c87..40128f1199 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -48,7 +48,6 @@ typedef struct grpc_bdp_estimator { } grpc_bdp_estimator; void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator); -void grpc_bdp_estimator_destroy(grpc_bdp_estimator *estimator); // Returns true if a reasonable estimate could be obtained bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index a40edfb090..e951222b09 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -162,6 +162,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/surface/server.c', 'src/core/lib/surface/validate_metadata.c', 'src/core/lib/surface/version.c', + 'src/core/lib/transport/bdp_estimator.c', 'src/core/lib/transport/byte_stream.c', 'src/core/lib/transport/connectivity_state.c', 'src/core/lib/transport/mdstr_hash_table.c', diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index e5c91cbb13..7c9c07f8e9 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -861,6 +861,7 @@ src/core/lib/surface/event_string.h \ src/core/lib/surface/init.h \ src/core/lib/surface/lame_client.h \ src/core/lib/surface/server.h \ +src/core/lib/transport/bdp_estimator.h \ src/core/lib/transport/byte_stream.h \ src/core/lib/transport/connectivity_state.h \ src/core/lib/transport/mdstr_hash_table.h \ @@ -1037,6 +1038,7 @@ src/core/lib/surface/metadata_array.c \ src/core/lib/surface/server.c \ src/core/lib/surface/validate_metadata.c \ src/core/lib/surface/version.c \ +src/core/lib/transport/bdp_estimator.c \ src/core/lib/transport/byte_stream.c \ src/core/lib/transport/connectivity_state.c \ src/core/lib/transport/mdstr_hash_table.c \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 7cfb1d4c17..dbf4a9b7c5 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -102,6 +102,23 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "gpr", + "gpr_test_util", + "grpc", + "grpc_test_util" + ], + "headers": [], + "is_filegroup": false, + "language": "c", + "name": "bdp_estimator_test", + "src": [ + "test/core/transport/bdp_estimator_test.c" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "grpc", @@ -6514,6 +6531,7 @@ "src/core/lib/surface/init.h", "src/core/lib/surface/lame_client.h", "src/core/lib/surface/server.h", + "src/core/lib/transport/bdp_estimator.h", "src/core/lib/transport/byte_stream.h", "src/core/lib/transport/connectivity_state.h", "src/core/lib/transport/mdstr_hash_table.h", @@ -6694,6 +6712,8 @@ "src/core/lib/surface/server.h", "src/core/lib/surface/validate_metadata.c", "src/core/lib/surface/version.c", + "src/core/lib/transport/bdp_estimator.c", + "src/core/lib/transport/bdp_estimator.h", "src/core/lib/transport/byte_stream.c", "src/core/lib/transport/byte_stream.h", "src/core/lib/transport/connectivity_state.c", diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 9ca4908eca..80de1a0bae 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -106,6 +106,27 @@ "windows" ] }, + { + "args": [], + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "gtest": false, + "language": "c", + "name": "bdp_estimator_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ] + }, { "args": [], "ci_platforms": [ @@ -17002,7 +17023,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17023,7 +17046,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17044,7 +17069,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17065,7 +17092,9 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17086,7 +17115,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17107,7 +17138,9 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17128,7 +17161,9 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17149,7 +17184,9 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17170,7 +17207,9 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17191,7 +17230,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17212,7 +17253,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17233,7 +17276,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17254,7 +17299,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17275,7 +17322,9 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17296,7 +17345,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17317,7 +17368,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17338,7 +17391,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17359,7 +17414,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17380,7 +17437,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17401,7 +17460,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17422,7 +17483,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17443,7 +17506,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17464,7 +17529,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17485,7 +17552,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17506,7 +17575,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17527,7 +17598,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17548,7 +17621,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17569,7 +17644,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17590,7 +17667,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17611,7 +17690,9 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17632,7 +17713,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17653,7 +17736,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17674,7 +17759,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17695,7 +17782,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17716,7 +17805,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17737,7 +17828,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17758,7 +17851,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17779,7 +17874,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17800,7 +17897,9 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [], + "exclude_configs": [ + "msan" + ], "flaky": false, "language": "c", "name": "h2_sockpair_1byte_test", @@ -17822,9 +17921,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -17846,9 +17943,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -17870,9 +17965,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -17894,9 +17987,7 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -17918,9 +18009,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -17942,9 +18031,7 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -17966,9 +18053,7 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -17990,9 +18075,7 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18014,9 +18097,7 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18038,9 +18119,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18128,9 +18207,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18152,9 +18229,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18176,9 +18251,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18200,9 +18273,7 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18224,9 +18295,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18248,9 +18317,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18272,9 +18339,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18318,9 +18383,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18342,9 +18405,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18366,9 +18427,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18390,9 +18449,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18414,9 +18471,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18438,9 +18493,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18462,9 +18515,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18486,9 +18537,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18510,9 +18559,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18534,9 +18581,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18558,9 +18603,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18582,9 +18625,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18606,9 +18647,7 @@ "posix" ], "cpu_cost": 0.1, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18630,9 +18669,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18654,9 +18691,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18678,9 +18713,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18702,9 +18735,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18726,9 +18757,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18772,9 +18801,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18796,9 +18823,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18820,9 +18845,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -18844,9 +18867,7 @@ "posix" ], "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], + "exclude_configs": [], "flaky": false, "language": "c", "name": "h2_ssl_test", diff --git a/vsprojects/buildtests_c.sln b/vsprojects/buildtests_c.sln index 339b42f9d7..4a22345b12 100644 --- a/vsprojects/buildtests_c.sln +++ b/vsprojects/buildtests_c.sln @@ -80,6 +80,17 @@ Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "badreq_bad_client_test", "v {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} EndProjectSection EndProject +Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "bdp_estimator_test", "vcxproj\test\bdp_estimator_test\bdp_estimator_test.vcxproj", "{56314C05-7748-B7FD-F9DE-F975A0275427}" + ProjectSection(myProperties) = preProject + lib = "False" + EndProjectSection + ProjectSection(ProjectDependencies) = postProject + {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} = {17BCAFC0-5FDC-4C94-AEB9-95F3E220614B} + {29D16885-7228-4C31-81ED-5F9187C7F2A9} = {29D16885-7228-4C31-81ED-5F9187C7F2A9} + {EAB0A629-17A9-44DB-B5FF-E91A721FE037} = {EAB0A629-17A9-44DB-B5FF-E91A721FE037} + {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} = {B23D3D1A-9438-4EDA-BEB6-9A0A03D17792} + EndProjectSection +EndProject Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "bin_decoder_test", "vcxproj\test\bin_decoder_test\bin_decoder_test.vcxproj", "{6BFAC6BA-3B9D-E8F5-BE35-91E8EFB9E25B}" ProjectSection(myProperties) = preProject lib = "False" @@ -1666,6 +1677,22 @@ Global {8A811C28-E04E-A444-E4C1-7588DF5B90AE}.Release-DLL|Win32.Build.0 = Release|Win32 {8A811C28-E04E-A444-E4C1-7588DF5B90AE}.Release-DLL|x64.ActiveCfg = Release|x64 {8A811C28-E04E-A444-E4C1-7588DF5B90AE}.Release-DLL|x64.Build.0 = Release|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug|Win32.ActiveCfg = Debug|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug|x64.ActiveCfg = Debug|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release|Win32.ActiveCfg = Release|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release|x64.ActiveCfg = Release|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug|Win32.Build.0 = Debug|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug|x64.Build.0 = Debug|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release|Win32.Build.0 = Release|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release|x64.Build.0 = Release|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug-DLL|Win32.ActiveCfg = Debug|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug-DLL|Win32.Build.0 = Debug|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug-DLL|x64.ActiveCfg = Debug|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Debug-DLL|x64.Build.0 = Debug|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release-DLL|Win32.ActiveCfg = Release|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release-DLL|Win32.Build.0 = Release|Win32 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release-DLL|x64.ActiveCfg = Release|x64 + {56314C05-7748-B7FD-F9DE-F975A0275427}.Release-DLL|x64.Build.0 = Release|x64 {6BFAC6BA-3B9D-E8F5-BE35-91E8EFB9E25B}.Debug|Win32.ActiveCfg = Debug|Win32 {6BFAC6BA-3B9D-E8F5-BE35-91E8EFB9E25B}.Debug|x64.ActiveCfg = Debug|x64 {6BFAC6BA-3B9D-E8F5-BE35-91E8EFB9E25B}.Release|Win32.ActiveCfg = Release|Win32 diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index b8c0049db5..fa271e150b 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -370,6 +370,7 @@ + @@ -633,6 +634,8 @@ + + diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index fb1f904811..05c15791d9 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -256,6 +256,9 @@ src\core\lib\surface + + src\core\lib\transport + src\core\lib\transport @@ -899,6 +902,9 @@ src\core\lib\surface + + src\core\lib\transport + src\core\lib\transport diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj index eb3a94df64..1d16ca42d3 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj @@ -263,6 +263,7 @@ + @@ -480,6 +481,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters index fcc8e34db3..3b04cb2296 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters @@ -310,6 +310,9 @@ src\core\lib\surface + + src\core\lib\transport + src\core\lib\transport @@ -683,6 +686,9 @@ src\core\lib\surface + + src\core\lib\transport + src\core\lib\transport diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index 519d7317ba..54fbbe8987 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -360,6 +360,7 @@ + @@ -601,6 +602,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index d30df5c03d..4042b47ca0 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -259,6 +259,9 @@ src\core\lib\surface + + src\core\lib\transport + src\core\lib\transport @@ -809,6 +812,9 @@ src\core\lib\surface + + src\core\lib\transport + src\core\lib\transport -- cgit v1.2.3 From 11a204c017c7c7a589b7b44a05fb25af1749f88b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 20 Oct 2016 16:51:08 -0700 Subject: Use max(recent_samples) as bdp estimation --- src/core/lib/transport/bdp_estimator.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/bdp_estimator.c b/src/core/lib/transport/bdp_estimator.c index f397440cab..d9129925b3 100644 --- a/src/core/lib/transport/bdp_estimator.c +++ b/src/core/lib/transport/bdp_estimator.c @@ -44,30 +44,18 @@ void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator) { estimator->sampling = false; } -static int compare_samples(const void *a, const void *b) { - return GPR_ICMP(*(int64_t *)a, *(int64_t *)b); -} - bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, int64_t *estimate) { if (estimator->num_samples < GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE) { return false; } - int64_t samples[GRPC_BDP_SAMPLES]; + *estimate = -1; for (uint8_t i = 0; i < estimator->num_samples; i++) { - samples[i] = + *estimate = GPR_MAX( + *estimate, estimator - ->samples[(estimator->first_sample_idx + i) % GRPC_BDP_SAMPLES]; - } - qsort(samples, estimator->num_samples, sizeof(*samples), compare_samples); - - if (estimator->num_samples & 1) { - *estimate = samples[estimator->num_samples / 2]; - } else { - *estimate = (samples[estimator->num_samples / 2] + - samples[estimator->num_samples / 2 + 1]) / - 2; + ->samples[(estimator->first_sample_idx + i) % GRPC_BDP_SAMPLES]); } return true; } -- cgit v1.2.3 From ad2a11fc3d21765d89b4f86106de9c497dae92c9 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 28 Dec 2016 13:05:00 -0800 Subject: Cleanup ping story: part 0 --- .../transport/chttp2/transport/chttp2_transport.c | 90 +++++++++++----------- .../ext/transport/chttp2/transport/frame_ping.h | 4 +- src/core/ext/transport/chttp2/transport/internal.h | 30 ++++---- src/core/lib/transport/bdp_estimator.h | 5 +- 4 files changed, 68 insertions(+), 61 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index fddf2f846d..103a7ac861 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -138,6 +138,10 @@ static void finish_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, static void cancel_pings(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_error *error); +static void send_ping_locked(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, + grpc_chttp2_ping_type ping_type, + grpc_closure *on_initiate, + grpc_closure *on_complete); /******************************************************************************* * CONSTRUCTION/DESTRUCTION/REFCOUNTING @@ -230,8 +234,6 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, t->is_client = is_client; t->outgoing_window = DEFAULT_WINDOW; t->incoming_window = DEFAULT_WINDOW; - t->ping_counter = 1; - t->pings.next = t->pings.prev = &t->pings; t->deframe_state = is_client ? GRPC_DTS_FH_0 : GRPC_DTS_CLIENT_PREFIX_0; t->is_first_frame = true; grpc_connectivity_state_init( @@ -1214,53 +1216,48 @@ static void cancel_pings(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_error *error) { /* callback remaining pings: they're not allowed to call into the transpot, and maybe they hold resources that need to be freed */ - while (t->pings.next != &t->pings) { - grpc_chttp2_outstanding_ping *ping = t->pings.next; - grpc_exec_ctx_sched(exec_ctx, ping->on_recv, GRPC_ERROR_REF(error), NULL); - ping->next->prev = ping->prev; - ping->prev->next = ping->next; - gpr_free(ping); + for (size_t i = 0; i < GRPC_CHTTP2_PING_TYPE_COUNT; i++) { + grpc_chttp2_ping_queue *pq = &t->ping_queues[i]; + grpc_closure_list_fail_all(&pq->next_queue, GRPC_ERROR_REF(error)); + grpc_closure_list_fail_all(&pq->initiate_queue, GRPC_ERROR_REF(error)); + grpc_closure_list_fail_all(&pq->inflight_queue, GRPC_ERROR_REF(error)); + grpc_exec_ctx_enqueue_list(exec_ctx, &pq->next_queue, NULL); + grpc_exec_ctx_enqueue_list(exec_ctx, &pq->initiate_queue, NULL); + grpc_exec_ctx_enqueue_list(exec_ctx, &pq->inflight_queue, NULL); } GRPC_ERROR_UNREF(error); } -static void send_ping_locked(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, - grpc_closure *on_recv) { - grpc_chttp2_outstanding_ping *p = gpr_malloc(sizeof(*p)); - p->next = &t->pings; - p->prev = p->next->prev; - p->prev->next = p->next->prev = p; - p->id[0] = (uint8_t)((t->ping_counter >> 56) & 0xff); - p->id[1] = (uint8_t)((t->ping_counter >> 48) & 0xff); - p->id[2] = (uint8_t)((t->ping_counter >> 40) & 0xff); - p->id[3] = (uint8_t)((t->ping_counter >> 32) & 0xff); - p->id[4] = (uint8_t)((t->ping_counter >> 24) & 0xff); - p->id[5] = (uint8_t)((t->ping_counter >> 16) & 0xff); - p->id[6] = (uint8_t)((t->ping_counter >> 8) & 0xff); - p->id[7] = (uint8_t)(t->ping_counter & 0xff); - t->ping_counter++; - p->on_recv = on_recv; - grpc_slice_buffer_add(&t->qbuf, grpc_chttp2_ping_create(0, p->id)); - grpc_chttp2_initiate_write(exec_ctx, t, true, "send_ping"); +static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, + grpc_chttp2_transport *t, + grpc_chttp2_ping_type ping_type, + grpc_slice_buffer *buf) { + grpc_chttp2_ping_queue *pq = &t->ping_queues[ping_type]; + if (grpc_closure_list_empty(pq->next_queue)) { + /* no ping needed: wait */ + return; + } + if (!grpc_closure_list_empty(pq->inflight_queue)) { + /* ping already in-flight: wait */ + return; + } + pq->inflight_id = t->ping_ctr * GRPC_CHTTP2_PING_TYPE_COUNT + ping_type; + t->ping_ctr++; + grpc_exec_ctx_enqueue_list(exec_ctx, &pq->initiate_queue, NULL); + grpc_slice_buffer_add(buf, grpc_chttp2_ping_create(false, pq->inflight_id)); } void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, - const uint8_t *opaque_8bytes) { - grpc_chttp2_outstanding_ping *ping; - for (ping = t->pings.next; ping != &t->pings; ping = ping->next) { - if (0 == memcmp(opaque_8bytes, ping->id, 8)) { - grpc_exec_ctx_sched(exec_ctx, ping->on_recv, GRPC_ERROR_NONE, NULL); - ping->next->prev = ping->prev; - ping->prev->next = ping->next; - gpr_free(ping); - return; - } + uint64_t id) { + grpc_chttp2_ping_queue *pq = + &t->ping_queues[id % GRPC_CHTTP2_PING_TYPE_COUNT]; + if (pq->inflight_id != id) { + char *from = grpc_endpoint_get_peer(t->ep); + gpr_log(GPR_DEBUG, "Unknown ping response from %s: %" PRIx64, from, id); + gpr_free(from); + return; } - char *msg = gpr_dump((const char *)opaque_8bytes, 8, GPR_DUMP_HEX); - char *from = grpc_endpoint_get_peer(t->ep); - gpr_log(GPR_DEBUG, "Unknown ping response from %s: %s", from, msg); - gpr_free(from); - gpr_free(msg); + grpc_exec_ctx_enqueue_list(exec_ctx, &pq->inflight_queue, NULL); } static void send_goaway(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, @@ -1305,7 +1302,8 @@ static void perform_transport_op_locked(grpc_exec_ctx *exec_ctx, } if (op->send_ping) { - send_ping_locked(exec_ctx, t, op->send_ping); + send_ping_locked(exec_ctx, t, GRPC_CHTTP2_PING_ON_NEXT_WRITE, NULL, + op->send_ping); } if (close_transport != GRPC_ERROR_NONE) { @@ -1915,8 +1913,10 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, gpr_time_from_millis(100, GPR_TIMESPAN)), gpr_now(GPR_CLOCK_MONOTONIC)) < 0) { GRPC_CHTTP2_REF_TRANSPORT(t, "bdp_ping"); - grpc_bdp_estimator_start_ping(&t->bdp_estimator); - send_ping_locked(exec_ctx, t, &t->finish_bdp_ping); + grpc_bdp_estimator_schedule_ping(&t->bdp_estimator); + send_ping_locked(exec_ctx, t, + GRPC_CHTTP2_PING_BEFORE_TRANSPORT_WINDOW_UPDATE, + &t->start_bdp_ping, &t->finish_bdp_ping); } int64_t estimate = -1; @@ -1933,7 +1933,7 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, double memory_pressure = grpc_resource_quota_get_memory_pressure( grpc_resource_user_get_quota(grpc_endpoint_get_resource_user(t->ep))); if (memory_pressure > 0.8) { - bdp_error = -(memory_pressure - 0.8) * 5 * 32768; + bdp_error -= GPR_MAX(0, t->bdp_guess) * (memory_pressure - 0.8) / 0.2; } if (t->bdp_guess < 1e-6 && bdp_error < 0) { bdp_error = 0; diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.h b/src/core/ext/transport/chttp2/transport/frame_ping.h index b9889e2d11..ef642465d7 100644 --- a/src/core/ext/transport/chttp2/transport/frame_ping.h +++ b/src/core/ext/transport/chttp2/transport/frame_ping.h @@ -41,10 +41,10 @@ typedef struct { uint8_t byte; uint8_t is_ack; - uint8_t opaque_8bytes[8]; + uint64_t opaque_8bytes; } grpc_chttp2_ping_parser; -grpc_slice grpc_chttp2_ping_create(uint8_t ack, uint8_t *opaque_8bytes); +grpc_slice grpc_chttp2_ping_create(uint8_t ack, uint64_t opaque_8bytes); grpc_error *grpc_chttp2_ping_parser_begin_frame(grpc_chttp2_ping_parser *parser, uint32_t length, uint8_t flags); diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 221f3b1bca..8497bc84b0 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -75,6 +75,19 @@ typedef enum { GRPC_CHTTP2_WRITE_STATE_WRITING_WITH_MORE_AND_COVERED_BY_POLLER, } grpc_chttp2_write_state; +typedef enum { + GRPC_CHTTP2_PING_ON_NEXT_WRITE = 0, + GRPC_CHTTP2_PING_BEFORE_TRANSPORT_WINDOW_UPDATE, + GRPC_CHTTP2_PING_TYPE_COUNT /* must be last */ +} grpc_chttp2_ping_type; + +typedef struct { + grpc_closure_list next_queue; + grpc_closure_list initiate_queue; + grpc_closure_list inflight_queue; + uint64_t inflight_id; +} grpc_chttp2_ping_queue; + /* deframer state for the overall http2 stream of bytes */ typedef enum { /* prefix: one entry per http2 connection prefix byte */ @@ -147,14 +160,6 @@ typedef enum { GRPC_CHTTP2_GOAWAY_SENT, } grpc_chttp2_sent_goaway_state; -/* Outstanding ping request data */ -typedef struct grpc_chttp2_outstanding_ping { - uint8_t id[8]; - grpc_closure *on_recv; - struct grpc_chttp2_outstanding_ping *next; - struct grpc_chttp2_outstanding_ping *prev; -} grpc_chttp2_outstanding_ping; - typedef struct grpc_chttp2_write_cb { int64_t call_at_byte; grpc_closure *closure; @@ -271,10 +276,9 @@ struct grpc_chttp2_transport { /** last new stream id */ uint32_t last_new_stream_id; - /** pings awaiting responses */ - grpc_chttp2_outstanding_ping pings; - /** next payload for an outgoing ping */ - uint64_t ping_counter; + /** ping queues for various ping insertion points */ + grpc_chttp2_ping_queue ping_queues[GRPC_CHTTP2_PING_TYPE_COUNT]; + uint64_t ping_ctr; /* unique id for pings */ /** parser for headers */ grpc_chttp2_hpack_parser hpack_parser; @@ -684,7 +688,7 @@ void grpc_chttp2_incoming_byte_stream_finished( grpc_error *error); void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, - const uint8_t *opaque_8bytes); + uint64_t id); /** add a ref to the stream and add it to the writable list; ref will be dropped in writing.c */ diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index 40128f1199..e81699cbee 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -43,6 +43,7 @@ typedef struct grpc_bdp_estimator { uint8_t num_samples; uint8_t first_sample_idx; + bool scheduled; bool sampling; int64_t samples[GRPC_BDP_SAMPLES]; } grpc_bdp_estimator; @@ -55,7 +56,9 @@ bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, // Returns true if the user should start a ping bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, int64_t num_bytes); -// Note that a ping is starting +// Schedule a ping +void grpc_bdp_estimator_schedule_ping(grpc_bdp_estimator *estimator); +// Start a ping void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator); // Completes a previously started ping void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator); -- cgit v1.2.3 From c0118b494ec2df10d7945925e9b1c9da6b868919 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 29 Dec 2016 12:17:57 -0800 Subject: Ping progress --- .../transport/chttp2/transport/chttp2_transport.c | 46 +++++++++++----------- .../ext/transport/chttp2/transport/frame_ping.c | 14 +++++-- src/core/ext/transport/chttp2/transport/internal.h | 11 ++++-- src/core/ext/transport/chttp2/transport/writing.c | 43 ++++++++++++++++++++ src/core/lib/iomgr/closure.c | 8 ++-- src/core/lib/iomgr/closure.h | 5 ++- src/core/lib/transport/bdp_estimator.c | 29 +++++++++----- src/core/lib/transport/bdp_estimator.h | 18 ++++++--- 8 files changed, 126 insertions(+), 48 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 04a8e727d1..6d302b0f11 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -244,6 +244,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_closure_init(&t->destructive_reclaimer_locked, destructive_reclaimer_locked, t, grpc_combiner_scheduler(t->combiner, false)); + grpc_closure_init(&t->start_bdp_ping_locked, start_bdp_ping_locked, t, + grpc_combiner_scheduler(t->combiner, false)); grpc_closure_init(&t->finish_bdp_ping_locked, finish_bdp_ping_locked, t, grpc_combiner_scheduler(t->combiner, false)); @@ -1204,33 +1206,24 @@ static void cancel_pings(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, and maybe they hold resources that need to be freed */ for (size_t i = 0; i < GRPC_CHTTP2_PING_TYPE_COUNT; i++) { grpc_chttp2_ping_queue *pq = &t->ping_queues[i]; - grpc_closure_list_fail_all(&pq->next_queue, GRPC_ERROR_REF(error)); - grpc_closure_list_fail_all(&pq->initiate_queue, GRPC_ERROR_REF(error)); - grpc_closure_list_fail_all(&pq->inflight_queue, GRPC_ERROR_REF(error)); - grpc_closure_list_sched(exec_ctx, &pq->next_queue); - grpc_closure_list_sched(exec_ctx, &pq->initiate_queue); - grpc_closure_list_sched(exec_ctx, &pq->inflight_queue); + for (size_t j = 0; j < GRPC_CHTTP2_PCL_COUNT; j++) { + grpc_closure_list_fail_all(&pq->lists[j], GRPC_ERROR_REF(error)); + grpc_closure_list_sched(exec_ctx, &pq->lists[j]); + } } GRPC_ERROR_UNREF(error); } -static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, - grpc_chttp2_transport *t, - grpc_chttp2_ping_type ping_type, - grpc_slice_buffer *buf) { +static void send_ping_locked(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, + grpc_chttp2_ping_type ping_type, + grpc_closure *on_initiate, grpc_closure *on_ack) { grpc_chttp2_ping_queue *pq = &t->ping_queues[ping_type]; - if (grpc_closure_list_empty(pq->next_queue)) { - /* no ping needed: wait */ - return; - } - if (!grpc_closure_list_empty(pq->inflight_queue)) { - /* ping already in-flight: wait */ - return; + grpc_closure_list_append(&pq->lists[GRPC_CHTTP2_PCL_INITIATE], on_initiate, + GRPC_ERROR_NONE); + if (grpc_closure_list_append(&pq->lists[GRPC_CHTTP2_PCL_NEXT], on_ack, + GRPC_ERROR_NONE)) { + grpc_chttp2_initiate_write(exec_ctx, t, false, "send_ping"); } - pq->inflight_id = t->ping_ctr * GRPC_CHTTP2_PING_TYPE_COUNT + ping_type; - t->ping_ctr++; - grpc_closure_list_sched(exec_ctx, &pq->initiate_queue); - grpc_slice_buffer_add(buf, grpc_chttp2_ping_create(false, pq->inflight_id)); } void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, @@ -1243,7 +1236,10 @@ void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, gpr_free(from); return; } - grpc_closure_list_sched(exec_ctx, &pq->inflight_queue); + grpc_closure_list_sched(exec_ctx, &pq->lists[GRPC_CHTTP2_PCL_INFLIGHT]); + if (!grpc_closure_list_empty(pq->lists[GRPC_CHTTP2_PCL_NEXT])) { + grpc_chttp2_initiate_write(exec_ctx, t, false, "continue_pings"); + } } static void send_goaway(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, @@ -1942,6 +1938,12 @@ static void finish_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "bdp_ping"); } +static void start_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, + grpc_error *error) { + grpc_chttp2_transport *t = tp; + grpc_bdp_estimator_start_ping(&t->bdp_estimator); +} + /******************************************************************************* * CALLBACK LOOP */ diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.c b/src/core/ext/transport/chttp2/transport/frame_ping.c index 7de5f6362d..66a9bc0966 100644 --- a/src/core/ext/transport/chttp2/transport/frame_ping.c +++ b/src/core/ext/transport/chttp2/transport/frame_ping.c @@ -40,7 +40,7 @@ #include #include -grpc_slice grpc_chttp2_ping_create(uint8_t ack, uint8_t *opaque_8bytes) { +grpc_slice grpc_chttp2_ping_create(uint8_t ack, uint64_t opaque_8bytes) { grpc_slice slice = grpc_slice_malloc(9 + 8); uint8_t *p = GRPC_SLICE_START_PTR(slice); @@ -53,7 +53,14 @@ grpc_slice grpc_chttp2_ping_create(uint8_t ack, uint8_t *opaque_8bytes) { *p++ = 0; *p++ = 0; *p++ = 0; - memcpy(p, opaque_8bytes, 8); + *p++ = (uint8_t)(opaque_8bytes >> 56); + *p++ = (uint8_t)(opaque_8bytes >> 48); + *p++ = (uint8_t)(opaque_8bytes >> 40); + *p++ = (uint8_t)(opaque_8bytes >> 32); + *p++ = (uint8_t)(opaque_8bytes >> 24); + *p++ = (uint8_t)(opaque_8bytes >> 16); + *p++ = (uint8_t)(opaque_8bytes >> 8); + *p++ = (uint8_t)(opaque_8bytes); return slice; } @@ -70,6 +77,7 @@ grpc_error *grpc_chttp2_ping_parser_begin_frame(grpc_chttp2_ping_parser *parser, } parser->byte = 0; parser->is_ack = flags; + parser->opaque_8bytes = 0; return GRPC_ERROR_NONE; } @@ -83,7 +91,7 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser, grpc_chttp2_ping_parser *p = parser; while (p->byte != 8 && cur != end) { - p->opaque_8bytes[p->byte] = *cur; + p->opaque_8bytes |= (((uint64_t)*cur) << (8 * p->byte)); cur++; p->byte++; } diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 1a6a1d8b2a..90abf386c7 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -81,10 +81,15 @@ typedef enum { GRPC_CHTTP2_PING_TYPE_COUNT /* must be last */ } grpc_chttp2_ping_type; +typedef enum { + GRPC_CHTTP2_PCL_NEXT = 0, + GRPC_CHTTP2_PCL_INITIATE, + GRPC_CHTTP2_PCL_INFLIGHT, + GRPC_CHTTP2_PCL_COUNT /* must be last */ +} grpc_chttp2_ping_closure_list; + typedef struct { - grpc_closure_list next_queue; - grpc_closure_list initiate_queue; - grpc_closure_list inflight_queue; + grpc_closure_list lists[GRPC_CHTTP2_PCL_COUNT]; uint64_t inflight_id; } grpc_chttp2_ping_queue; diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index bbf5c0b1b4..43205a82f6 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -55,6 +55,45 @@ static void finish_write_cb(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, t->write_cb_pool = cb; } +static void collapse_pings_from_into(grpc_chttp2_transport *t, + grpc_chttp2_ping_type ping_type, + grpc_chttp2_ping_queue *pq) { + for (size_t i = 0; i < GRPC_CHTTP2_PCL_COUNT; i++) { + grpc_closure_list_move(&t->ping_queues[ping_type].lists[i], &pq->lists[i]); + } +} + +static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, + grpc_chttp2_transport *t, + grpc_chttp2_ping_type ping_type) { + grpc_chttp2_ping_queue *pq = &t->ping_queues[ping_type]; + if (grpc_closure_list_empty(pq->lists[GRPC_CHTTP2_PCL_NEXT])) { + /* no ping needed: wait */ + return; + } + if (!grpc_closure_list_empty(pq->lists[GRPC_CHTTP2_PCL_INFLIGHT])) { + /* ping already in-flight: wait */ + return; + } + /* coalesce equivalent pings into this one */ + switch (ping_type) { + case GRPC_CHTTP2_PING_BEFORE_TRANSPORT_WINDOW_UPDATE: + collapse_pings_from_into(t, GRPC_CHTTP2_PING_ON_NEXT_WRITE, pq); + break; + case GRPC_CHTTP2_PING_ON_NEXT_WRITE: + break; + case GRPC_CHTTP2_PING_TYPE_COUNT: + GPR_UNREACHABLE_CODE(break); + } + pq->inflight_id = t->ping_ctr * GRPC_CHTTP2_PING_TYPE_COUNT + ping_type; + t->ping_ctr++; + grpc_closure_list_sched(exec_ctx, &pq->lists[GRPC_CHTTP2_PCL_INITIATE]); + grpc_closure_list_move(&pq->lists[GRPC_CHTTP2_PCL_NEXT], + &pq->lists[GRPC_CHTTP2_PCL_INFLIGHT]); + grpc_slice_buffer_add(&t->outbuf, + grpc_chttp2_ping_create(false, pq->inflight_id)); +} + static void update_list(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_chttp2_stream *s, int64_t send_bytes, grpc_chttp2_write_cb **list, grpc_error *error) { @@ -226,6 +265,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx, t->settings[GRPC_SENT_SETTINGS][GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE], 1024); if (t->incoming_window < 3 * target_incoming_window / 4) { + maybe_initiate_ping(exec_ctx, t, + GRPC_CHTTP2_PING_BEFORE_TRANSPORT_WINDOW_UPDATE); uint32_t announced = (uint32_t)GPR_CLAMP( target_incoming_window - t->incoming_window, 0, UINT32_MAX); GRPC_CHTTP2_FLOW_CREDIT_TRANSPORT("write", t, incoming_window, announced); @@ -234,6 +275,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx, 0, announced, &throwaway_stats)); } + maybe_initiate_ping(exec_ctx, t, GRPC_CHTTP2_PING_ON_NEXT_WRITE); + GPR_TIMER_END("grpc_chttp2_begin_write", 0); return t->outbuf.count > 0; diff --git a/src/core/lib/iomgr/closure.c b/src/core/lib/iomgr/closure.c index da0ec878a3..ec197c7d82 100644 --- a/src/core/lib/iomgr/closure.c +++ b/src/core/lib/iomgr/closure.c @@ -50,20 +50,22 @@ void grpc_closure_list_init(grpc_closure_list *closure_list) { closure_list->head = closure_list->tail = NULL; } -void grpc_closure_list_append(grpc_closure_list *closure_list, +bool grpc_closure_list_append(grpc_closure_list *closure_list, grpc_closure *closure, grpc_error *error) { if (closure == NULL) { GRPC_ERROR_UNREF(error); - return; + return false; } closure->error_data.error = error; closure->next_data.next = NULL; - if (closure_list->head == NULL) { + bool was_empty = (closure_list->head == NULL); + if (was_empty) { closure_list->head = closure; } else { closure_list->tail->next_data.next = closure; } closure_list->tail = closure; + return was_empty; } void grpc_closure_list_fail_all(grpc_closure_list *list, diff --git a/src/core/lib/iomgr/closure.h b/src/core/lib/iomgr/closure.h index 1b5d9b20a0..1a4ebbd360 100644 --- a/src/core/lib/iomgr/closure.h +++ b/src/core/lib/iomgr/closure.h @@ -117,8 +117,9 @@ grpc_closure *grpc_closure_create(grpc_iomgr_cb_func cb, void *cb_arg, void grpc_closure_list_init(grpc_closure_list *list); /** add \a closure to the end of \a list - and set \a closure's result to \a error */ -void grpc_closure_list_append(grpc_closure_list *list, grpc_closure *closure, + and set \a closure's result to \a error + Returns true if \a list becomes non-empty */ +bool grpc_closure_list_append(grpc_closure_list *list, grpc_closure *closure, grpc_error *error); /** force all success bits in \a list to false */ diff --git a/src/core/lib/transport/bdp_estimator.c b/src/core/lib/transport/bdp_estimator.c index d9129925b3..4c2c1d3e8c 100644 --- a/src/core/lib/transport/bdp_estimator.c +++ b/src/core/lib/transport/bdp_estimator.c @@ -41,7 +41,7 @@ void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator) { estimator->num_samples = 0; estimator->first_sample_idx = 0; - estimator->sampling = false; + estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; } bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, @@ -68,17 +68,26 @@ static int64_t *sampling(grpc_bdp_estimator *estimator) { bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, int64_t num_bytes) { - if (estimator->sampling) { - *sampling(estimator) += num_bytes; - return false; - } else { - return true; + switch (estimator->ping_state) { + case GRPC_BDP_PING_UNSCHEDULED: + return true; + case GRPC_BDP_PING_SCHEDULED: + return false; + case GRPC_BDP_PING_STARTED: + *sampling(estimator) += num_bytes; + return false; } + GPR_UNREACHABLE_CODE(return false); +} + +void grpc_bdp_estimator_schedule_ping(grpc_bdp_estimator *estimator) { + GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_UNSCHEDULED); + estimator->ping_state = GRPC_BDP_PING_SCHEDULED; } void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator) { - GPR_ASSERT(!estimator->sampling); - estimator->sampling = true; + GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_SCHEDULED); + estimator->ping_state = GRPC_BDP_PING_STARTED; if (estimator->num_samples == GRPC_BDP_SAMPLES) { estimator->first_sample_idx++; estimator->num_samples--; @@ -87,7 +96,7 @@ void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator) { } void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator) { - GPR_ASSERT(estimator->sampling); + GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_STARTED); estimator->num_samples++; - estimator->sampling = false; + estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; } diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index e81699cbee..d812f90a35 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -40,11 +40,16 @@ #define GRPC_BDP_SAMPLES 16 #define GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE 3 +typedef enum { + GRPC_BDP_PING_UNSCHEDULED, + GRPC_BDP_PING_SCHEDULED, + GRPC_BDP_PING_STARTED +} grpc_bdp_estimator_ping_state; + typedef struct grpc_bdp_estimator { uint8_t num_samples; uint8_t first_sample_idx; - bool scheduled; - bool sampling; + grpc_bdp_estimator_ping_state ping_state; int64_t samples[GRPC_BDP_SAMPLES]; } grpc_bdp_estimator; @@ -53,12 +58,15 @@ void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator); // Returns true if a reasonable estimate could be obtained bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, int64_t *estimate); -// Returns true if the user should start a ping +// Returns true if the user should schedule a ping bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, int64_t num_bytes); -// Schedule a ping +// Schedule a ping: call in response to receiving a true from +// grpc_bdp_estimator_add_incoming_bytes once a ping has been scheduled by a +// transport (but not necessarily started) void grpc_bdp_estimator_schedule_ping(grpc_bdp_estimator *estimator); -// Start a ping +// Start a ping: call after calling grpc_bdp_estimator_schedule_ping and once +// the ping is on the wire void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator); // Completes a previously started ping void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator); -- cgit v1.2.3 From 91f77eacce6342329bb1995e600d39fe16720fa4 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 29 Dec 2016 16:59:43 -0800 Subject: PID controller stabilization --- .../transport/chttp2/transport/chttp2_transport.c | 59 ++++++++++++---------- src/core/ext/transport/chttp2/transport/internal.h | 6 +-- src/core/lib/transport/bdp_estimator.c | 34 +++---------- src/core/lib/transport/bdp_estimator.h | 5 +- 4 files changed, 45 insertions(+), 59 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 6d302b0f11..8cce283b7a 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -252,8 +252,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_bdp_estimator_init(&t->bdp_estimator); t->last_bdp_ping_finished = gpr_now(GPR_CLOCK_MONOTONIC); t->last_pid_update = t->last_bdp_ping_finished; - grpc_pid_controller_init(&t->pid_controller, 16, 8, 0); - t->bdp_guess = DEFAULT_WINDOW; + grpc_pid_controller_init(&t->pid_controller, 4, 4, 0); + t->log2_bdp_guess = log2(DEFAULT_WINDOW); grpc_chttp2_goaway_parser_init(&t->goaway_parser); grpc_chttp2_hpack_parser_init(&t->hpack_parser); @@ -1897,26 +1897,29 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, int64_t estimate = -1; double bdp_error = 0.0; if (grpc_bdp_estimator_get_estimate(&t->bdp_estimator, &estimate)) { - bdp_error = 2.0 * (double)estimate - t->bdp_guess; - } - gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); - gpr_timespec dt_timespec = gpr_time_sub(now, t->last_pid_update); - double dt = (double)dt_timespec.tv_sec + dt_timespec.tv_nsec * 1e-9; - if (dt > 3) { - grpc_pid_controller_reset(&t->pid_controller); - } - double memory_pressure = grpc_resource_quota_get_memory_pressure( - grpc_resource_user_get_quota(grpc_endpoint_get_resource_user(t->ep))); - if (memory_pressure > 0.8) { - bdp_error -= GPR_MAX(0, t->bdp_guess) * (memory_pressure - 0.8) / 0.2; - } - if (t->bdp_guess < 1e-6 && bdp_error < 0) { - bdp_error = 0; + double target = (double)estimate; + double memory_pressure = grpc_resource_quota_get_memory_pressure( + grpc_resource_user_get_quota(grpc_endpoint_get_resource_user(t->ep))); + if (memory_pressure > 0.8) { + target *= 1 - GPR_MIN(1, (memory_pressure - 0.8) / 0.1); + } + bdp_error = target > 0 ? log2(target) - t->log2_bdp_guess + : GPR_MIN(0, -t->log2_bdp_guess); + gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); + gpr_timespec dt_timespec = gpr_time_sub(now, t->last_pid_update); + double dt = (double)dt_timespec.tv_sec + dt_timespec.tv_nsec * 1e-9; + if (dt > 3) { + grpc_pid_controller_reset(&t->pid_controller); + } + t->log2_bdp_guess += + grpc_pid_controller_update(&t->pid_controller, bdp_error, dt); + t->log2_bdp_guess = GPR_CLAMP(t->log2_bdp_guess, -5, 21); + gpr_log(GPR_DEBUG, "%s: err=%lf cur=%lf pressure=%lf target=%lf", + t->peer_string, bdp_error, t->log2_bdp_guess, memory_pressure, + target); + update_bdp(exec_ctx, t, pow(2, t->log2_bdp_guess)); + t->last_pid_update = now; } - t->bdp_guess += - grpc_pid_controller_update(&t->pid_controller, bdp_error, dt); - update_bdp(exec_ctx, t, t->bdp_guess); - t->last_pid_update = now; GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "keep_reading"); } else { GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "reading_action"); @@ -1929,21 +1932,23 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, GPR_TIMER_END("reading_action_locked", 0); } +static void start_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, + grpc_error *error) { + grpc_chttp2_transport *t = tp; + gpr_log(GPR_DEBUG, "%s: Start BDP ping", t->peer_string); + grpc_bdp_estimator_start_ping(&t->bdp_estimator); +} + static void finish_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, grpc_error *error) { grpc_chttp2_transport *t = tp; + gpr_log(GPR_DEBUG, "%s: Complete BDP ping", t->peer_string); grpc_bdp_estimator_complete_ping(&t->bdp_estimator); t->last_bdp_ping_finished = gpr_now(GPR_CLOCK_MONOTONIC); GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "bdp_ping"); } -static void start_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, - grpc_error *error) { - grpc_chttp2_transport *t = tp; - grpc_bdp_estimator_start_ping(&t->bdp_estimator); -} - /******************************************************************************* * CALLBACK LOOP */ diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 90abf386c7..e320dd091d 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -82,8 +82,8 @@ typedef enum { } grpc_chttp2_ping_type; typedef enum { - GRPC_CHTTP2_PCL_NEXT = 0, - GRPC_CHTTP2_PCL_INITIATE, + GRPC_CHTTP2_PCL_INITIATE = 0, + GRPC_CHTTP2_PCL_NEXT, GRPC_CHTTP2_PCL_INFLIGHT, GRPC_CHTTP2_PCL_COUNT /* must be last */ } grpc_chttp2_ping_closure_list; @@ -330,7 +330,7 @@ struct grpc_chttp2_transport { /* bdp estimator */ grpc_bdp_estimator bdp_estimator; grpc_pid_controller pid_controller; - double bdp_guess; + double log2_bdp_guess; grpc_closure start_bdp_ping_locked; grpc_closure finish_bdp_ping_locked; gpr_timespec last_bdp_ping_finished; diff --git a/src/core/lib/transport/bdp_estimator.c b/src/core/lib/transport/bdp_estimator.c index 4c2c1d3e8c..90e4332023 100644 --- a/src/core/lib/transport/bdp_estimator.c +++ b/src/core/lib/transport/bdp_estimator.c @@ -39,33 +39,16 @@ #include void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator) { - estimator->num_samples = 0; - estimator->first_sample_idx = 0; + estimator->estimate = 65536; estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; } bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, int64_t *estimate) { - if (estimator->num_samples < GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE) { - return false; - } - - *estimate = -1; - for (uint8_t i = 0; i < estimator->num_samples; i++) { - *estimate = GPR_MAX( - *estimate, - estimator - ->samples[(estimator->first_sample_idx + i) % GRPC_BDP_SAMPLES]); - } + *estimate = estimator->estimate; return true; } -static int64_t *sampling(grpc_bdp_estimator *estimator) { - return &estimator - ->samples[(estimator->first_sample_idx + estimator->num_samples) % - GRPC_BDP_SAMPLES]; -} - bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, int64_t num_bytes) { switch (estimator->ping_state) { @@ -74,7 +57,7 @@ bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, case GRPC_BDP_PING_SCHEDULED: return false; case GRPC_BDP_PING_STARTED: - *sampling(estimator) += num_bytes; + estimator->accumulator += num_bytes; return false; } GPR_UNREACHABLE_CODE(return false); @@ -88,15 +71,14 @@ void grpc_bdp_estimator_schedule_ping(grpc_bdp_estimator *estimator) { void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator) { GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_SCHEDULED); estimator->ping_state = GRPC_BDP_PING_STARTED; - if (estimator->num_samples == GRPC_BDP_SAMPLES) { - estimator->first_sample_idx++; - estimator->num_samples--; - } - *sampling(estimator) = 0; + estimator->accumulator = 0; } void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator) { GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_STARTED); - estimator->num_samples++; + if (estimator->accumulator > 2 * estimator->estimate / 3) { + estimator->estimate *= 2; + gpr_log(GPR_DEBUG, "est --> %" PRId64, estimator->estimate); + } estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; } diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index d812f90a35..ea74f2b5d5 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -47,10 +47,9 @@ typedef enum { } grpc_bdp_estimator_ping_state; typedef struct grpc_bdp_estimator { - uint8_t num_samples; - uint8_t first_sample_idx; grpc_bdp_estimator_ping_state ping_state; - int64_t samples[GRPC_BDP_SAMPLES]; + int64_t accumulator; + int64_t estimate; } grpc_bdp_estimator; void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator); -- cgit v1.2.3 From 95234e1baa12c61aa6fbfefb7010e2ea54b52368 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 3 Jan 2017 10:48:10 -0800 Subject: Increase stability of integration for PID controller --- .../transport/chttp2/transport/chttp2_transport.c | 16 +++++++-------- src/core/ext/transport/chttp2/transport/internal.h | 1 - src/core/lib/transport/pid_controller.c | 24 +++++++++++++++++----- src/core/lib/transport/pid_controller.h | 10 +++++++-- test/core/transport/pid_controller_test.c | 11 +++++----- 5 files changed, 40 insertions(+), 22 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 8cce283b7a..5f3f3d855b 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -252,8 +252,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_bdp_estimator_init(&t->bdp_estimator); t->last_bdp_ping_finished = gpr_now(GPR_CLOCK_MONOTONIC); t->last_pid_update = t->last_bdp_ping_finished; - grpc_pid_controller_init(&t->pid_controller, 4, 4, 0); - t->log2_bdp_guess = log2(DEFAULT_WINDOW); + grpc_pid_controller_init(&t->pid_controller, log2(DEFAULT_WINDOW), 4, 4, 0); grpc_chttp2_goaway_parser_init(&t->goaway_parser); grpc_chttp2_hpack_parser_init(&t->hpack_parser); @@ -1903,21 +1902,22 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, if (memory_pressure > 0.8) { target *= 1 - GPR_MIN(1, (memory_pressure - 0.8) / 0.1); } - bdp_error = target > 0 ? log2(target) - t->log2_bdp_guess - : GPR_MIN(0, -t->log2_bdp_guess); + bdp_error = + target > 0 + ? log2(target) - grpc_pid_controller_last(&t->pid_controller) + : GPR_MIN(0, -grpc_pid_controller_last(&t->pid_controller)); gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); gpr_timespec dt_timespec = gpr_time_sub(now, t->last_pid_update); double dt = (double)dt_timespec.tv_sec + dt_timespec.tv_nsec * 1e-9; if (dt > 3) { grpc_pid_controller_reset(&t->pid_controller); } - t->log2_bdp_guess += + double log2_bdp_guess = grpc_pid_controller_update(&t->pid_controller, bdp_error, dt); - t->log2_bdp_guess = GPR_CLAMP(t->log2_bdp_guess, -5, 21); gpr_log(GPR_DEBUG, "%s: err=%lf cur=%lf pressure=%lf target=%lf", - t->peer_string, bdp_error, t->log2_bdp_guess, memory_pressure, + t->peer_string, bdp_error, log2_bdp_guess, memory_pressure, target); - update_bdp(exec_ctx, t, pow(2, t->log2_bdp_guess)); + update_bdp(exec_ctx, t, pow(2, log2_bdp_guess)); t->last_pid_update = now; } GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "keep_reading"); diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index e320dd091d..8b7c040358 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -330,7 +330,6 @@ struct grpc_chttp2_transport { /* bdp estimator */ grpc_bdp_estimator bdp_estimator; grpc_pid_controller pid_controller; - double log2_bdp_guess; grpc_closure start_bdp_ping_locked; grpc_closure finish_bdp_ping_locked; gpr_timespec last_bdp_ping_finished; diff --git a/src/core/lib/transport/pid_controller.c b/src/core/lib/transport/pid_controller.c index 3cef225d4b..ba56874503 100644 --- a/src/core/lib/transport/pid_controller.c +++ b/src/core/lib/transport/pid_controller.c @@ -34,7 +34,9 @@ #include "src/core/lib/transport/pid_controller.h" void grpc_pid_controller_init(grpc_pid_controller *pid_controller, - double gain_p, double gain_i, double gain_d) { + double initial_control_value, double gain_p, + double gain_i, double gain_d) { + pid_controller->last_control_value = initial_control_value; pid_controller->gain_p = gain_p; pid_controller->gain_i = gain_i; pid_controller->gain_d = gain_d; @@ -48,10 +50,22 @@ void grpc_pid_controller_reset(grpc_pid_controller *pid_controller) { double grpc_pid_controller_update(grpc_pid_controller *pid_controller, double error, double dt) { - pid_controller->error_integral += error * dt; + /* integrate error using the trapezoid rule */ + pid_controller->error_integral += + dt * (pid_controller->last_error + error) * 0.5; double diff_error = (error - pid_controller->last_error) / dt; + /* calculate derivative of control value vs time */ + double dc_dt = pid_controller->gain_p * error + + pid_controller->gain_i * pid_controller->error_integral + + pid_controller->gain_d * diff_error; + double new_control_value = pid_controller->last_control_value + + dt * (pid_controller->last_dc_dt + dc_dt) * 0.5; pid_controller->last_error = error; - return dt * (pid_controller->gain_p * error + - pid_controller->gain_i * pid_controller->error_integral + - pid_controller->gain_d * diff_error); + pid_controller->last_dc_dt = dc_dt; + pid_controller->last_control_value = new_control_value; + return new_control_value; +} + +double grpc_pid_controller_last(grpc_pid_controller *pid_controller) { + return pid_controller->last_control_value; } diff --git a/src/core/lib/transport/pid_controller.h b/src/core/lib/transport/pid_controller.h index 059b5b0834..dd2b120052 100644 --- a/src/core/lib/transport/pid_controller.h +++ b/src/core/lib/transport/pid_controller.h @@ -47,18 +47,24 @@ typedef struct { double gain_d; double last_error; double error_integral; + double last_control_value; + double last_dc_dt; } grpc_pid_controller; /** Initialize the controller */ void grpc_pid_controller_init(grpc_pid_controller *pid_controller, - double gain_p, double gain_i, double gain_d); + double initial_control_value, double gain_p, + double gain_i, double gain_d); /** Reset the controller: useful when things have changed significantly */ void grpc_pid_controller_reset(grpc_pid_controller *pid_controller); /** Update the controller: given a current error estimate, and the time since - the last update, returns a delta to the control value */ + the last update, returns a new control value */ double grpc_pid_controller_update(grpc_pid_controller *pid_controller, double error, double dt); +/** Returns the last control value calculated */ +double grpc_pid_controller_last(grpc_pid_controller *pid_controller); + #endif diff --git a/test/core/transport/pid_controller_test.c b/test/core/transport/pid_controller_test.c index 9614983b00..3935a25322 100644 --- a/test/core/transport/pid_controller_test.c +++ b/test/core/transport/pid_controller_test.c @@ -45,7 +45,7 @@ static void test_noop(void) { gpr_log(GPR_INFO, "test_noop"); grpc_pid_controller pid; - grpc_pid_controller_init(&pid, 1, 1, 1); + grpc_pid_controller_init(&pid, 0, 1, 1, 1); } static void test_simple_convergence(double gain_p, double gain_i, double gain_d, @@ -55,15 +55,14 @@ static void test_simple_convergence(double gain_p, double gain_i, double gain_d, "start=%lf", gain_p, gain_i, gain_d, dt, set_point, start); grpc_pid_controller pid; - grpc_pid_controller_init(&pid, 0.2, 0.1, 0.1); - - double current = start; + grpc_pid_controller_init(&pid, start, 0.2, 0.1, 0.1); for (int i = 0; i < 1000; i++) { - current += grpc_pid_controller_update(&pid, set_point - current, 1); + grpc_pid_controller_update(&pid, set_point - grpc_pid_controller_last(&pid), + 1); } - GPR_ASSERT(fabs(set_point - current) < 0.1); + GPR_ASSERT(fabs(set_point - grpc_pid_controller_last(&pid)) < 0.1); GPR_ASSERT(fabs(pid.error_integral) < 0.1); } -- cgit v1.2.3 From 68c9dbe69414bb19e0d810304c48dc417d82a4c8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 3 Jan 2017 11:31:34 -0800 Subject: Add clamping to pid controller, make arguments more readable --- .../transport/chttp2/transport/chttp2_transport.c | 10 +++++++++- src/core/lib/transport/pid_controller.c | 23 +++++++++++++--------- src/core/lib/transport/pid_controller.h | 11 +++++++++-- test/core/transport/pid_controller_test.c | 19 ++++++++++++++++-- 4 files changed, 49 insertions(+), 14 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 816b76cb83..c07b51d35f 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -252,7 +252,15 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_bdp_estimator_init(&t->bdp_estimator); t->last_bdp_ping_finished = gpr_now(GPR_CLOCK_MONOTONIC); t->last_pid_update = t->last_bdp_ping_finished; - grpc_pid_controller_init(&t->pid_controller, log2(DEFAULT_WINDOW), 4, 4, 0); + grpc_pid_controller_init( + &t->pid_controller, + (grpc_pid_controller_args){.gain_p = 4, + .gain_i = 8, + .gain_d = 0, + .initial_control_value = log2(DEFAULT_WINDOW), + .min_control_value = -1, + .max_control_value = 22, + .integral_range = 10}); grpc_chttp2_goaway_parser_init(&t->goaway_parser); grpc_chttp2_hpack_parser_init(&t->hpack_parser); diff --git a/src/core/lib/transport/pid_controller.c b/src/core/lib/transport/pid_controller.c index ba56874503..3a4845d7ef 100644 --- a/src/core/lib/transport/pid_controller.c +++ b/src/core/lib/transport/pid_controller.c @@ -32,14 +32,12 @@ */ #include "src/core/lib/transport/pid_controller.h" +#include void grpc_pid_controller_init(grpc_pid_controller *pid_controller, - double initial_control_value, double gain_p, - double gain_i, double gain_d) { - pid_controller->last_control_value = initial_control_value; - pid_controller->gain_p = gain_p; - pid_controller->gain_i = gain_i; - pid_controller->gain_d = gain_d; + grpc_pid_controller_args args) { + pid_controller->args = args; + pid_controller->last_control_value = args.initial_control_value; grpc_pid_controller_reset(pid_controller); } @@ -53,13 +51,20 @@ double grpc_pid_controller_update(grpc_pid_controller *pid_controller, /* integrate error using the trapezoid rule */ pid_controller->error_integral += dt * (pid_controller->last_error + error) * 0.5; + pid_controller->error_integral = GPR_CLAMP( + pid_controller->error_integral, -pid_controller->args.integral_range, + pid_controller->args.integral_range); double diff_error = (error - pid_controller->last_error) / dt; /* calculate derivative of control value vs time */ - double dc_dt = pid_controller->gain_p * error + - pid_controller->gain_i * pid_controller->error_integral + - pid_controller->gain_d * diff_error; + double dc_dt = pid_controller->args.gain_p * error + + pid_controller->args.gain_i * pid_controller->error_integral + + pid_controller->args.gain_d * diff_error; + /* and perform trapezoidal integration */ double new_control_value = pid_controller->last_control_value + dt * (pid_controller->last_dc_dt + dc_dt) * 0.5; + new_control_value = + GPR_CLAMP(new_control_value, pid_controller->args.min_control_value, + pid_controller->args.max_control_value); pid_controller->last_error = error; pid_controller->last_dc_dt = dc_dt; pid_controller->last_control_value = new_control_value; diff --git a/src/core/lib/transport/pid_controller.h b/src/core/lib/transport/pid_controller.h index dd2b120052..1bf2fbc564 100644 --- a/src/core/lib/transport/pid_controller.h +++ b/src/core/lib/transport/pid_controller.h @@ -45,16 +45,23 @@ typedef struct { double gain_p; double gain_i; double gain_d; + double initial_control_value; + double min_control_value; + double max_control_value; + double integral_range; +} grpc_pid_controller_args; + +typedef struct { double last_error; double error_integral; double last_control_value; double last_dc_dt; + grpc_pid_controller_args args; } grpc_pid_controller; /** Initialize the controller */ void grpc_pid_controller_init(grpc_pid_controller *pid_controller, - double initial_control_value, double gain_p, - double gain_i, double gain_d); + grpc_pid_controller_args args); /** Reset the controller: useful when things have changed significantly */ void grpc_pid_controller_reset(grpc_pid_controller *pid_controller); diff --git a/test/core/transport/pid_controller_test.c b/test/core/transport/pid_controller_test.c index 3935a25322..af53d5b8cb 100644 --- a/test/core/transport/pid_controller_test.c +++ b/test/core/transport/pid_controller_test.c @@ -33,6 +33,7 @@ #include "src/core/lib/transport/pid_controller.h" +#include #include #include @@ -45,7 +46,14 @@ static void test_noop(void) { gpr_log(GPR_INFO, "test_noop"); grpc_pid_controller pid; - grpc_pid_controller_init(&pid, 0, 1, 1, 1); + grpc_pid_controller_init( + &pid, (grpc_pid_controller_args){.gain_p = 1, + .gain_i = 1, + .gain_d = 1, + .initial_control_value = 1, + .min_control_value = DBL_MIN, + .max_control_value = DBL_MAX, + .integral_range = DBL_MAX}); } static void test_simple_convergence(double gain_p, double gain_i, double gain_d, @@ -55,7 +63,14 @@ static void test_simple_convergence(double gain_p, double gain_i, double gain_d, "start=%lf", gain_p, gain_i, gain_d, dt, set_point, start); grpc_pid_controller pid; - grpc_pid_controller_init(&pid, start, 0.2, 0.1, 0.1); + grpc_pid_controller_init( + &pid, (grpc_pid_controller_args){.gain_p = gain_p, + .gain_i = gain_i, + .gain_d = gain_d, + .initial_control_value = start, + .min_control_value = DBL_MIN, + .max_control_value = DBL_MAX, + .integral_range = DBL_MAX}); for (int i = 0; i < 1000; i++) { grpc_pid_controller_update(&pid, set_point - grpc_pid_controller_last(&pid), -- cgit v1.2.3 From 13a875d13eda8d885acd515dab12f8eb6e40f756 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 Jan 2017 15:15:13 -0800 Subject: Fix simple tests --- src/core/lib/transport/pid_controller.c | 1 + test/core/transport/bdp_estimator_test.c | 30 ++++++++++++++++++++++++------ test/core/transport/pid_controller_test.c | 6 ++++-- 3 files changed, 29 insertions(+), 8 deletions(-) (limited to 'src/core/lib/transport') diff --git a/src/core/lib/transport/pid_controller.c b/src/core/lib/transport/pid_controller.c index 3a4845d7ef..19cb1c0b36 100644 --- a/src/core/lib/transport/pid_controller.c +++ b/src/core/lib/transport/pid_controller.c @@ -43,6 +43,7 @@ void grpc_pid_controller_init(grpc_pid_controller *pid_controller, void grpc_pid_controller_reset(grpc_pid_controller *pid_controller) { pid_controller->last_error = 0.0; + pid_controller->last_dc_dt = 0.0; pid_controller->error_integral = 0.0; } diff --git a/test/core/transport/bdp_estimator_test.c b/test/core/transport/bdp_estimator_test.c index af011abf8f..1272c74b51 100644 --- a/test/core/transport/bdp_estimator_test.c +++ b/test/core/transport/bdp_estimator_test.c @@ -51,12 +51,14 @@ static void test_get_estimate_no_samples(void) { gpr_log(GPR_INFO, "test_get_estimate_no_samples"); grpc_bdp_estimator est; grpc_bdp_estimator_init(&est); - GPR_ASSERT(!grpc_bdp_estimator_get_estimate(&est, NULL)); + int64_t estimate; + grpc_bdp_estimator_get_estimate(&est, &estimate); } static void add_samples(grpc_bdp_estimator *estimator, int64_t *samples, size_t n) { GPR_ASSERT(grpc_bdp_estimator_add_incoming_bytes(estimator, 1234567) == true); + grpc_bdp_estimator_schedule_ping(estimator); grpc_bdp_estimator_start_ping(estimator); for (size_t i = 0; i < n; i++) { GPR_ASSERT(grpc_bdp_estimator_add_incoming_bytes(estimator, samples[i]) == @@ -74,7 +76,8 @@ static void test_get_estimate_1_sample(void) { grpc_bdp_estimator est; grpc_bdp_estimator_init(&est); add_sample(&est, 100); - GPR_ASSERT(!grpc_bdp_estimator_get_estimate(&est, NULL)); + int64_t estimate; + grpc_bdp_estimator_get_estimate(&est, &estimate); } static void test_get_estimate_2_samples(void) { @@ -83,7 +86,8 @@ static void test_get_estimate_2_samples(void) { grpc_bdp_estimator_init(&est); add_sample(&est, 100); add_sample(&est, 100); - GPR_ASSERT(!grpc_bdp_estimator_get_estimate(&est, NULL)); + int64_t estimate; + grpc_bdp_estimator_get_estimate(&est, &estimate); } static int64_t get_estimate(grpc_bdp_estimator *estimator) { @@ -99,7 +103,20 @@ static void test_get_estimate_3_samples(void) { add_sample(&est, 100); add_sample(&est, 100); add_sample(&est, 100); - GPR_ASSERT(get_estimate(&est) == 100); + int64_t estimate; + grpc_bdp_estimator_get_estimate(&est, &estimate); +} + +static int64_t next_pow_2(int64_t v) { + v--; + v |= v >> 1; + v |= v >> 2; + v |= v >> 4; + v |= v >> 8; + v |= v >> 16; + v |= v >> 32; + v++; + return v; } static void test_get_estimate_random_values(size_t n) { @@ -114,8 +131,9 @@ static void test_get_estimate_random_values(size_t n) { if (sample > max) max = sample; add_sample(&est, sample); if (i >= 3) { - GPR_ASSERT(get_estimate(&est) <= max); - GPR_ASSERT(get_estimate(&est) >= min); + gpr_log(GPR_DEBUG, "est:%" PRId64 " min:%d max:%d", get_estimate(&est), + min, max); + GPR_ASSERT(get_estimate(&est) <= 2 * next_pow_2(max)); } } } diff --git a/test/core/transport/pid_controller_test.c b/test/core/transport/pid_controller_test.c index af53d5b8cb..831343c815 100644 --- a/test/core/transport/pid_controller_test.c +++ b/test/core/transport/pid_controller_test.c @@ -72,13 +72,15 @@ static void test_simple_convergence(double gain_p, double gain_i, double gain_d, .max_control_value = DBL_MAX, .integral_range = DBL_MAX}); - for (int i = 0; i < 1000; i++) { + for (int i = 0; i < 100000; i++) { grpc_pid_controller_update(&pid, set_point - grpc_pid_controller_last(&pid), 1); } GPR_ASSERT(fabs(set_point - grpc_pid_controller_last(&pid)) < 0.1); - GPR_ASSERT(fabs(pid.error_integral) < 0.1); + if (gain_i > 0) { + GPR_ASSERT(fabs(pid.error_integral) < 0.1); + } } int main(int argc, char **argv) { -- cgit v1.2.3 From efbd7c2a0f82af841ad79998645e3729dee9676d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 27 Jan 2017 14:07:44 -0800 Subject: Bug fixes, tracing for bdp estimation --- doc/environment_variables.md | 5 ++- .../transport/chttp2/transport/chttp2_transport.c | 15 ++------ .../ext/transport/chttp2/transport/frame_data.c | 2 - src/core/ext/transport/chttp2/transport/internal.h | 3 -- src/core/ext/transport/chttp2/transport/writing.c | 45 ++++++++++++++-------- src/core/lib/surface/init.c | 2 + src/core/lib/transport/bdp_estimator.c | 26 +++++++++++-- src/core/lib/transport/bdp_estimator.h | 5 ++- test/core/util/passthru_endpoint.c | 4 +- 9 files changed, 67 insertions(+), 40 deletions(-) (limited to 'src/core/lib/transport') diff --git a/doc/environment_variables.md b/doc/environment_variables.md index 832762a5f5..3bcbd82130 100644 --- a/doc/environment_variables.md +++ b/doc/environment_variables.md @@ -35,6 +35,7 @@ some configuration as environment variables that can be set. A comma separated list of tracers that provide additional insight into how gRPC C core is processing requests via debug logs. Available tracers include: - api - traces api calls to the C core + - bdp_estimator - traces behavior of bdp estimation logic - channel - traces operations on the C core channel stack - combiner - traces combiner lock state - compression - traces compression operations @@ -55,10 +56,10 @@ some configuration as environment variables that can be set. - secure_endpoint - traces bytes flowing through encrypted channels - transport_security - traces metadata about secure channel establishment - tcp - traces bytes in and out of a channel - + 'all' can additionally be used to turn all traces on. Individual traces can be disabled by prefixing them with '-'. - + Example: export GRPC_TRACE=all,-pending_tags diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 17ed3a2dcb..f60256ae2d 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -254,9 +254,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_closure_init(&t->finish_bdp_ping_locked, finish_bdp_ping_locked, t, grpc_combiner_scheduler(t->combiner, false)); - grpc_bdp_estimator_init(&t->bdp_estimator); - t->last_bdp_ping_finished = gpr_now(GPR_CLOCK_MONOTONIC); - t->last_pid_update = t->last_bdp_ping_finished; + grpc_bdp_estimator_init(&t->bdp_estimator, t->peer_string); + t->last_pid_update = gpr_now(GPR_CLOCK_MONOTONIC); grpc_pid_controller_init( &t->pid_controller, (grpc_pid_controller_args){.gain_p = 4, @@ -1887,10 +1886,6 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, errors[1] = grpc_chttp2_perform_read(exec_ctx, t, t->read_buffer.slices[i]); } - if (!t->parse_saw_data_frames) { - need_bdp_ping = false; - } - t->parse_saw_data_frames = false; if (errors[1] != GRPC_ERROR_NONE) { errors[2] = try_http_parsing(exec_ctx, t); GRPC_ERROR_UNREF(error); @@ -1933,10 +1928,7 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, grpc_endpoint_read(exec_ctx, t->ep, &t->read_buffer, &t->read_action_locked); - if (need_bdp_ping && - gpr_time_cmp(gpr_time_add(t->last_bdp_ping_finished, - gpr_time_from_millis(100, GPR_TIMESPAN)), - gpr_now(GPR_CLOCK_MONOTONIC)) < 0) { + if (need_bdp_ping) { GRPC_CHTTP2_REF_TRANSPORT(t, "bdp_ping"); grpc_bdp_estimator_schedule_ping(&t->bdp_estimator); send_ping_locked(exec_ctx, t, @@ -1992,7 +1984,6 @@ static void finish_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, gpr_log(GPR_DEBUG, "%s: Complete BDP ping", t->peer_string); } grpc_bdp_estimator_complete_ping(&t->bdp_estimator); - t->last_bdp_ping_finished = gpr_now(GPR_CLOCK_MONOTONIC); GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "bdp_ping"); } diff --git a/src/core/ext/transport/chttp2/transport/frame_data.c b/src/core/ext/transport/chttp2/transport/frame_data.c index 4562983e7c..f9b9e1b309 100644 --- a/src/core/ext/transport/chttp2/transport/frame_data.c +++ b/src/core/ext/transport/chttp2/transport/frame_data.c @@ -156,8 +156,6 @@ static grpc_error *parse_inner(grpc_exec_ctx *exec_ctx, return GRPC_ERROR_NONE; } - t->parse_saw_data_frames = true; - switch (p->state) { case GRPC_CHTTP2_DATA_ERROR: p->state = GRPC_CHTTP2_DATA_ERROR; diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 6003eebe8d..b9474a92e7 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -322,8 +322,6 @@ struct grpc_chttp2_transport { /** initial window change */ int64_t initial_window_update; - /** did the current parse see actual data bytes? */ - bool parse_saw_data_frames; /** window available for peer to send to us */ int64_t incoming_window; @@ -357,7 +355,6 @@ struct grpc_chttp2_transport { grpc_pid_controller pid_controller; grpc_closure start_bdp_ping_locked; grpc_closure finish_bdp_ping_locked; - gpr_timespec last_bdp_ping_finished; gpr_timespec last_pid_update; /* if non-NULL, close the transport with this error when writes are finished diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index 343bc7aaae..438a062d10 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -74,22 +74,33 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, } if (!grpc_closure_list_empty(pq->lists[GRPC_CHTTP2_PCL_INFLIGHT])) { /* ping already in-flight: wait */ - gpr_log(GPR_DEBUG, "already pinging"); + if (grpc_http_trace || grpc_bdp_estimator_trace) { + gpr_log(GPR_DEBUG, "Ping delayed [%p]: already pinging", t->peer_string); + } return; } if (t->ping_state.pings_before_data_required == 0 && t->ping_policy.max_pings_without_data != 0) { /* need to send something of substance before sending a ping again */ - gpr_log(GPR_DEBUG, "too many pings: %d/%d", - t->ping_state.pings_before_data_required, - t->ping_policy.max_pings_without_data); + if (grpc_http_trace || grpc_bdp_estimator_trace) { + gpr_log(GPR_DEBUG, "Ping delayed [%p]: too many recent pings: %d/%d", + t->peer_string, t->ping_state.pings_before_data_required, + t->ping_policy.max_pings_without_data); + } return; } gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); - if (gpr_time_cmp(gpr_time_sub(now, t->ping_state.last_ping_sent_time), - t->ping_policy.min_time_between_pings) < 0) { + gpr_timespec elapsed = gpr_time_sub(now, t->ping_state.last_ping_sent_time); + /*gpr_log(GPR_DEBUG, "elapsed:%d.%09d min:%d.%09d", (int)elapsed.tv_sec, + elapsed.tv_nsec, (int)t->ping_policy.min_time_between_pings.tv_sec, + (int)t->ping_policy.min_time_between_pings.tv_nsec);*/ + if (gpr_time_cmp(elapsed, t->ping_policy.min_time_between_pings) < 0) { /* not enough elapsed time between successive pings */ - gpr_log(GPR_DEBUG, "not enough time"); + if (grpc_http_trace || grpc_bdp_estimator_trace) { + gpr_log(GPR_DEBUG, + "Ping delayed [%p]: not enough time elapsed since last ping", + t->peer_string); + } return; } /* coalesce equivalent pings into this one */ @@ -297,20 +308,14 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx, } } - for (size_t i = 0; i < t->ping_ack_count; i++) { - grpc_slice_buffer_add(&t->outbuf, - grpc_chttp2_ping_create(1, t->ping_acks[i])); - } - t->ping_ack_count = 0; - /* if the grpc_chttp2_transport is ready to send a window update, do so here also; 3/4 is a magic number that will likely get tuned soon */ uint32_t target_incoming_window = GPR_MAX( t->settings[GRPC_SENT_SETTINGS][GRPC_CHTTP2_SETTINGS_INITIAL_WINDOW_SIZE], 1024); uint32_t threshold_to_send_transport_window_update = - t->outbuf.count > 0 ? target_incoming_window - : 3 * target_incoming_window / 4; + t->outbuf.count > 0 ? 3 * target_incoming_window / 4 + : target_incoming_window / 2; if (t->incoming_window < threshold_to_send_transport_window_update) { maybe_initiate_ping(exec_ctx, t, GRPC_CHTTP2_PING_BEFORE_TRANSPORT_WINDOW_UPDATE); @@ -324,7 +329,15 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx, t->ping_policy.max_pings_without_data; } - maybe_initiate_ping(exec_ctx, t, GRPC_CHTTP2_PING_ON_NEXT_WRITE); + for (size_t i = 0; i < t->ping_ack_count; i++) { + grpc_slice_buffer_add(&t->outbuf, + grpc_chttp2_ping_create(1, t->ping_acks[i])); + } + t->ping_ack_count = 0; + + if (t->outbuf.count > 0) { + maybe_initiate_ping(exec_ctx, t, GRPC_CHTTP2_PING_ON_NEXT_WRITE); + } GPR_TIMER_END("grpc_chttp2_begin_write", 0); diff --git a/src/core/lib/surface/init.c b/src/core/lib/surface/init.c index f61bf1582e..cf721be896 100644 --- a/src/core/lib/surface/init.c +++ b/src/core/lib/surface/init.c @@ -62,6 +62,7 @@ #include "src/core/lib/surface/init.h" #include "src/core/lib/surface/lame_client.h" #include "src/core/lib/surface/server.h" +#include "src/core/lib/transport/bdp_estimator.h" #include "src/core/lib/transport/connectivity_state.h" #include "src/core/lib/transport/transport_impl.h" @@ -190,6 +191,7 @@ void grpc_init(void) { grpc_register_tracer("queue_pluck", &grpc_cq_pluck_trace); grpc_register_tracer("combiner", &grpc_combiner_trace); grpc_register_tracer("server_channel", &grpc_server_channel_trace); + grpc_register_tracer("bdp_estimator", &grpc_bdp_estimator_trace); // Default pluck trace to 1 grpc_cq_pluck_trace = 1; grpc_register_tracer("queue_timeout", &grpc_cq_event_timeout_trace); diff --git a/src/core/lib/transport/bdp_estimator.c b/src/core/lib/transport/bdp_estimator.c index 90e4332023..e1483677fd 100644 --- a/src/core/lib/transport/bdp_estimator.c +++ b/src/core/lib/transport/bdp_estimator.c @@ -38,9 +38,12 @@ #include #include -void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator) { +int grpc_bdp_estimator_trace = 0; + +void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator, const char *name) { estimator->estimate = 65536; estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; + estimator->name = name; } bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, @@ -51,34 +54,51 @@ bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, bool grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, int64_t num_bytes) { + estimator->accumulator += num_bytes; switch (estimator->ping_state) { case GRPC_BDP_PING_UNSCHEDULED: return true; case GRPC_BDP_PING_SCHEDULED: return false; case GRPC_BDP_PING_STARTED: - estimator->accumulator += num_bytes; return false; } GPR_UNREACHABLE_CODE(return false); } void grpc_bdp_estimator_schedule_ping(grpc_bdp_estimator *estimator) { + if (grpc_bdp_estimator_trace) { + gpr_log(GPR_DEBUG, "bdp[%s]:sched acc=%" PRId64 " est=%" PRId64, + estimator->name, estimator->accumulator, estimator->estimate); + } GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_UNSCHEDULED); estimator->ping_state = GRPC_BDP_PING_SCHEDULED; + estimator->accumulator = 0; } void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator) { + if (grpc_bdp_estimator_trace) { + gpr_log(GPR_DEBUG, "bdp[%s]:start acc=%" PRId64 " est=%" PRId64, + estimator->name, estimator->accumulator, estimator->estimate); + } GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_SCHEDULED); estimator->ping_state = GRPC_BDP_PING_STARTED; estimator->accumulator = 0; } void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator) { + if (grpc_bdp_estimator_trace) { + gpr_log(GPR_DEBUG, "bdp[%s]:complete acc=%" PRId64 " est=%" PRId64, + estimator->name, estimator->accumulator, estimator->estimate); + } GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_STARTED); if (estimator->accumulator > 2 * estimator->estimate / 3) { estimator->estimate *= 2; - gpr_log(GPR_DEBUG, "est --> %" PRId64, estimator->estimate); + if (grpc_bdp_estimator_trace) { + gpr_log(GPR_DEBUG, "bdp[%s]: estimate increased to %" PRId64, + estimator->name, estimator->estimate); + } } estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; + estimator->accumulator = 0; } diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index ea74f2b5d5..bcaf899910 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -40,6 +40,8 @@ #define GRPC_BDP_SAMPLES 16 #define GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE 3 +extern int grpc_bdp_estimator_trace; + typedef enum { GRPC_BDP_PING_UNSCHEDULED, GRPC_BDP_PING_SCHEDULED, @@ -50,9 +52,10 @@ typedef struct grpc_bdp_estimator { grpc_bdp_estimator_ping_state ping_state; int64_t accumulator; int64_t estimate; + const char *name; } grpc_bdp_estimator; -void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator); +void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator, const char *name); // Returns true if a reasonable estimate could be obtained bool grpc_bdp_estimator_get_estimate(grpc_bdp_estimator *estimator, diff --git a/test/core/util/passthru_endpoint.c b/test/core/util/passthru_endpoint.c index 96f541abe9..686f406b0a 100644 --- a/test/core/util/passthru_endpoint.c +++ b/test/core/util/passthru_endpoint.c @@ -148,7 +148,9 @@ static void me_destroy(grpc_exec_ctx *exec_ctx, grpc_endpoint *ep) { } static char *me_get_peer(grpc_endpoint *ep) { - return gpr_strdup("fake:mock_endpoint"); + passthru_endpoint *p = ((half *)ep)->parent; + return ((half *)ep) == &p->client ? gpr_strdup("fake:mock_client_endpoint") + : gpr_strdup("fake:mock_server_endpoint"); } static int me_get_fd(grpc_endpoint *ep) { return -1; } -- cgit v1.2.3