diff --git a/BUILD b/BUILD index 8ccc7480397..d8bb109492a 100644 --- a/BUILD +++ b/BUILD @@ -515,6 +515,7 @@ grpc_cc_library( "src/core/lib/support/atomic_with_std.h", "src/core/lib/support/env.h", "src/core/lib/support/memory.h", + "src/core/lib/support/manual_constructor.h", "src/core/lib/support/mpscq.h", "src/core/lib/support/murmur_hash.h", "src/core/lib/support/spinlock.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index 33e613c42fc..e8137ea73b3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -381,7 +381,6 @@ add_dependencies(buildtests_c alpn_test) add_dependencies(buildtests_c arena_test) add_dependencies(buildtests_c backoff_test) add_dependencies(buildtests_c bad_server_response_test) -add_dependencies(buildtests_c bdp_estimator_test) add_dependencies(buildtests_c bin_decoder_test) add_dependencies(buildtests_c bin_encoder_test) add_dependencies(buildtests_c byte_stream_test) @@ -636,6 +635,7 @@ add_custom_target(buildtests_cxx) add_dependencies(buildtests_cxx alarm_cpp_test) add_dependencies(buildtests_cxx async_end2end_test) add_dependencies(buildtests_cxx auth_property_iterator_test) +add_dependencies(buildtests_cxx bdp_estimator_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx bm_arena) endif() @@ -1616,7 +1616,7 @@ add_library(grpc_test_util test/core/end2end/fixtures/http_proxy_fixture.c test/core/end2end/fixtures/proxy.c test/core/iomgr/endpoint_tests.c - test/core/util/debugger_macros.c + test/core/util/debugger_macros.cc test/core/util/grpc_profiler.c test/core/util/memory_counters.c test/core/util/mock_endpoint.c @@ -1880,7 +1880,7 @@ add_library(grpc_test_util_unsecure test/core/end2end/fixtures/http_proxy_fixture.c test/core/end2end/fixtures/proxy.c test/core/iomgr/endpoint_tests.c - test/core/util/debugger_macros.c + test/core/util/debugger_macros.cc test/core/util/grpc_profiler.c test/core/util/memory_counters.c test/core/util/mock_endpoint.c @@ -5259,35 +5259,6 @@ target_link_libraries(bad_server_response_test endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) -add_executable(bdp_estimator_test - test/core/transport/bdp_estimator_test.c -) - - -target_include_directories(bdp_estimator_test - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} - PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include - PRIVATE ${BORINGSSL_ROOT_DIR}/include - PRIVATE ${PROTOBUF_ROOT_DIR}/src - PRIVATE ${BENCHMARK_ROOT_DIR}/include - PRIVATE ${ZLIB_ROOT_DIR} - PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/zlib - PRIVATE ${CARES_INCLUDE_DIR} - PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/cares/cares - PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/gflags/include -) - -target_link_libraries(bdp_estimator_test - ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util - grpc - gpr_test_util - gpr -) - -endif (gRPC_BUILD_TESTS) -if (gRPC_BUILD_TESTS) - add_executable(bin_decoder_test test/core/transport/chttp2/bin_decoder_test.c ) @@ -9275,6 +9246,46 @@ target_link_libraries(auth_property_iterator_test ${_gRPC_GFLAGS_LIBRARIES} ) +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + +add_executable(bdp_estimator_test + test/core/transport/bdp_estimator_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + + +target_include_directories(bdp_estimator_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${BORINGSSL_ROOT_DIR}/include + PRIVATE ${PROTOBUF_ROOT_DIR}/src + PRIVATE ${BENCHMARK_ROOT_DIR}/include + PRIVATE ${ZLIB_ROOT_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/zlib + PRIVATE ${CARES_INCLUDE_DIR} + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/cares/cares + PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/third_party/gflags/include + PRIVATE third_party/googletest/googletest/include + PRIVATE third_party/googletest/googletest + PRIVATE third_party/googletest/googlemock/include + PRIVATE third_party/googletest/googlemock + PRIVATE ${_gRPC_PROTO_GENS_DIR} +) + +target_link_libraries(bdp_estimator_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc++_test_util + grpc++ + grpc_test_util + grpc + gpr_test_util + gpr + ${_gRPC_GFLAGS_LIBRARIES} +) + endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) diff --git a/Makefile b/Makefile index 3960d2c115b..b42d710e2c1 100644 --- a/Makefile +++ b/Makefile @@ -952,7 +952,6 @@ api_fuzzer: $(BINDIR)/$(CONFIG)/api_fuzzer arena_test: $(BINDIR)/$(CONFIG)/arena_test backoff_test: $(BINDIR)/$(CONFIG)/backoff_test 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 byte_stream_test: $(BINDIR)/$(CONFIG)/byte_stream_test @@ -1101,6 +1100,7 @@ wakeup_fd_cv_test: $(BINDIR)/$(CONFIG)/wakeup_fd_cv_test alarm_cpp_test: $(BINDIR)/$(CONFIG)/alarm_cpp_test async_end2end_test: $(BINDIR)/$(CONFIG)/async_end2end_test auth_property_iterator_test: $(BINDIR)/$(CONFIG)/auth_property_iterator_test +bdp_estimator_test: $(BINDIR)/$(CONFIG)/bdp_estimator_test bm_arena: $(BINDIR)/$(CONFIG)/bm_arena bm_call_create: $(BINDIR)/$(CONFIG)/bm_call_create bm_chttp2_hpack: $(BINDIR)/$(CONFIG)/bm_chttp2_hpack @@ -1352,7 +1352,6 @@ buildtests_c: privatelibs_c \ $(BINDIR)/$(CONFIG)/arena_test \ $(BINDIR)/$(CONFIG)/backoff_test \ $(BINDIR)/$(CONFIG)/bad_server_response_test \ - $(BINDIR)/$(CONFIG)/bdp_estimator_test \ $(BINDIR)/$(CONFIG)/bin_decoder_test \ $(BINDIR)/$(CONFIG)/bin_encoder_test \ $(BINDIR)/$(CONFIG)/byte_stream_test \ @@ -1545,6 +1544,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/alarm_cpp_test \ $(BINDIR)/$(CONFIG)/async_end2end_test \ $(BINDIR)/$(CONFIG)/auth_property_iterator_test \ + $(BINDIR)/$(CONFIG)/bdp_estimator_test \ $(BINDIR)/$(CONFIG)/bm_arena \ $(BINDIR)/$(CONFIG)/bm_call_create \ $(BINDIR)/$(CONFIG)/bm_chttp2_hpack \ @@ -1666,6 +1666,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/alarm_cpp_test \ $(BINDIR)/$(CONFIG)/async_end2end_test \ $(BINDIR)/$(CONFIG)/auth_property_iterator_test \ + $(BINDIR)/$(CONFIG)/bdp_estimator_test \ $(BINDIR)/$(CONFIG)/bm_arena \ $(BINDIR)/$(CONFIG)/bm_call_create \ $(BINDIR)/$(CONFIG)/bm_chttp2_hpack \ @@ -1766,8 +1767,6 @@ test_c: buildtests_c $(Q) $(BINDIR)/$(CONFIG)/backoff_test || ( echo test backoff_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" @@ -2040,6 +2039,8 @@ test_cxx: buildtests_cxx $(Q) $(BINDIR)/$(CONFIG)/async_end2end_test || ( echo test async_end2end_test failed ; exit 1 ) $(E) "[RUN] Testing auth_property_iterator_test" $(Q) $(BINDIR)/$(CONFIG)/auth_property_iterator_test || ( echo test auth_property_iterator_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 bm_arena" $(Q) $(BINDIR)/$(CONFIG)/bm_arena || ( echo test bm_arena failed ; exit 1 ) $(E) "[RUN] Testing bm_call_create" @@ -3609,7 +3610,7 @@ LIBGRPC_TEST_UTIL_SRC = \ test/core/end2end/fixtures/http_proxy_fixture.c \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ - test/core/util/debugger_macros.c \ + test/core/util/debugger_macros.cc \ test/core/util/grpc_profiler.c \ test/core/util/memory_counters.c \ test/core/util/mock_endpoint.c \ @@ -3864,7 +3865,7 @@ LIBGRPC_TEST_UTIL_UNSECURE_SRC = \ test/core/end2end/fixtures/http_proxy_fixture.c \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ - test/core/util/debugger_macros.c \ + test/core/util/debugger_macros.cc \ test/core/util/grpc_profiler.c \ test/core/util/memory_counters.c \ test/core/util/mock_endpoint.c \ @@ -8964,38 +8965,6 @@ 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 \ @@ -13771,6 +13740,49 @@ endif endif +BDP_ESTIMATOR_TEST_SRC = \ + test/core/transport/bdp_estimator_test.cc \ + +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 + + + + +ifeq ($(NO_PROTOBUF),true) + +# You can't build the protoc plugins or protobuf-enabled targets if you don't have protobuf 3.0.0+. + +$(BINDIR)/$(CONFIG)/bdp_estimator_test: protobuf_dep_error + +else + +$(BINDIR)/$(CONFIG)/bdp_estimator_test: $(PROTOBUF_DEP) $(BDP_ESTIMATOR_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(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) $(LDXX) $(LDFLAGS) $(BDP_ESTIMATOR_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr_test_util.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBSXX) $(LDLIBS_PROTOBUF) $(LDLIBS) $(LDLIBS_SECURE) $(GTEST_LIB) -o $(BINDIR)/$(CONFIG)/bdp_estimator_test + +endif + +endif + +$(OBJDIR)/$(CONFIG)/test/core/transport/bdp_estimator_test.o: $(LIBDIR)/$(CONFIG)/libgrpc++_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(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 + + BM_ARENA_SRC = \ test/cpp/microbenchmarks/bm_arena.cc \ diff --git a/build.yaml b/build.yaml index 9acd8e0bd61..81d3526b92f 100644 --- a/build.yaml +++ b/build.yaml @@ -143,6 +143,7 @@ filegroups: - src/core/lib/support/atomic_with_atm.h - src/core/lib/support/atomic_with_std.h - src/core/lib/support/env.h + - src/core/lib/support/manual_constructor.h - src/core/lib/support/memory.h - src/core/lib/support/mpscq.h - src/core/lib/support/murmur_hash.h @@ -741,7 +742,7 @@ filegroups: - test/core/end2end/fixtures/http_proxy_fixture.c - test/core/end2end/fixtures/proxy.c - test/core/iomgr/endpoint_tests.c - - test/core/util/debugger_macros.c + - test/core/util/debugger_macros.cc - test/core/util/grpc_profiler.c - test/core/util/memory_counters.c - test/core/util/mock_endpoint.c @@ -1798,16 +1799,6 @@ targets: - gpr exclude_iomgrs: - uv -- 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 @@ -3460,6 +3451,18 @@ targets: - grpc - gpr_test_util - gpr +- name: bdp_estimator_test + build: test + language: c++ + src: + - test/core/transport/bdp_estimator_test.cc + deps: + - grpc++_test_util + - grpc++ + - grpc_test_util + - grpc + - gpr_test_util + - gpr - name: bm_arena build: test language: c++ diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index f19b672f5ac..ce1425a483a 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -191,6 +191,7 @@ Pod::Spec.new do |s| 'src/core/lib/support/atomic_with_atm.h', 'src/core/lib/support/atomic_with_std.h', 'src/core/lib/support/env.h', + 'src/core/lib/support/manual_constructor.h', 'src/core/lib/support/memory.h', 'src/core/lib/support/mpscq.h', 'src/core/lib/support/murmur_hash.h', @@ -736,6 +737,7 @@ Pod::Spec.new do |s| 'src/core/lib/support/atomic_with_atm.h', 'src/core/lib/support/atomic_with_std.h', 'src/core/lib/support/env.h', + 'src/core/lib/support/manual_constructor.h', 'src/core/lib/support/memory.h', 'src/core/lib/support/mpscq.h', 'src/core/lib/support/murmur_hash.h', diff --git a/grpc.gemspec b/grpc.gemspec index 85d2a2e393e..ab51cc8420b 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -89,6 +89,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/support/atomic_with_atm.h ) s.files += %w( src/core/lib/support/atomic_with_std.h ) s.files += %w( src/core/lib/support/env.h ) + s.files += %w( src/core/lib/support/manual_constructor.h ) s.files += %w( src/core/lib/support/memory.h ) s.files += %w( src/core/lib/support/mpscq.h ) s.files += %w( src/core/lib/support/murmur_hash.h ) diff --git a/grpc.gyp b/grpc.gyp index 53e388561f9..d5a20fa5558 100644 --- a/grpc.gyp +++ b/grpc.gyp @@ -515,7 +515,7 @@ 'test/core/end2end/fixtures/http_proxy_fixture.c', 'test/core/end2end/fixtures/proxy.c', 'test/core/iomgr/endpoint_tests.c', - 'test/core/util/debugger_macros.c', + 'test/core/util/debugger_macros.cc', 'test/core/util/grpc_profiler.c', 'test/core/util/memory_counters.c', 'test/core/util/mock_endpoint.c', @@ -722,7 +722,7 @@ 'test/core/end2end/fixtures/http_proxy_fixture.c', 'test/core/end2end/fixtures/proxy.c', 'test/core/iomgr/endpoint_tests.c', - 'test/core/util/debugger_macros.c', + 'test/core/util/debugger_macros.cc', 'test/core/util/grpc_profiler.c', 'test/core/util/memory_counters.c', 'test/core/util/mock_endpoint.c', diff --git a/package.xml b/package.xml index df0142124d5..e590745a67b 100644 --- a/package.xml +++ b/package.xml @@ -101,6 +101,7 @@ + diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index e4b19a2c4ac..0ef06ae6e00 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -218,6 +218,8 @@ static void destruct_transport(grpc_exec_ctx *exec_ctx, t->write_cb_pool = next; } + t->flow_control.bdp_estimator.Destroy(); + gpr_free(t->ping_acks); gpr_free(t->peer_string); gpr_free(t); @@ -315,7 +317,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, keepalive_watchdog_fired_locked, t, grpc_combiner_scheduler(t->combiner)); - grpc_bdp_estimator_init(&t->flow_control.bdp_estimator, t->peer_string); + t->flow_control.bdp_estimator.Init(t->peer_string); grpc_chttp2_goaway_parser_init(&t->goaway_parser); grpc_chttp2_hpack_parser_init(exec_ctx, &t->hpack_parser); @@ -2434,7 +2436,7 @@ void grpc_chttp2_act_on_flowctl_action(grpc_exec_ctx *exec_ctx, } if (action.need_ping) { GRPC_CHTTP2_REF_TRANSPORT(t, "bdp_ping"); - grpc_bdp_estimator_schedule_ping(&t->flow_control.bdp_estimator); + t->flow_control.bdp_estimator->SchedulePing(); send_ping_locked(exec_ctx, t, &t->start_bdp_ping_locked, &t->finish_bdp_ping_locked); } @@ -2493,8 +2495,7 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, grpc_error *errors[3] = {GRPC_ERROR_REF(error), GRPC_ERROR_NONE, GRPC_ERROR_NONE}; for (; i < t->read_buffer.count && errors[1] == GRPC_ERROR_NONE; i++) { - grpc_bdp_estimator_add_incoming_bytes( - &t->flow_control.bdp_estimator, + t->flow_control.bdp_estimator->AddIncomingBytes( (int64_t)GRPC_SLICE_LENGTH(t->read_buffer.slices[i])); errors[1] = grpc_chttp2_perform_read(exec_ctx, t, t->read_buffer.slices[i]); @@ -2569,7 +2570,7 @@ static void start_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, if (t->keepalive_state == GRPC_CHTTP2_KEEPALIVE_STATE_WAITING) { grpc_timer_cancel(exec_ctx, &t->keepalive_ping_timer); } - grpc_bdp_estimator_start_ping(&t->flow_control.bdp_estimator); + t->flow_control.bdp_estimator->StartPing(); } static void finish_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, @@ -2578,7 +2579,7 @@ static void finish_bdp_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, if (GRPC_TRACER_ON(grpc_http_trace)) { gpr_log(GPR_DEBUG, "%s: Complete BDP ping", t->peer_string); } - grpc_bdp_estimator_complete_ping(exec_ctx, &t->flow_control.bdp_estimator); + t->flow_control.bdp_estimator->CompletePing(exec_ctx); GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "bdp_ping"); } diff --git a/src/core/ext/transport/chttp2/transport/flow_control.cc b/src/core/ext/transport/chttp2/transport/flow_control.cc index 2428e2526d3..60c43d840a4 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.cc +++ b/src/core/ext/transport/chttp2/transport/flow_control.cc @@ -459,12 +459,11 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action( } } if (tfc->enable_bdp_probe) { - action.need_ping = - grpc_bdp_estimator_need_ping(exec_ctx, &tfc->bdp_estimator); + action.need_ping = tfc->bdp_estimator->NeedPing(exec_ctx); // get bdp estimate and update initial_window accordingly. int64_t estimate = -1; - if (grpc_bdp_estimator_get_estimate(&tfc->bdp_estimator, &estimate)) { + if (tfc->bdp_estimator->EstimateBdp(&estimate)) { double target = 1 + log2((double)estimate); // target might change based on how much memory pressure we are under @@ -491,7 +490,7 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action( // get bandwidth estimate and update max_frame accordingly. double bw_dbl = -1; - if (grpc_bdp_estimator_get_bw(&tfc->bdp_estimator, &bw_dbl)) { + if (tfc->bdp_estimator->EstimateBandwidth(&bw_dbl)) { // we target the max of BDP or bandwidth in microseconds. int32_t frame_size = (int32_t)GPR_CLAMP( GPR_MAX((int32_t)GPR_CLAMP(bw_dbl, 0, INT_MAX) / 1000, diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 5d27bb8d673..f7a57a6543e 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -37,6 +37,7 @@ #include "src/core/lib/iomgr/combiner.h" #include "src/core/lib/iomgr/endpoint.h" #include "src/core/lib/iomgr/timer.h" +#include "src/core/lib/support/manual_constructor.h" #include "src/core/lib/transport/bdp_estimator.h" #include "src/core/lib/transport/connectivity_state.h" #include "src/core/lib/transport/pid_controller.h" @@ -268,7 +269,7 @@ typedef struct { bool enable_bdp_probe; /* bdp estimation */ - grpc_bdp_estimator bdp_estimator; + grpc_core::ManualConstructor bdp_estimator; /* pid controller */ bool pid_controller_initialized; diff --git a/src/core/lib/support/manual_constructor.h b/src/core/lib/support/manual_constructor.h new file mode 100644 index 00000000000..d753cf98a0c --- /dev/null +++ b/src/core/lib/support/manual_constructor.h @@ -0,0 +1,76 @@ +/* + * + * Copyright 2016 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#ifndef GRPC_CORE_LIB_SUPPORT_MANUAL_CONSTRUCTOR_H +#define GRPC_CORE_LIB_SUPPORT_MANUAL_CONSTRUCTOR_H + +// manually construct a region of memory with some type + +#include +#include +#include +#include + +namespace grpc_core { + +template +class ManualConstructor { + public: + // No constructor or destructor because one of the most useful uses of + // this class is as part of a union, and members of a union could not have + // constructors or destructors till C++11. And, anyway, the whole point of + // this class is to bypass constructor and destructor. + + Type* get() { return reinterpret_cast(&space_); } + const Type* get() const { return reinterpret_cast(&space_); } + + Type* operator->() { return get(); } + const Type* operator->() const { return get(); } + + Type& operator*() { return *get(); } + const Type& operator*() const { return *get(); } + + void Init() { new (&space_) Type; } + + // Init() constructs the Type instance using the given arguments + // (which are forwarded to Type's constructor). + // + // Note that Init() with no arguments performs default-initialization, + // not zero-initialization (i.e it behaves the same as "new Type;", not + // "new Type();"), so it will leave non-class types uninitialized. + template + void Init(Ts&&... args) { + new (&space_) Type(std::forward(args)...); + } + + // Init() that is equivalent to copy and move construction. + // Enables usage like this: + // ManualConstructor> v; + // v.Init({1, 2, 3}); + void Init(const Type& x) { new (&space_) Type(x); } + void Init(Type&& x) { new (&space_) Type(std::move(x)); } + + void Destroy() { get()->~Type(); } + + private: + typename std::aligned_storage::type space_; +}; + +} // namespace grpc_core + +#endif diff --git a/src/core/lib/transport/bdp_estimator.cc b/src/core/lib/transport/bdp_estimator.cc index 6ed427ce5c6..2a1c97c84e3 100644 --- a/src/core/lib/transport/bdp_estimator.cc +++ b/src/core/lib/transport/bdp_estimator.cc @@ -21,117 +21,65 @@ #include #include -#include #include grpc_tracer_flag grpc_bdp_estimator_trace = GRPC_TRACER_INITIALIZER(false, "bdp_estimator"); -void grpc_bdp_estimator_init(grpc_bdp_estimator *estimator, const char *name) { - estimator->estimate = 65536; - estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; - estimator->ping_start_time = gpr_time_0(GPR_CLOCK_MONOTONIC); - estimator->next_ping_scheduled = 0; - estimator->name = name; - estimator->bw_est = 0; - estimator->inter_ping_delay = 100.0; // start at 100ms - estimator->stable_estimate_count = 0; -} - -bool grpc_bdp_estimator_get_estimate(const grpc_bdp_estimator *estimator, - int64_t *estimate) { - *estimate = estimator->estimate; - return true; -} - -bool grpc_bdp_estimator_get_bw(const grpc_bdp_estimator *estimator, - double *bw) { - *bw = estimator->bw_est; - return true; -} - -void grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, - int64_t num_bytes) { - estimator->accumulator += num_bytes; -} - -bool grpc_bdp_estimator_need_ping(grpc_exec_ctx *exec_ctx, - const grpc_bdp_estimator *estimator) { - switch (estimator->ping_state) { - case GRPC_BDP_PING_UNSCHEDULED: - return grpc_exec_ctx_now(exec_ctx) >= estimator->next_ping_scheduled; - case GRPC_BDP_PING_SCHEDULED: - return false; - case GRPC_BDP_PING_STARTED: - return false; - } - GPR_UNREACHABLE_CODE(return false); -} +namespace grpc_core { -void grpc_bdp_estimator_schedule_ping(grpc_bdp_estimator *estimator) { - if (GRPC_TRACER_ON(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; -} +BdpEstimator::BdpEstimator(const char *name) + : ping_state_(PingState::UNSCHEDULED), + accumulator_(0), + estimate_(65536), + ping_start_time_(gpr_time_0(GPR_CLOCK_MONOTONIC)), + next_ping_scheduled_(0), + inter_ping_delay_(100.0), // start at 100ms + stable_estimate_count_(0), + bw_est_(0), + name_(name) {} -void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator) { - if (GRPC_TRACER_ON(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; - estimator->ping_start_time = gpr_now(GPR_CLOCK_MONOTONIC); -} - -void grpc_bdp_estimator_complete_ping(grpc_exec_ctx *exec_ctx, - grpc_bdp_estimator *estimator) { +void BdpEstimator::CompletePing(grpc_exec_ctx *exec_ctx) { gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); - gpr_timespec dt_ts = gpr_time_sub(now, estimator->ping_start_time); + gpr_timespec dt_ts = gpr_time_sub(now, ping_start_time_); double dt = (double)dt_ts.tv_sec + 1e-9 * (double)dt_ts.tv_nsec; - double bw = dt > 0 ? ((double)estimator->accumulator / dt) : 0; - int start_inter_ping_delay = estimator->inter_ping_delay; + double bw = dt > 0 ? ((double)accumulator_ / dt) : 0; + int start_inter_ping_delay = inter_ping_delay_; if (GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { gpr_log(GPR_DEBUG, "bdp[%s]:complete acc=%" PRId64 " est=%" PRId64 " dt=%lf bw=%lfMbs bw_est=%lfMbs", - estimator->name, estimator->accumulator, estimator->estimate, dt, - bw / 125000.0, estimator->bw_est / 125000.0); + name_, accumulator_, estimate_, dt, bw / 125000.0, + bw_est_ / 125000.0); } - GPR_ASSERT(estimator->ping_state == GRPC_BDP_PING_STARTED); - if (estimator->accumulator > 2 * estimator->estimate / 3 && - bw > estimator->bw_est) { - estimator->estimate = - GPR_MAX(estimator->accumulator, estimator->estimate * 2); - estimator->bw_est = bw; + GPR_ASSERT(ping_state_ == PingState::STARTED); + if (accumulator_ > 2 * estimate_ / 3 && bw > bw_est_) { + estimate_ = GPR_MAX(accumulator_, estimate_ * 2); + bw_est_ = bw; if (GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { - gpr_log(GPR_DEBUG, "bdp[%s]: estimate increased to %" PRId64, - estimator->name, estimator->estimate); + gpr_log(GPR_DEBUG, "bdp[%s]: estimate increased to %" PRId64, name_, + estimate_); } - estimator->inter_ping_delay /= 2; // if the ping estimate changes, - // exponentially get faster at probing - } else if (estimator->inter_ping_delay < 10000) { - estimator->stable_estimate_count++; - if (estimator->stable_estimate_count >= 2) { - estimator->inter_ping_delay += + inter_ping_delay_ /= 2; // if the ping estimate changes, + // exponentially get faster at probing + } else if (inter_ping_delay_ < 10000) { + stable_estimate_count_++; + if (stable_estimate_count_ >= 2) { + inter_ping_delay_ += 100 + (int)(rand() * 100.0 / RAND_MAX); // if the ping estimate is steady, // slowly ramp down the probe time } } - if (start_inter_ping_delay != estimator->inter_ping_delay) { - estimator->stable_estimate_count = 0; + if (start_inter_ping_delay != inter_ping_delay_) { + stable_estimate_count_ = 0; if (GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { - gpr_log(GPR_DEBUG, "bdp[%s]:update_inter_time to %dms", estimator->name, - estimator->inter_ping_delay); + gpr_log(GPR_DEBUG, "bdp[%s]:update_inter_time to %dms", name_, + inter_ping_delay_); } } - estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; - estimator->accumulator = 0; - estimator->next_ping_scheduled = - grpc_exec_ctx_now(exec_ctx) + estimator->inter_ping_delay; + ping_state_ = PingState::UNSCHEDULED; + accumulator_ = 0; + next_ping_scheduled_ = grpc_exec_ctx_now(exec_ctx) + inter_ping_delay_; } + +} // namespace grpc_core diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index a9d986782ca..60b9fba0838 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -19,67 +19,95 @@ #ifndef GRPC_CORE_LIB_TRANSPORT_BDP_ESTIMATOR_H #define GRPC_CORE_LIB_TRANSPORT_BDP_ESTIMATOR_H -#include +#include #include #include + +#include +#include + #include "src/core/lib/debug/trace.h" #include "src/core/lib/iomgr/exec_ctx.h" -#define GRPC_BDP_SAMPLES 16 -#define GRPC_BDP_MIN_SAMPLES_FOR_ESTIMATE 3 +extern grpc_tracer_flag grpc_bdp_estimator_trace; -#ifdef __cplusplus -extern "C" { -#endif +namespace grpc_core { -extern grpc_tracer_flag grpc_bdp_estimator_trace; +class BdpEstimator { + public: + explicit BdpEstimator(const char *name); + ~BdpEstimator() {} + + // Returns true if a reasonable estimate could be obtained + bool EstimateBdp(int64_t *estimate_out) const { + *estimate_out = estimate_; + return true; + } + bool EstimateBandwidth(double *bw_out) const { + *bw_out = bw_est_; + return true; + } -typedef enum { - GRPC_BDP_PING_UNSCHEDULED, - GRPC_BDP_PING_SCHEDULED, - GRPC_BDP_PING_STARTED -} grpc_bdp_estimator_ping_state; + void AddIncomingBytes(int64_t num_bytes) { accumulator_ += num_bytes; } -typedef struct grpc_bdp_estimator { - grpc_bdp_estimator_ping_state ping_state; - int64_t accumulator; - int64_t estimate; + // Returns true if the user should schedule a ping + bool NeedPing(grpc_exec_ctx *exec_ctx) const { + switch (ping_state_) { + case PingState::UNSCHEDULED: + return grpc_exec_ctx_now(exec_ctx) >= next_ping_scheduled_; + case PingState::SCHEDULED: + case PingState::STARTED: + return false; + } + GPR_UNREACHABLE_CODE(return false); + } + + // 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 SchedulePing() { + if (GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { + gpr_log(GPR_DEBUG, "bdp[%s]:sched acc=%" PRId64 " est=%" PRId64, name_, + accumulator_, estimate_); + } + GPR_ASSERT(ping_state_ == PingState::UNSCHEDULED); + ping_state_ = PingState::SCHEDULED; + accumulator_ = 0; + } + + // Start a ping: call after calling grpc_bdp_estimator_schedule_ping and + // once + // the ping is on the wire + void StartPing() { + if (GRPC_TRACER_ON(grpc_bdp_estimator_trace)) { + gpr_log(GPR_DEBUG, "bdp[%s]:start acc=%" PRId64 " est=%" PRId64, name_, + accumulator_, estimate_); + } + GPR_ASSERT(ping_state_ == PingState::SCHEDULED); + ping_state_ = PingState::STARTED; + accumulator_ = 0; + ping_start_time_ = gpr_now(GPR_CLOCK_MONOTONIC); + } + + // Completes a previously started ping + void CompletePing(grpc_exec_ctx *exec_ctx); + + private: + enum class PingState { UNSCHEDULED, SCHEDULED, STARTED }; + + PingState ping_state_; + int64_t accumulator_; + int64_t estimate_; // when was the current ping started? - gpr_timespec ping_start_time; + gpr_timespec ping_start_time_; // when should the next ping start? - grpc_millis next_ping_scheduled; - int inter_ping_delay; - int stable_estimate_count; - double bw_est; - const char *name; -} grpc_bdp_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(const grpc_bdp_estimator *estimator, - int64_t *estimate); -// Tracks new bytes read. -bool grpc_bdp_estimator_get_bw(const grpc_bdp_estimator *estimator, double *bw); -// Returns true if the user should schedule a ping -void grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, - int64_t num_bytes); -// Returns true if the user should schedule a ping -bool grpc_bdp_estimator_need_ping(grpc_exec_ctx *exec_ctx, - const grpc_bdp_estimator *estimator); -// 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: 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_exec_ctx *exec_ctx, - grpc_bdp_estimator *estimator); - -#ifdef __cplusplus -} -#endif + grpc_millis next_ping_scheduled_; + int inter_ping_delay_; + int stable_estimate_count_; + double bw_est_; + const char *name_; +}; + +} // namespace grpc_core #endif /* GRPC_CORE_LIB_TRANSPORT_BDP_ESTIMATOR_H */ diff --git a/test/core/transport/BUILD b/test/core/transport/BUILD index 12e36132c83..ea5e577bd89 100644 --- a/test/core/transport/BUILD +++ b/test/core/transport/BUILD @@ -20,14 +20,17 @@ grpc_package(name = "test/core/transport") grpc_cc_test( name = "bdp_estimator_test", - srcs = ["bdp_estimator_test.c"], - language = "C", + srcs = ["bdp_estimator_test.cc"], + language = "C++", deps = [ "//:gpr", "//:grpc", "//test/core/util:gpr_test_util", "//test/core/util:grpc_test_util", ], + external_deps = [ + "gtest", + ], ) grpc_cc_test( diff --git a/test/core/transport/bdp_estimator_test.c b/test/core/transport/bdp_estimator_test.c deleted file mode 100644 index 4912ad58872..00000000000 --- a/test/core/transport/bdp_estimator_test.c +++ /dev/null @@ -1,162 +0,0 @@ -/* - * - * Copyright 2016 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -#include "src/core/lib/transport/bdp_estimator.h" - -#include -#include -#include -#include -#include -#include -#include "src/core/lib/iomgr/timer_manager.h" -#include "src/core/lib/support/string.h" -#include "test/core/util/test_config.h" - -extern gpr_timespec (*gpr_now_impl)(gpr_clock_type clock_type); - -static int g_clock = 0; - -static gpr_timespec fake_gpr_now(gpr_clock_type clock_type) { - return (gpr_timespec){ - .tv_sec = g_clock, .tv_nsec = 0, .clock_type = clock_type, - }; -} - -static void inc_time(void) { g_clock += 30; } - -static void test_noop(void) { - gpr_log(GPR_INFO, "test_noop"); - grpc_bdp_estimator est; - grpc_bdp_estimator_init(&est, "test"); -} - -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, "test"); - int64_t estimate; - grpc_bdp_estimator_get_estimate(&est, &estimate); -} - -static void add_samples(grpc_bdp_estimator *estimator, int64_t *samples, - size_t n) { - grpc_bdp_estimator_add_incoming_bytes(estimator, 1234567); - inc_time(); - grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; - GPR_ASSERT(grpc_bdp_estimator_need_ping(&exec_ctx, estimator) == true); - grpc_bdp_estimator_schedule_ping(estimator); - grpc_bdp_estimator_start_ping(estimator); - for (size_t i = 0; i < n; i++) { - grpc_bdp_estimator_add_incoming_bytes(estimator, samples[i]); - GPR_ASSERT(grpc_bdp_estimator_need_ping(&exec_ctx, estimator) == false); - } - gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), - gpr_time_from_millis(1, GPR_TIMESPAN))); - grpc_bdp_estimator_complete_ping(&exec_ctx, estimator); - grpc_exec_ctx_finish(&exec_ctx); -} - -static void add_sample(grpc_bdp_estimator *estimator, int64_t sample) { - add_samples(estimator, &sample, 1); -} - -static void test_get_estimate_1_sample(void) { - gpr_log(GPR_INFO, "test_get_estimate_1_sample"); - grpc_bdp_estimator est; - grpc_bdp_estimator_init(&est, "test"); - add_sample(&est, 100); - int64_t estimate; - grpc_bdp_estimator_get_estimate(&est, &estimate); -} - -static void test_get_estimate_2_samples(void) { - gpr_log(GPR_INFO, "test_get_estimate_2_samples"); - grpc_bdp_estimator est; - grpc_bdp_estimator_init(&est, "test"); - add_sample(&est, 100); - add_sample(&est, 100); - int64_t estimate; - grpc_bdp_estimator_get_estimate(&est, &estimate); -} - -static int64_t get_estimate(grpc_bdp_estimator *estimator) { - int64_t out; - GPR_ASSERT(grpc_bdp_estimator_get_estimate(estimator, &out)); - return out; -} - -static void test_get_estimate_3_samples(void) { - gpr_log(GPR_INFO, "test_get_estimate_3_samples"); - grpc_bdp_estimator est; - grpc_bdp_estimator_init(&est, "test"); - add_sample(&est, 100); - add_sample(&est, 100); - add_sample(&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) { - gpr_log(GPR_INFO, "test_get_estimate_random_values(%" PRIdPTR ")", n); - grpc_bdp_estimator est; - grpc_bdp_estimator_init(&est, "test"); - const int kMaxSample = 65535; - int min = kMaxSample; - int max = 0; - for (size_t i = 0; i < n; i++) { - int sample = rand() % (kMaxSample + 1); - if (sample < min) min = sample; - if (sample > max) max = sample; - add_sample(&est, sample); - if (i >= 3) { - gpr_log(GPR_DEBUG, "est:%" PRId64 " min:%d max:%d", get_estimate(&est), - min, max); - GPR_ASSERT(get_estimate(&est) <= GPR_MAX(65536, 2 * next_pow_2(max))); - } - } -} - -int main(int argc, char **argv) { - grpc_test_init(argc, argv); - gpr_now_impl = fake_gpr_now; - grpc_init(); - grpc_timer_manager_set_threading(false); - test_noop(); - test_get_estimate_no_samples(); - test_get_estimate_1_sample(); - test_get_estimate_2_samples(); - test_get_estimate_3_samples(); - for (size_t i = 3; i < 1000; i = i * 3 / 2) { - test_get_estimate_random_values(i); - } - grpc_shutdown(); - return 0; -} diff --git a/test/core/transport/bdp_estimator_test.cc b/test/core/transport/bdp_estimator_test.cc new file mode 100644 index 00000000000..bfa77217c3f --- /dev/null +++ b/test/core/transport/bdp_estimator_test.cc @@ -0,0 +1,158 @@ +/* + * + * Copyright 2016 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include "src/core/lib/transport/bdp_estimator.h" + +#include +#include +#include +#include +#include +#include +#include +#include "src/core/lib/iomgr/timer_manager.h" +#include "src/core/lib/support/string.h" +#include "test/core/util/test_config.h" + +extern "C" gpr_timespec (*gpr_now_impl)(gpr_clock_type clock_type); + +namespace grpc_core { +namespace testing { +namespace { +int g_clock = 0; + +gpr_timespec fake_gpr_now(gpr_clock_type clock_type) { + return (gpr_timespec){ + .tv_sec = g_clock, .tv_nsec = 0, .clock_type = clock_type, + }; +} + +void inc_time(void) { g_clock += 30; } +} // namespace + +TEST(BdpEstimatorTest, NoOp) { BdpEstimator est("test"); } + +TEST(BdpEstimatorTest, EstimateBdpNoSamples) { + BdpEstimator est("test"); + int64_t estimate; + est.EstimateBdp(&estimate); +} + +namespace { +void AddSamples(BdpEstimator *estimator, int64_t *samples, size_t n) { + estimator->AddIncomingBytes(1234567); + inc_time(); + grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + EXPECT_TRUE(estimator->NeedPing(&exec_ctx)); + estimator->SchedulePing(); + estimator->StartPing(); + for (size_t i = 0; i < n; i++) { + estimator->AddIncomingBytes(samples[i]); + EXPECT_FALSE(estimator->NeedPing(&exec_ctx)); + } + gpr_sleep_until(gpr_time_add(gpr_now(GPR_CLOCK_REALTIME), + gpr_time_from_millis(1, GPR_TIMESPAN))); + grpc_exec_ctx_invalidate_now(&exec_ctx); + estimator->CompletePing(&exec_ctx); + grpc_exec_ctx_finish(&exec_ctx); +} + +void AddSample(BdpEstimator *estimator, int64_t sample) { + AddSamples(estimator, &sample, 1); +} +} // namespace + +TEST(BdpEstimatorTest, GetEstimate1Sample) { + BdpEstimator est("test"); + AddSample(&est, 100); + int64_t estimate; + est.EstimateBdp(&estimate); +} + +TEST(BdpEstimatorTest, GetEstimate2Samples) { + BdpEstimator est("test"); + AddSample(&est, 100); + AddSample(&est, 100); + int64_t estimate; + est.EstimateBdp(&estimate); +} + +TEST(BdpEstimatorTest, GetEstimate3Samples) { + BdpEstimator est("test"); + AddSample(&est, 100); + AddSample(&est, 100); + AddSample(&est, 100); + int64_t estimate; + est.EstimateBdp(&estimate); +} + +namespace { +static int64_t GetEstimate(const BdpEstimator &estimator) { + int64_t out; + EXPECT_TRUE(estimator.EstimateBdp(&out)); + return out; +} + +int64_t NextPow2(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; +} +} // namespace + +class BdpEstimatorRandomTest : public ::testing::TestWithParam {}; + +TEST_P(BdpEstimatorRandomTest, GetEstimateRandomValues) { + BdpEstimator est("test"); + const int kMaxSample = 65535; + int min = kMaxSample; + int max = 0; + for (size_t i = 0; i < GetParam(); i++) { + int sample = rand() % (kMaxSample + 1); + if (sample < min) min = sample; + if (sample > max) max = sample; + AddSample(&est, sample); + if (i >= 3) { + EXPECT_LE(GetEstimate(est), GPR_MAX(65536, 2 * NextPow2(max))) + << " min:" << min << " max:" << max << " sample:" << sample; + } + } +} + +INSTANTIATE_TEST_CASE_P(TooManyNames, BdpEstimatorRandomTest, + ::testing::Values(3, 4, 6, 9, 13, 19, 28, 42, 63, 94, + 141, 211, 316, 474, 711)); +} // namespace testing +} // namespace grpc_core + +int main(int argc, char **argv) { + grpc_test_init(argc, argv); + gpr_now_impl = grpc_core::testing::fake_gpr_now; + grpc_init(); + grpc_timer_manager_set_threading(false); + ::testing::InitGoogleTest(&argc, argv); + int ret = RUN_ALL_TESTS(); + grpc_shutdown(); + return ret; +} diff --git a/test/core/util/BUILD b/test/core/util/BUILD index abb50a0c996..5844a17728f 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -31,10 +31,23 @@ grpc_cc_library( deps = ["//:gpr"], ) +grpc_cc_library( + name = "grpc_debugger_macros", + srcs = [ + "debugger_macros.cc", + ], + hdrs = [ + "debugger_macros.h", + ], + deps = [ + ":gpr_test_util", + "//:grpc_common", + ], +) + grpc_cc_library( name = "grpc_test_util_base", srcs = [ - "debugger_macros.c", "grpc_profiler.c", "mock_endpoint.c", "parse_hexstring.c", @@ -47,7 +60,6 @@ grpc_cc_library( "trickle_endpoint.c", ], hdrs = [ - "debugger_macros.h", "grpc_profiler.h", "mock_endpoint.h", "parse_hexstring.h", @@ -63,6 +75,7 @@ grpc_cc_library( deps = [ ":gpr_test_util", "//:grpc_common", + ":grpc_debugger_macros" ], ) diff --git a/test/core/util/debugger_macros.c b/test/core/util/debugger_macros.cc similarity index 97% rename from test/core/util/debugger_macros.c rename to test/core/util/debugger_macros.cc index ebe74f1fd6f..72384f2dd7f 100644 --- a/test/core/util/debugger_macros.c +++ b/test/core/util/debugger_macros.cc @@ -29,7 +29,7 @@ #include "src/core/lib/channel/connected_channel.h" #include "src/core/lib/surface/call.h" -void grpc_summon_debugger_macros() {} +extern "C" void grpc_summon_debugger_macros() {} grpc_stream *grpc_transport_stream_from_call(grpc_call *call) { grpc_call_stack *cs = grpc_call_get_call_stack(call); diff --git a/test/core/util/debugger_macros.h b/test/core/util/debugger_macros.h index c6b3720c5ad..24718d93075 100644 --- a/test/core/util/debugger_macros.h +++ b/test/core/util/debugger_macros.h @@ -19,6 +19,14 @@ #ifndef GRPC_TEST_CORE_UTIL_DEBUGGER_MACROS_H #define GRPC_TEST_CORE_UTIL_DEBUGGER_MACROS_H +#ifdef __cplusplus +extern "C" { +#endif /* __cplusplus */ + void grpc_summon_debugger_macros(); +#ifdef __cplusplus +} +#endif /* __cplusplus */ + #endif /* GRPC_TEST_CORE_UTIL_DEBUGGER_MACROS_H */ diff --git a/test/core/util/trickle_endpoint.h b/test/core/util/trickle_endpoint.h index 78f1eeeda2f..ca39638ba0d 100644 --- a/test/core/util/trickle_endpoint.h +++ b/test/core/util/trickle_endpoint.h @@ -21,6 +21,10 @@ #include "src/core/lib/iomgr/endpoint.h" +#ifdef __cplusplus +extern "C" { +#endif // __cplusplus + grpc_endpoint *grpc_trickle_endpoint_create(grpc_endpoint *wrap, double bytes_per_second); @@ -30,4 +34,8 @@ size_t grpc_trickle_endpoint_trickle(grpc_exec_ctx *exec_ctx, size_t grpc_trickle_get_backlog(grpc_endpoint *endpoint); +#ifdef __cplusplus +} +#endif // __cplusplus + #endif diff --git a/test/cpp/microbenchmarks/bm_chttp2_transport.cc b/test/cpp/microbenchmarks/bm_chttp2_transport.cc index 6f9dee78224..8ee3ae72686 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_transport.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_transport.cc @@ -26,14 +26,12 @@ #include #include #include -extern "C" { #include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" #include "src/core/ext/transport/chttp2/transport/internal.h" #include "src/core/lib/iomgr/closure.h" #include "src/core/lib/iomgr/resource_quota.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/transport/static_metadata.h" -} #include "test/cpp/microbenchmarks/helpers.h" #include "third_party/benchmark/include/benchmark/benchmark.h" diff --git a/test/cpp/microbenchmarks/bm_fullstack_trickle.cc b/test/cpp/microbenchmarks/bm_fullstack_trickle.cc index adb5e6657f2..06ae3429857 100644 --- a/test/cpp/microbenchmarks/bm_fullstack_trickle.cc +++ b/test/cpp/microbenchmarks/bm_fullstack_trickle.cc @@ -21,17 +21,15 @@ #include #include #include +#include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" +#include "src/core/ext/transport/chttp2/transport/internal.h" +#include "src/core/lib/iomgr/timer_manager.h" #include "src/core/lib/profiling/timers.h" #include "src/proto/grpc/testing/echo.grpc.pb.h" +#include "test/core/util/trickle_endpoint.h" #include "test/cpp/microbenchmarks/fullstack_context_mutators.h" #include "test/cpp/microbenchmarks/fullstack_fixtures.h" #include "test/cpp/util/test_config.h" -extern "C" { -#include "src/core/ext/transport/chttp2/transport/chttp2_transport.h" -#include "src/core/ext/transport/chttp2/transport/internal.h" -#include "src/core/lib/iomgr/timer_manager.h" -#include "test/core/util/trickle_endpoint.h" -} DEFINE_bool(log, false, "Log state to CSV files"); DEFINE_int32( diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index 0f7e8cd3a8d..bcf91e7b055 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -1031,6 +1031,7 @@ src/core/lib/support/atomic.h \ src/core/lib/support/atomic_with_atm.h \ src/core/lib/support/atomic_with_std.h \ src/core/lib/support/env.h \ +src/core/lib/support/manual_constructor.h \ src/core/lib/support/memory.h \ src/core/lib/support/mpscq.h \ src/core/lib/support/murmur_hash.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index f89cbf08cd1..8435178d2e2 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1321,6 +1321,7 @@ src/core/lib/support/log_android.cc \ src/core/lib/support/log_linux.cc \ src/core/lib/support/log_posix.cc \ src/core/lib/support/log_windows.cc \ +src/core/lib/support/manual_constructor.h \ src/core/lib/support/memory.h \ src/core/lib/support/mpscq.cc \ src/core/lib/support/mpscq.h \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index cda7b5f5a8a..2e18ed2e8a4 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -134,23 +134,6 @@ "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", @@ -2613,6 +2596,25 @@ "third_party": false, "type": "target" }, + { + "deps": [ + "gpr", + "gpr_test_util", + "grpc", + "grpc++", + "grpc++_test_util", + "grpc_test_util" + ], + "headers": [], + "is_filegroup": false, + "language": "c++", + "name": "bdp_estimator_test", + "src": [ + "test/core/transport/bdp_estimator_test.cc" + ], + "third_party": false, + "type": "target" + }, { "deps": [ "benchmark", @@ -7862,6 +7864,7 @@ "src/core/lib/support/atomic_with_atm.h", "src/core/lib/support/atomic_with_std.h", "src/core/lib/support/env.h", + "src/core/lib/support/manual_constructor.h", "src/core/lib/support/memory.h", "src/core/lib/support/mpscq.h", "src/core/lib/support/murmur_hash.h", @@ -7909,6 +7912,7 @@ "src/core/lib/support/atomic_with_atm.h", "src/core/lib/support/atomic_with_std.h", "src/core/lib/support/env.h", + "src/core/lib/support/manual_constructor.h", "src/core/lib/support/memory.h", "src/core/lib/support/mpscq.h", "src/core/lib/support/murmur_hash.h", @@ -8934,7 +8938,7 @@ "test/core/end2end/fixtures/proxy.h", "test/core/iomgr/endpoint_tests.c", "test/core/iomgr/endpoint_tests.h", - "test/core/util/debugger_macros.c", + "test/core/util/debugger_macros.cc", "test/core/util/debugger_macros.h", "test/core/util/grpc_profiler.c", "test/core/util/grpc_profiler.h", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index a1644dfa097..452afc7d96b 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -164,29 +164,6 @@ ], "uses_polling": true }, - { - "args": [], - "ci_platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "gtest": false, - "language": "c", - "name": "bdp_estimator_test", - "platforms": [ - "linux", - "mac", - "posix", - "windows" - ], - "uses_polling": true - }, { "args": [], "ci_platforms": [ @@ -2954,6 +2931,29 @@ ], "uses_polling": true }, + { + "args": [], + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": false, + "language": "c++", + "name": "bdp_estimator_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, { "args": [ "--benchmark_min_time=0"