From 43c1b5f4377d3f1640b7bdae099cd3300a624bc0 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 2 Oct 2017 14:42:49 -0700 Subject: [PATCH] Fixes --- build.yaml | 2 +- gRPC-Core.podspec | 4 ++-- grpc.gemspec | 2 +- package.xml | 2 +- .../transport/chttp2/transport/chttp2_transport.c | 2 +- .../ext/transport/chttp2/transport/flow_control.c | 3 ++- src/core/lib/iomgr/iocp_windows.c | 6 +++--- src/core/lib/transport/bdp_estimator.c | 14 ++++++++------ src/core/lib/transport/bdp_estimator.h | 13 ++++++++----- tools/doxygen/Doxyfile.c++.internal | 2 +- tools/doxygen/Doxyfile.core.internal | 2 +- tools/run_tests/generated/sources_and_headers.json | 4 ++-- 12 files changed, 31 insertions(+), 25 deletions(-) diff --git a/build.yaml b/build.yaml index 9fac2e11260..5843be28188 100644 --- a/build.yaml +++ b/build.yaml @@ -142,7 +142,6 @@ 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/support/env.h - src/core/lib/support/memory.h - src/core/lib/support/mpscq.h @@ -355,6 +354,7 @@ filegroups: - src/core/lib/http/format_request.h - src/core/lib/http/httpcli.h - src/core/lib/http/parser.h + - src/core/lib/iomgr/block_annotate.h - src/core/lib/iomgr/call_combiner.h - src/core/lib/iomgr/closure.h - src/core/lib/iomgr/combiner.h diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 8b7bdfc6177..804345a9cb1 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -189,7 +189,6 @@ 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/support/env.h', 'src/core/lib/support/memory.h', 'src/core/lib/support/mpscq.h', @@ -336,6 +335,7 @@ Pod::Spec.new do |s| 'src/core/lib/http/format_request.h', 'src/core/lib/http/httpcli.h', 'src/core/lib/http/parser.h', + 'src/core/lib/iomgr/block_annotate.h', 'src/core/lib/iomgr/call_combiner.h', 'src/core/lib/iomgr/closure.h', 'src/core/lib/iomgr/combiner.h', @@ -734,7 +734,6 @@ 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/support/env.h', 'src/core/lib/support/memory.h', 'src/core/lib/support/mpscq.h', @@ -836,6 +835,7 @@ Pod::Spec.new do |s| 'src/core/lib/http/format_request.h', 'src/core/lib/http/httpcli.h', 'src/core/lib/http/parser.h', + 'src/core/lib/iomgr/block_annotate.h', 'src/core/lib/iomgr/call_combiner.h', 'src/core/lib/iomgr/closure.h', 'src/core/lib/iomgr/combiner.h', diff --git a/grpc.gemspec b/grpc.gemspec index 26b123d47b0..f6927b2cb50 100644 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -88,7 +88,6 @@ 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/support/env.h ) s.files += %w( src/core/lib/support/memory.h ) s.files += %w( src/core/lib/support/mpscq.h ) @@ -269,6 +268,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/http/format_request.h ) s.files += %w( src/core/lib/http/httpcli.h ) s.files += %w( src/core/lib/http/parser.h ) + s.files += %w( src/core/lib/iomgr/block_annotate.h ) s.files += %w( src/core/lib/iomgr/call_combiner.h ) s.files += %w( src/core/lib/iomgr/closure.h ) s.files += %w( src/core/lib/iomgr/combiner.h ) diff --git a/package.xml b/package.xml index 4c24070124e..824de0b8832 100644 --- a/package.xml +++ b/package.xml @@ -100,7 +100,6 @@ - @@ -281,6 +280,7 @@ + diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index 1c575badb87..452651af900 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -2576,7 +2576,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(&t->flow_control.bdp_estimator); + grpc_bdp_estimator_complete_ping(exec_ctx, &t->flow_control.bdp_estimator); GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "bdp_ping"); } diff --git a/src/core/ext/transport/chttp2/transport/flow_control.c b/src/core/ext/transport/chttp2/transport/flow_control.c index a95b4b3d052..a8ac80cf43b 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.c +++ b/src/core/ext/transport/chttp2/transport/flow_control.c @@ -457,7 +457,8 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_action( } } if (tfc->enable_bdp_probe) { - action.need_ping = grpc_bdp_estimator_need_ping(&tfc->bdp_estimator); + action.need_ping = + grpc_bdp_estimator_need_ping(exec_ctx, &tfc->bdp_estimator); // get bdp estimate and update initial_window accordingly. int64_t estimate = -1; diff --git a/src/core/lib/iomgr/iocp_windows.c b/src/core/lib/iomgr/iocp_windows.c index 3c38105ef99..a6e064c8c04 100644 --- a/src/core/lib/iomgr/iocp_windows.c +++ b/src/core/lib/iomgr/iocp_windows.c @@ -43,7 +43,7 @@ static HANDLE g_iocp; static DWORD deadline_to_millis_timeout(grpc_exec_ctx *exec_ctx, grpc_millis deadline) { gpr_timespec timeout; - if (deadline == GRPC_MILLIS_INF_FUTURE) == 0) { + if (deadline == GRPC_MILLIS_INF_FUTURE) { return INFINITE; } return (DWORD)GPR_MAX(0, deadline - grpc_exec_ctx_now(exec_ctx)); @@ -113,7 +113,7 @@ void grpc_iocp_flush(void) { grpc_iocp_work_status work_status; do { - work_status = grpc_iocp_work(&exec_ctx, gpr_inf_past(GPR_CLOCK_MONOTONIC)); + work_status = grpc_iocp_work(&exec_ctx, GRPC_MILLIS_INF_PAST); } while (work_status == GRPC_IOCP_WORK_KICK || grpc_exec_ctx_flush(&exec_ctx)); } @@ -121,7 +121,7 @@ void grpc_iocp_flush(void) { void grpc_iocp_shutdown(void) { grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; while (gpr_atm_acq_load(&g_custom_events)) { - grpc_iocp_work(&exec_ctx, gpr_inf_future(GPR_CLOCK_MONOTONIC)); + grpc_iocp_work(&exec_ctx, GRPC_MILLIS_INF_FUTURE); grpc_exec_ctx_flush(&exec_ctx); } grpc_exec_ctx_finish(&exec_ctx); diff --git a/src/core/lib/transport/bdp_estimator.c b/src/core/lib/transport/bdp_estimator.c index 2a3ffbbd018..2bf1d0b5e5c 100644 --- a/src/core/lib/transport/bdp_estimator.c +++ b/src/core/lib/transport/bdp_estimator.c @@ -30,6 +30,7 @@ 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 @@ -53,11 +54,11 @@ void grpc_bdp_estimator_add_incoming_bytes(grpc_bdp_estimator *estimator, estimator->accumulator += num_bytes; } -bool grpc_bdp_estimator_need_ping(const grpc_bdp_estimator *estimator) { +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 gpr_time_cmp(gpr_now(GPR_CLOCK_MONOTONIC), - estimator->ping_start_time) >= 0; + return grpc_exec_ctx_now(exec_ctx) >= estimator->next_ping_scheduled; case GRPC_BDP_PING_SCHEDULED: return false; case GRPC_BDP_PING_STARTED: @@ -87,7 +88,8 @@ void grpc_bdp_estimator_start_ping(grpc_bdp_estimator *estimator) { estimator->ping_start_time = gpr_now(GPR_CLOCK_MONOTONIC); } -void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator) { +void grpc_bdp_estimator_complete_ping(grpc_exec_ctx *exec_ctx, + grpc_bdp_estimator *estimator) { gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); gpr_timespec dt_ts = gpr_time_sub(now, estimator->ping_start_time); double dt = (double)dt_ts.tv_sec + 1e-9 * (double)dt_ts.tv_nsec; @@ -129,6 +131,6 @@ void grpc_bdp_estimator_complete_ping(grpc_bdp_estimator *estimator) { } estimator->ping_state = GRPC_BDP_PING_UNSCHEDULED; estimator->accumulator = 0; - estimator->ping_start_time = gpr_time_add( - now, gpr_time_from_millis(estimator->inter_ping_delay, GPR_TIMESPAN)); + estimator->next_ping_scheduled = + grpc_exec_ctx_now(exec_ctx) + estimator->inter_ping_delay; } diff --git a/src/core/lib/transport/bdp_estimator.h b/src/core/lib/transport/bdp_estimator.h index 670e38dd4bb..c74de5a6bb8 100644 --- a/src/core/lib/transport/bdp_estimator.h +++ b/src/core/lib/transport/bdp_estimator.h @@ -23,6 +23,7 @@ #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 @@ -39,10 +40,10 @@ typedef struct grpc_bdp_estimator { grpc_bdp_estimator_ping_state ping_state; int64_t accumulator; int64_t estimate; - // case ping_state of - // GRPC_BDP_PING_UNSCHEDULED => when to start the next ping - // GRPC_BDP_PING_STARTED => when the current ping was started + // when was the current ping started? 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; @@ -60,7 +61,8 @@ bool grpc_bdp_estimator_get_bw(const grpc_bdp_estimator *estimator, double *bw); 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(const grpc_bdp_estimator *estimator); +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) @@ -69,6 +71,7 @@ void grpc_bdp_estimator_schedule_ping(grpc_bdp_estimator *estimator); // 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); +void grpc_bdp_estimator_complete_ping(grpc_exec_ctx *exec_ctx, + grpc_bdp_estimator *estimator); #endif /* GRPC_CORE_LIB_TRANSPORT_BDP_ESTIMATOR_H */ diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index d220365fc07..cb8bb93c738 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -953,6 +953,7 @@ src/core/lib/debug/trace.h \ src/core/lib/http/format_request.h \ src/core/lib/http/httpcli.h \ src/core/lib/http/parser.h \ +src/core/lib/iomgr/block_annotate.h \ src/core/lib/iomgr/call_combiner.h \ src/core/lib/iomgr/closure.h \ src/core/lib/iomgr/combiner.h \ @@ -1029,7 +1030,6 @@ src/core/lib/support/arena.h \ 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/support/env.h \ src/core/lib/support/memory.h \ src/core/lib/support/mpscq.h \ diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 430566bfbef..792d0b3f684 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1098,6 +1098,7 @@ src/core/lib/http/httpcli_security_connector.c \ src/core/lib/http/parser.c \ src/core/lib/http/parser.h \ src/core/lib/iomgr/README.md \ +src/core/lib/iomgr/block_annotate.h \ src/core/lib/iomgr/call_combiner.c \ src/core/lib/iomgr/call_combiner.h \ src/core/lib/iomgr/closure.c \ @@ -1303,7 +1304,6 @@ 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/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 5e825e2082a..07acce58a2f 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -7855,7 +7855,6 @@ "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/support/env.h", "src/core/lib/support/memory.h", "src/core/lib/support/mpscq.h", @@ -7903,7 +7902,6 @@ "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/support/env.h", "src/core/lib/support/memory.h", "src/core/lib/support/mpscq.h", @@ -8169,6 +8167,7 @@ "src/core/lib/http/format_request.h", "src/core/lib/http/httpcli.h", "src/core/lib/http/parser.h", + "src/core/lib/iomgr/block_annotate.h", "src/core/lib/iomgr/call_combiner.h", "src/core/lib/iomgr/closure.h", "src/core/lib/iomgr/combiner.h", @@ -8303,6 +8302,7 @@ "src/core/lib/http/format_request.h", "src/core/lib/http/httpcli.h", "src/core/lib/http/parser.h", + "src/core/lib/iomgr/block_annotate.h", "src/core/lib/iomgr/call_combiner.h", "src/core/lib/iomgr/closure.h", "src/core/lib/iomgr/combiner.h",