From 2e37d001a3bd56cde5f11083cd7f679a50b49022 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 19 Jul 2017 13:34:31 -0700 Subject: [PATCH] Review feedback --- BUILD | 2 +- build.yaml | 2 +- gRPC-Core.podspec | 4 ++-- grpc.gemspec | 2 +- package.xml | 2 +- src/core/ext/filters/max_age/max_age_filter.c | 15 ++++++++------- src/core/lib/{support => iomgr}/block_annotate.h | 0 src/core/lib/iomgr/ev_epoll1_linux.c | 2 +- .../lib/iomgr/ev_epoll_limited_pollers_linux.c | 2 +- src/core/lib/iomgr/ev_epoll_thread_pool_linux.c | 2 +- src/core/lib/iomgr/ev_epollex_linux.c | 2 +- src/core/lib/iomgr/ev_epollsig_linux.c | 2 +- src/core/lib/iomgr/ev_poll_posix.c | 2 +- src/core/lib/iomgr/load_file.c | 2 +- src/core/lib/iomgr/resolve_address_posix.c | 2 +- src/core/lib/iomgr/resolve_address_windows.c | 2 +- src/core/lib/support/time_posix.c | 2 +- src/core/lib/support/time_windows.c | 2 +- src/core/lib/surface/completion_queue.c | 1 + tools/doxygen/Doxyfile.core.internal | 2 +- .../run_tests/generated/sources_and_headers.json | 4 ++-- 21 files changed, 29 insertions(+), 27 deletions(-) rename src/core/lib/{support => iomgr}/block_annotate.h (100%) diff --git a/BUILD b/BUILD index c69dd5e4613..8294b0be2e3 100644 --- a/BUILD +++ b/BUILD @@ -513,7 +513,7 @@ grpc_cc_library( "src/core/lib/support/atomic_with_atm.h", "src/core/lib/support/atomic_with_std.h", "src/core/lib/backoff/backoff.h", - "src/core/lib/support/block_annotate.h", + "src/core/lib/iomgr/block_annotate.h", "src/core/lib/support/env.h", "src/core/lib/support/memory.h", "src/core/lib/support/mpscq.h", diff --git a/build.yaml b/build.yaml index 01c6b42baa7..ac631ddadcf 100644 --- a/build.yaml +++ b/build.yaml @@ -93,7 +93,7 @@ filegroups: - 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/block_annotate.h + - src/core/lib/iomgr/block_annotate.h - src/core/lib/support/env.h - src/core/lib/support/memory.h - src/core/lib/support/mpscq.h diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 5e1f663bb6e..01995401a31 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -186,7 +186,7 @@ Pod::Spec.new do |s| '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/block_annotate.h', + 'src/core/lib/iomgr/block_annotate.h', 'src/core/lib/support/env.h', 'src/core/lib/support/memory.h', 'src/core/lib/support/mpscq.h', @@ -714,7 +714,7 @@ Pod::Spec.new do |s| '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/block_annotate.h', + 'src/core/lib/iomgr/block_annotate.h', 'src/core/lib/support/env.h', 'src/core/lib/support/memory.h', 'src/core/lib/support/mpscq.h', diff --git a/grpc.gemspec b/grpc.gemspec index 45fed61c272..10b4e92a3be 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -86,7 +86,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/support/atomic.h ) 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/block_annotate.h ) + s.files += %w( src/core/lib/iomgr/block_annotate.h ) s.files += %w( src/core/lib/support/env.h ) s.files += %w( src/core/lib/support/memory.h ) s.files += %w( src/core/lib/support/mpscq.h ) diff --git a/package.xml b/package.xml index 0d5f49c889b..9a030850b0a 100644 --- a/package.xml +++ b/package.xml @@ -100,7 +100,7 @@ - + diff --git a/src/core/ext/filters/max_age/max_age_filter.c b/src/core/ext/filters/max_age/max_age_filter.c index 14565fd5308..2a785efc045 100644 --- a/src/core/ext/filters/max_age/max_age_filter.c +++ b/src/core/ext/filters/max_age/max_age_filter.c @@ -245,7 +245,8 @@ static void channel_connectivity_changed(grpc_exec_ctx* exec_ctx, void* arg, connection storms. Note that the MAX_CONNECTION_AGE option without jitter would not create connection storms by itself, but if there happened to be a connection storm it could cause it to repeat at a fixed period. */ -static int add_random_max_connection_age_jitter(int value) { +static grpc_millis +add_random_max_connection_age_jitter_and_convert_to_grpc_millis(int value) { /* generate a random number between 1 - MAX_CONNECTION_AGE_JITTER and 1 + MAX_CONNECTION_AGE_JITTER */ double multiplier = rand() * MAX_CONNECTION_AGE_JITTER * 2.0 / RAND_MAX + @@ -253,7 +254,8 @@ static int add_random_max_connection_age_jitter(int value) { double result = multiplier * value; /* INT_MAX - 0.5 converts the value to float, so that result will not be cast to int implicitly before the comparison. */ - return result > INT_MAX - 0.5 ? INT_MAX : (int)result; + return result > GRPC_MILLIS_INF_FUTURE - 0.5 ? GRPC_MILLIS_INF_FUTURE + : (int)result; } /* Constructor for call_data. */ @@ -283,9 +285,8 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, chand->max_age_grace_timer_pending = false; chand->channel_stack = args->channel_stack; chand->max_connection_age = - DEFAULT_MAX_CONNECTION_AGE_MS == INT_MAX - ? GRPC_MILLIS_INF_FUTURE - : add_random_max_connection_age_jitter(DEFAULT_MAX_CONNECTION_AGE_MS); + add_random_max_connection_age_jitter_and_convert_to_grpc_millis( + DEFAULT_MAX_CONNECTION_AGE_MS); chand->max_connection_age_grace = DEFAULT_MAX_CONNECTION_AGE_GRACE_MS == INT_MAX ? GRPC_MILLIS_INF_FUTURE @@ -299,8 +300,8 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], MAX_CONNECTION_AGE_INTEGER_OPTIONS); chand->max_connection_age = - value == INT_MAX ? GRPC_MILLIS_INF_FUTURE - : add_random_max_connection_age_jitter(value); + add_random_max_connection_age_jitter_and_convert_to_grpc_millis( + value); } else if (0 == strcmp(args->channel_args->args[i].key, GRPC_ARG_MAX_CONNECTION_AGE_GRACE_MS)) { const int value = grpc_channel_arg_get_integer( diff --git a/src/core/lib/support/block_annotate.h b/src/core/lib/iomgr/block_annotate.h similarity index 100% rename from src/core/lib/support/block_annotate.h rename to src/core/lib/iomgr/block_annotate.h diff --git a/src/core/lib/iomgr/ev_epoll1_linux.c b/src/core/lib/iomgr/ev_epoll1_linux.c index 4eaaaa06dda..a3586547d46 100644 --- a/src/core/lib/iomgr/ev_epoll1_linux.c +++ b/src/core/lib/iomgr/ev_epoll1_linux.c @@ -45,7 +45,7 @@ #include "src/core/lib/iomgr/lockfree_event.h" #include "src/core/lib/iomgr/wakeup_fd_posix.h" #include "src/core/lib/profiling/timers.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" static grpc_wakeup_fd global_wakeup_fd; static int g_epfd; diff --git a/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c b/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c index 055acffbdd4..05e9ca448a7 100644 --- a/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c +++ b/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c @@ -47,7 +47,7 @@ #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/iomgr/wakeup_fd_posix.h" #include "src/core/lib/profiling/timers.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #include "src/core/lib/support/env.h" #define GRPC_POLLING_TRACE(fmt, ...) \ diff --git a/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c b/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c index 46bf828cabe..bbcc475d7ed 100644 --- a/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c +++ b/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c @@ -47,7 +47,7 @@ #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/iomgr/wakeup_fd_posix.h" #include "src/core/lib/profiling/timers.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" /* TODO: sreek - Move this to init.c and initialize this like other tracers. */ #define GRPC_POLLING_TRACE(fmt, ...) \ diff --git a/src/core/lib/iomgr/ev_epollex_linux.c b/src/core/lib/iomgr/ev_epollex_linux.c index 030f18aaf25..7db95fe9b43 100644 --- a/src/core/lib/iomgr/ev_epollex_linux.c +++ b/src/core/lib/iomgr/ev_epollex_linux.c @@ -46,7 +46,7 @@ #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/iomgr/wakeup_fd_posix.h" #include "src/core/lib/profiling/timers.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #include "src/core/lib/support/spinlock.h" /******************************************************************************* diff --git a/src/core/lib/iomgr/ev_epollsig_linux.c b/src/core/lib/iomgr/ev_epollsig_linux.c index 1e7ec0a1bc0..fc5212ab5e4 100644 --- a/src/core/lib/iomgr/ev_epollsig_linux.c +++ b/src/core/lib/iomgr/ev_epollsig_linux.c @@ -46,7 +46,7 @@ #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/iomgr/wakeup_fd_posix.h" #include "src/core/lib/profiling/timers.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #define GRPC_POLLSET_KICK_BROADCAST ((grpc_pollset_worker *)1) diff --git a/src/core/lib/iomgr/ev_poll_posix.c b/src/core/lib/iomgr/ev_poll_posix.c index e7cdfc18603..c6456bee188 100644 --- a/src/core/lib/iomgr/ev_poll_posix.c +++ b/src/core/lib/iomgr/ev_poll_posix.c @@ -42,7 +42,7 @@ #include "src/core/lib/iomgr/wakeup_fd_cv.h" #include "src/core/lib/iomgr/wakeup_fd_posix.h" #include "src/core/lib/profiling/timers.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #define GRPC_POLLSET_KICK_BROADCAST ((grpc_pollset_worker *)1) diff --git a/src/core/lib/iomgr/load_file.c b/src/core/lib/iomgr/load_file.c index a8c0fa13390..aafcd79e38d 100644 --- a/src/core/lib/iomgr/load_file.c +++ b/src/core/lib/iomgr/load_file.c @@ -25,7 +25,7 @@ #include #include -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #include "src/core/lib/support/string.h" grpc_error *grpc_load_file(const char *filename, int add_null_terminator, diff --git a/src/core/lib/iomgr/resolve_address_posix.c b/src/core/lib/iomgr/resolve_address_posix.c index 160235ca99d..d58ec4168ef 100644 --- a/src/core/lib/iomgr/resolve_address_posix.c +++ b/src/core/lib/iomgr/resolve_address_posix.c @@ -36,7 +36,7 @@ #include "src/core/lib/iomgr/executor.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/unix_sockets_posix.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #include "src/core/lib/support/string.h" static grpc_error *blocking_resolve_address_impl( diff --git a/src/core/lib/iomgr/resolve_address_windows.c b/src/core/lib/iomgr/resolve_address_windows.c index c5d8dd9893e..ee09d127b45 100644 --- a/src/core/lib/iomgr/resolve_address_windows.c +++ b/src/core/lib/iomgr/resolve_address_windows.c @@ -36,7 +36,7 @@ #include "src/core/lib/iomgr/executor.h" #include "src/core/lib/iomgr/iomgr_internal.h" #include "src/core/lib/iomgr/sockaddr_utils.h" -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #include "src/core/lib/support/string.h" typedef struct { diff --git a/src/core/lib/support/time_posix.c b/src/core/lib/support/time_posix.c index 1248d875edd..ad7c5036dae 100644 --- a/src/core/lib/support/time_posix.c +++ b/src/core/lib/support/time_posix.c @@ -30,7 +30,7 @@ #include #include #include -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" static struct timespec timespec_from_gpr(gpr_timespec gts) { struct timespec rv; diff --git a/src/core/lib/support/time_windows.c b/src/core/lib/support/time_windows.c index e28cf0f4cbc..b8015c272f8 100644 --- a/src/core/lib/support/time_windows.c +++ b/src/core/lib/support/time_windows.c @@ -28,7 +28,7 @@ #include #include -#include "src/core/lib/support/block_annotate.h" +#include "src/core/lib/iomgr/block_annotate.h" #include "src/core/lib/support/time_precise.h" static LARGE_INTEGER g_start_time; diff --git a/src/core/lib/surface/completion_queue.c b/src/core/lib/surface/completion_queue.c index e0547415201..f3a89772411 100644 --- a/src/core/lib/surface/completion_queue.c +++ b/src/core/lib/surface/completion_queue.c @@ -1114,6 +1114,7 @@ static grpc_event cq_pluck(grpc_completion_queue *cq, void *tag, dump_pending_tags(cq); break; } + cq->num_polls++; grpc_error *err = cq->poller_vtable->work(&exec_ctx, POLLSET_FROM_CQ(cq), &worker, deadline_millis); if (err != GRPC_ERROR_NONE) { diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index cb625e2a8ec..c0ff8c9806b 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1286,7 +1286,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/avl.c \ -src/core/lib/support/block_annotate.h \ +src/core/lib/iomgr/block_annotate.h \ src/core/lib/support/cmdline.c \ src/core/lib/support/cpu_iphone.c \ src/core/lib/support/cpu_linux.c \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 589cafa29b4..51252724ace 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -7518,7 +7518,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/block_annotate.h", + "src/core/lib/iomgr/block_annotate.h", "src/core/lib/support/env.h", "src/core/lib/support/memory.h", "src/core/lib/support/mpscq.h", @@ -7574,7 +7574,7 @@ "src/core/lib/support/atomic_with_atm.h", "src/core/lib/support/atomic_with_std.h", "src/core/lib/support/avl.c", - "src/core/lib/support/block_annotate.h", + "src/core/lib/iomgr/block_annotate.h", "src/core/lib/support/cmdline.c", "src/core/lib/support/cpu_iphone.c", "src/core/lib/support/cpu_linux.c",