diff --git a/CMakeLists.txt b/CMakeLists.txt index 86a77c956633ed4152d5f64b02aff17ba033bda9..ce632d9e1faa185439125f34beb54a6d9be14b43 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -731,6 +731,7 @@ add_dependencies(buildtests_cxx thread_stress_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) add_dependencies(buildtests_cxx time_change_test) endif() +add_dependencies(buildtests_cxx timer_test) add_dependencies(buildtests_cxx transport_pid_controller_test) add_dependencies(buildtests_cxx transport_security_common_api_test) if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX) @@ -17679,6 +17680,48 @@ endif() endif (gRPC_BUILD_TESTS) if (gRPC_BUILD_TESTS) +add_executable(timer_test + test/cpp/common/timer_test.cc + third_party/googletest/googletest/src/gtest-all.cc + third_party/googletest/googlemock/src/gmock-all.cc +) + + +target_include_directories(timer_test + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include + PRIVATE ${_gRPC_ADDRESS_SORTING_INCLUDE_DIR} + PRIVATE ${_gRPC_BENCHMARK_INCLUDE_DIR} + PRIVATE ${_gRPC_CARES_INCLUDE_DIR} + PRIVATE ${_gRPC_GFLAGS_INCLUDE_DIR} + PRIVATE ${_gRPC_NANOPB_INCLUDE_DIR} + PRIVATE ${_gRPC_PROTOBUF_INCLUDE_DIR} + PRIVATE ${_gRPC_SSL_INCLUDE_DIR} + PRIVATE ${_gRPC_UPB_GENERATED_DIR} + PRIVATE ${_gRPC_UPB_GRPC_GENERATED_DIR} + PRIVATE ${_gRPC_UPB_INCLUDE_DIR} + PRIVATE ${_gRPC_ZLIB_INCLUDE_DIR} + 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(timer_test + ${_gRPC_PROTOBUF_LIBRARIES} + ${_gRPC_ALLTARGETS_LIBRARIES} + grpc_test_util + grpc++ + grpc + gpr + ${_gRPC_GFLAGS_LIBRARIES} +) + + +endif (gRPC_BUILD_TESTS) +if (gRPC_BUILD_TESTS) + add_executable(transport_pid_controller_test test/core/transport/pid_controller_test.cc third_party/googletest/googletest/src/gtest-all.cc diff --git a/Makefile b/Makefile index a0692f38d20d0bcf9af65619d67edd9e8f1e7966..2f86c96d37972eaf575e311b1e9e9d3955750794 100644 --- a/Makefile +++ b/Makefile @@ -1287,6 +1287,7 @@ string_view_test: $(BINDIR)/$(CONFIG)/string_view_test thread_manager_test: $(BINDIR)/$(CONFIG)/thread_manager_test thread_stress_test: $(BINDIR)/$(CONFIG)/thread_stress_test time_change_test: $(BINDIR)/$(CONFIG)/time_change_test +timer_test: $(BINDIR)/$(CONFIG)/timer_test transport_pid_controller_test: $(BINDIR)/$(CONFIG)/transport_pid_controller_test transport_security_common_api_test: $(BINDIR)/$(CONFIG)/transport_security_common_api_test writes_per_rpc_test: $(BINDIR)/$(CONFIG)/writes_per_rpc_test @@ -1758,6 +1759,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/thread_manager_test \ $(BINDIR)/$(CONFIG)/thread_stress_test \ $(BINDIR)/$(CONFIG)/time_change_test \ + $(BINDIR)/$(CONFIG)/timer_test \ $(BINDIR)/$(CONFIG)/transport_pid_controller_test \ $(BINDIR)/$(CONFIG)/transport_security_common_api_test \ $(BINDIR)/$(CONFIG)/writes_per_rpc_test \ @@ -1926,6 +1928,7 @@ buildtests_cxx: privatelibs_cxx \ $(BINDIR)/$(CONFIG)/thread_manager_test \ $(BINDIR)/$(CONFIG)/thread_stress_test \ $(BINDIR)/$(CONFIG)/time_change_test \ + $(BINDIR)/$(CONFIG)/timer_test \ $(BINDIR)/$(CONFIG)/transport_pid_controller_test \ $(BINDIR)/$(CONFIG)/transport_security_common_api_test \ $(BINDIR)/$(CONFIG)/writes_per_rpc_test \ @@ -2471,6 +2474,8 @@ test_cxx: buildtests_cxx $(Q) $(BINDIR)/$(CONFIG)/thread_stress_test || ( echo test thread_stress_test failed ; exit 1 ) $(E) "[RUN] Testing time_change_test" $(Q) $(BINDIR)/$(CONFIG)/time_change_test || ( echo test time_change_test failed ; exit 1 ) + $(E) "[RUN] Testing timer_test" + $(Q) $(BINDIR)/$(CONFIG)/timer_test || ( echo test timer_test failed ; exit 1 ) $(E) "[RUN] Testing transport_pid_controller_test" $(Q) $(BINDIR)/$(CONFIG)/transport_pid_controller_test || ( echo test transport_pid_controller_test failed ; exit 1 ) $(E) "[RUN] Testing transport_security_common_api_test" @@ -19809,6 +19814,49 @@ endif endif +TIMER_TEST_SRC = \ + test/cpp/common/timer_test.cc \ + +TIMER_TEST_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(TIMER_TEST_SRC)))) +ifeq ($(NO_SECURE),true) + +# You can't build secure targets if you don't have OpenSSL. + +$(BINDIR)/$(CONFIG)/timer_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.5.0+. + +$(BINDIR)/$(CONFIG)/timer_test: protobuf_dep_error + +else + +$(BINDIR)/$(CONFIG)/timer_test: $(PROTOBUF_DEP) $(TIMER_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a + $(E) "[LD] Linking $@" + $(Q) mkdir -p `dirname $@` + $(Q) $(LDXX) $(LDFLAGS) $(TIMER_TEST_OBJS) $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a $(LDLIBSXX) $(LDLIBS_PROTOBUF) $(LDLIBS) $(LDLIBS_SECURE) $(GTEST_LIB) -o $(BINDIR)/$(CONFIG)/timer_test + +endif + +endif + +$(OBJDIR)/$(CONFIG)/test/cpp/common/timer_test.o: $(LIBDIR)/$(CONFIG)/libgrpc_test_util.a $(LIBDIR)/$(CONFIG)/libgrpc++.a $(LIBDIR)/$(CONFIG)/libgrpc.a $(LIBDIR)/$(CONFIG)/libgpr.a + +deps_timer_test: $(TIMER_TEST_OBJS:.o=.dep) + +ifneq ($(NO_SECURE),true) +ifneq ($(NO_DEPS),true) +-include $(TIMER_TEST_OBJS:.o=.dep) +endif +endif + + TRANSPORT_PID_CONTROLLER_TEST_SRC = \ test/core/transport/pid_controller_test.cc \ diff --git a/build.yaml b/build.yaml index 7431dbb2a5357e776d5635bb3d95b73ae0dd65ea..7cebdd3c78d16221deea2f4b0aac7511968c1252 100644 --- a/build.yaml +++ b/build.yaml @@ -5973,6 +5973,17 @@ targets: - mac - linux - posix +- name: timer_test + gtest: true + build: test + language: c++ + src: + - test/cpp/common/timer_test.cc + deps: + - grpc_test_util + - grpc++ + - grpc + - gpr - name: transport_pid_controller_test build: test language: c++ diff --git a/src/core/lib/iomgr/timer_manager.cc b/src/core/lib/iomgr/timer_manager.cc index 17cae5cd4c31dad1247c84774b43032524567f5e..96d502c5108bdd3b0705b97117fbea6aa948d1ba 100644 --- a/src/core/lib/iomgr/timer_manager.cc +++ b/src/core/lib/iomgr/timer_manager.cc @@ -18,6 +18,8 @@ #include <grpc/support/port_platform.h> +#include "src/core/lib/iomgr/timer_manager.h" + #include <inttypes.h> #include <grpc/support/alloc.h> @@ -26,7 +28,6 @@ #include "src/core/lib/debug/trace.h" #include "src/core/lib/gprpp/thd.h" #include "src/core/lib/iomgr/timer.h" -#include "src/core/lib/iomgr/timer_manager.h" struct completed_thread { grpc_core::Thread thd; @@ -58,6 +59,8 @@ static bool g_has_timed_waiter; static grpc_millis g_timed_waiter_deadline; // generation counter to track which thread is waiting for the next timer static uint64_t g_timed_waiter_generation; +// number of timer wakeups +static uint64_t g_wakeups; static void timer_thread(void* completed_thread_ptr); @@ -206,6 +209,7 @@ static bool wait_until(grpc_millis next) { // that there's now no timed waiter... we'll look for a replacement if // there's work to do after checking timers (code above) if (my_timed_waiter_generation == g_timed_waiter_generation) { + ++g_wakeups; g_has_timed_waiter = false; g_timed_waiter_deadline = GRPC_MILLIS_INF_FUTURE; } @@ -326,6 +330,7 @@ static void stop_threads(void) { gc_completed_threads(); } } + g_wakeups = 0; gpr_mu_unlock(&g_mu); } @@ -354,3 +359,5 @@ void grpc_kick_poller(void) { gpr_cv_signal(&g_cv_wait); gpr_mu_unlock(&g_mu); } + +uint64_t grpc_timer_manager_get_wakeups_testonly(void) { return g_wakeups; } diff --git a/src/core/lib/iomgr/timer_manager.h b/src/core/lib/iomgr/timer_manager.h index 00dcdc461b544648202b4fd58c1331a6d85009a3..d407cbbc2b795291c34ef16ec65fda41074e33cc 100644 --- a/src/core/lib/iomgr/timer_manager.h +++ b/src/core/lib/iomgr/timer_manager.h @@ -35,5 +35,7 @@ void grpc_timer_manager_set_threading(bool enabled); /* explicitly perform one tick of the timer system - for when threading is * disabled */ void grpc_timer_manager_tick(void); +/* get global counter that tracks timer wakeups */ +uint64_t grpc_timer_manager_get_wakeups_testonly(void); #endif /* GRPC_CORE_LIB_IOMGR_TIMER_MANAGER_H */ diff --git a/test/cpp/common/BUILD b/test/cpp/common/BUILD index b67c1995ff72f518137eded0d737d41021833e86..f0d0e9a6223be02cf79af37a4e7d83eb2fc7b004 100644 --- a/test/cpp/common/BUILD +++ b/test/cpp/common/BUILD @@ -28,7 +28,18 @@ grpc_cc_test( "//:grpc++_unsecure", "//test/core/util:grpc_test_util_unsecure", ], - tags = ["no_windows"], +) + +grpc_cc_test( + name = "timer_test", + srcs = ["timer_test.cc"], + external_deps = [ + "gtest", + ], + deps = [ + "//:grpc++", + "//test/core/util:grpc_test_util", + ], ) grpc_cc_test( diff --git a/test/cpp/common/timer_test.cc b/test/cpp/common/timer_test.cc new file mode 100644 index 0000000000000000000000000000000000000000..f5083d66e76da76271a4be4a34c74c67831809b6 --- /dev/null +++ b/test/cpp/common/timer_test.cc @@ -0,0 +1,150 @@ +/* + * + * Copyright 2019 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 <grpc/grpc.h> +#include <grpc/support/log.h> +#include <gtest/gtest.h> + +#include "src/core/lib/iomgr/closure.h" +#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/iomgr/exec_ctx.h" +#include "src/core/lib/iomgr/timer.h" +#include "src/core/lib/iomgr/timer_manager.h" +#include "test/core/util/test_config.h" + +// MAYBE_SKIP_TEST is a macro to determine if this particular test configuration +// should be skipped based on a decision made at SetUp time. +#define MAYBE_SKIP_TEST \ + do { \ + if (do_not_test_) { \ + return; \ + } \ + } while (0) + +class TimerTest : public ::testing::Test { + protected: + void SetUp() override { + // Skip test if slowdown factor > 1. + do_not_test_ = (grpc_test_slowdown_factor() != 1); + grpc_init(); + } + + void TearDown() override { grpc_shutdown_blocking(); } + + bool do_not_test_{false}; +}; + +TEST_F(TimerTest, NoTimers) { + MAYBE_SKIP_TEST; + grpc_core::ExecCtx exec_ctx; + gpr_sleep_until(grpc_timeout_milliseconds_to_deadline(1500)); + + // We expect to get 1 wakeup per second. Sometimes we also get a wakeup + // during initialization, so in 1.5 seconds we expect to get 1 or 2 wakeups. + int64_t wakeups = grpc_timer_manager_get_wakeups_testonly(); + GPR_ASSERT(wakeups == 1 || wakeups == 2); +} + +TEST_F(TimerTest, OneTimerExpires) { + MAYBE_SKIP_TEST; + grpc_core::ExecCtx exec_ctx; + grpc_timer timer; + int timer_fired = 0; + grpc_timer_init(&timer, 500, + GRPC_CLOSURE_CREATE( + [](void* arg, grpc_error*) { + int* timer_fired = static_cast<int*>(arg); + ++*timer_fired; + }, + &timer_fired, grpc_schedule_on_exec_ctx)); + gpr_sleep_until(grpc_timeout_milliseconds_to_deadline(1500)); + GPR_ASSERT(1 == timer_fired); + + // We expect to get 1 wakeup/second + 1 wakeup for the expired timer + maybe 1 + // wakeup during initialization. i.e. in 1.5 seconds we expect 2 or 3 wakeups. + // Actual number of wakeups is more due to bug + // https://github.com/grpc/grpc/issues/19947 + int64_t wakeups = grpc_timer_manager_get_wakeups_testonly(); + gpr_log(GPR_DEBUG, "wakeups: %" PRId64 "", wakeups); +} + +TEST_F(TimerTest, MultipleTimersExpire) { + MAYBE_SKIP_TEST; + grpc_core::ExecCtx exec_ctx; + const int kNumTimers = 10; + grpc_timer timers[kNumTimers]; + int timer_fired = 0; + for (int i = 0; i < kNumTimers; ++i) { + grpc_timer_init(&timers[i], 500 + i, + GRPC_CLOSURE_CREATE( + [](void* arg, grpc_error*) { + int* timer_fired = static_cast<int*>(arg); + ++*timer_fired; + }, + &timer_fired, grpc_schedule_on_exec_ctx)); + } + + gpr_sleep_until(grpc_timeout_milliseconds_to_deadline(1500)); + GPR_ASSERT(kNumTimers == timer_fired); + + // We expect to get 1 wakeup/second + 1 wakeup for per timer fired + maybe 1 + // wakeup during initialization. i.e. in 1.5 seconds we expect 11 or 12 + // wakeups. Actual number of wakeups is more due to bug + // https://github.com/grpc/grpc/issues/19947 + int64_t wakeups = grpc_timer_manager_get_wakeups_testonly(); + gpr_log(GPR_DEBUG, "wakeups: %" PRId64 "", wakeups); +} + +TEST_F(TimerTest, CancelSomeTimers) { + MAYBE_SKIP_TEST; + grpc_core::ExecCtx exec_ctx; + const int kNumTimers = 10; + grpc_timer timers[kNumTimers]; + int timer_fired = 0; + for (int i = 0; i < kNumTimers; ++i) { + grpc_timer_init(&timers[i], 500 + i, + GRPC_CLOSURE_CREATE( + [](void* arg, grpc_error* error) { + if (error == GRPC_ERROR_CANCELLED) { + return; + } + int* timer_fired = static_cast<int*>(arg); + ++*timer_fired; + }, + &timer_fired, grpc_schedule_on_exec_ctx)); + } + for (int i = 0; i < kNumTimers / 2; ++i) { + grpc_timer_cancel(&timers[i]); + } + + gpr_sleep_until(grpc_timeout_milliseconds_to_deadline(1500)); + GPR_ASSERT(kNumTimers / 2 == timer_fired); + + // We expect to get 1 wakeup/second + 1 wakeup per timer fired + maybe 1 + // wakeup during initialization. i.e. in 1.5 seconds we expect 6 or 7 wakeups. + // Actual number of wakeups is more due to bug + // https://github.com/grpc/grpc/issues/19947 + int64_t wakeups = grpc_timer_manager_get_wakeups_testonly(); + gpr_log(GPR_DEBUG, "wakeups: %" PRId64 "", wakeups); +} + +int main(int argc, char** argv) { + grpc::testing::TestEnvironment env(argc, argv); + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 6dd3f1422e5974c2c5753eeb6edb607860923842..a6efca9014be91fcd864d476781cb59058333b6d 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -5988,6 +5988,30 @@ ], "uses_polling": true }, + { + "args": [], + "benchmark": false, + "ci_platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "gtest": true, + "language": "c++", + "name": "timer_test", + "platforms": [ + "linux", + "mac", + "posix", + "windows" + ], + "uses_polling": true + }, { "args": [], "benchmark": false,