diff options
Diffstat (limited to 'src')
20 files changed, 160 insertions, 98 deletions
diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index fa0c280f80..bc6f733e15 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -1421,7 +1421,7 @@ static void do_retry(grpc_call_element* elem, } if (grpc_client_channel_trace.enabled()) { gpr_log(GPR_INFO, - "chand=%p calld=%p: retrying failed call in %" PRIuPTR " ms", chand, + "chand=%p calld=%p: retrying failed call in %" PRId64 " ms", chand, calld, next_attempt_time - grpc_core::ExecCtx::Get()->Now()); } // Schedule retry after computed delay. diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 0d57fa2d02..263b51ae89 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -776,7 +776,7 @@ void GrpcLb::BalancerCallState::OnBalancerMessageReceivedLocked( if (grpc_lb_glb_trace.enabled()) { gpr_log(GPR_INFO, "[grpclb %p] Received initial LB response message; " - "client load reporting interval = %" PRIdPTR " milliseconds", + "client load reporting interval = %" PRId64 " milliseconds", grpclb_policy, lb_calld->client_stats_report_interval_); } } else if (grpc_lb_glb_trace.enabled()) { @@ -1417,7 +1417,7 @@ void GrpcLb::StartBalancerCallRetryTimerLocked() { gpr_log(GPR_INFO, "[grpclb %p] Connection to LB server lost...", this); grpc_millis timeout = next_try - ExecCtx::Get()->Now(); if (timeout > 0) { - gpr_log(GPR_INFO, "[grpclb %p] ... retry_timer_active in %" PRIuPTR "ms.", + gpr_log(GPR_INFO, "[grpclb %p] ... retry_timer_active in %" PRId64 "ms.", this, timeout); } else { gpr_log(GPR_INFO, "[grpclb %p] ... retry_timer_active immediately.", diff --git a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc index 76df976698..ff2140e628 100644 --- a/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc +++ b/src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc @@ -490,7 +490,7 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked( if (grpc_lb_pick_first_trace.enabled()) { gpr_log(GPR_INFO, "Servicing pending pick with selected subchannel %p", - p->selected_); + p->selected_->subchannel()); } GRPC_CLOSURE_SCHED(pick->on_complete, GRPC_ERROR_NONE); } diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc index c3c62b60bf..3c40ae14b8 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/c_ares/dns_resolver_ares.cc @@ -346,7 +346,7 @@ void AresDnsResolver::OnResolvedLocked(void* arg, grpc_error* error) { RefCountedPtr<Resolver> self = r->Ref(DEBUG_LOCATION, "retry-timer"); self.release(); if (timeout > 0) { - gpr_log(GPR_DEBUG, "retrying in %" PRIdPTR " milliseconds", timeout); + gpr_log(GPR_DEBUG, "retrying in %" PRId64 " milliseconds", timeout); } else { gpr_log(GPR_DEBUG, "retrying immediately"); } @@ -381,8 +381,8 @@ void AresDnsResolver::MaybeStartResolvingLocked() { const grpc_millis last_resolution_ago = grpc_core::ExecCtx::Get()->Now() - last_resolution_timestamp_; gpr_log(GPR_DEBUG, - "In cooldown from last resolution (from %" PRIdPTR - " ms ago). Will resolve again in %" PRIdPTR " ms", + "In cooldown from last resolution (from %" PRId64 + " ms ago). Will resolve again in %" PRId64 " ms", last_resolution_ago, ms_until_next_resolution); have_next_resolution_timer_ = true; // TODO(roth): We currently deal with this ref manually. Once the diff --git a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc index 920a492fdb..fae4c33a17 100644 --- a/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc +++ b/src/core/ext/filters/client_channel/resolver/dns/native/dns_resolver.cc @@ -218,7 +218,7 @@ void NativeDnsResolver::OnResolvedLocked(void* arg, grpc_error* error) { r->Ref(DEBUG_LOCATION, "next_resolution_timer"); self.release(); if (timeout > 0) { - gpr_log(GPR_DEBUG, "retrying in %" PRIdPTR " milliseconds", timeout); + gpr_log(GPR_DEBUG, "retrying in %" PRId64 " milliseconds", timeout); } else { gpr_log(GPR_DEBUG, "retrying immediately"); } @@ -254,8 +254,8 @@ void NativeDnsResolver::MaybeStartResolvingLocked() { const grpc_millis last_resolution_ago = grpc_core::ExecCtx::Get()->Now() - last_resolution_timestamp_; gpr_log(GPR_DEBUG, - "In cooldown from last resolution (from %" PRIdPTR - " ms ago). Will resolve again in %" PRIdPTR " ms", + "In cooldown from last resolution (from %" PRId64 + " ms ago). Will resolve again in %" PRId64 " ms", last_resolution_ago, ms_until_next_resolution); have_next_resolution_timer_ = true; // TODO(roth): We currently deal with this ref manually. Once the diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc index ad6b6dd192..f010002ab9 100644 --- a/src/core/ext/filters/client_channel/subchannel.cc +++ b/src/core/ext/filters/client_channel/subchannel.cc @@ -467,7 +467,7 @@ static void maybe_start_connecting_locked(grpc_subchannel* c) { if (time_til_next <= 0) { gpr_log(GPR_INFO, "Subchannel %p: Retry immediately", c); } else { - gpr_log(GPR_INFO, "Subchannel %p: Retry in %" PRIdPTR " milliseconds", c, + gpr_log(GPR_INFO, "Subchannel %p: Retry in %" PRId64 " milliseconds", c, time_til_next); } GRPC_CLOSURE_INIT(&c->on_alarm, on_alarm, c, grpc_schedule_on_exec_ctx); diff --git a/src/core/lib/debug/trace.h b/src/core/lib/debug/trace.h index 28157c6383..fe6301a3fc 100644 --- a/src/core/lib/debug/trace.h +++ b/src/core/lib/debug/trace.h @@ -57,14 +57,14 @@ class TraceFlag { const char* name() const { return name_; } -// This following define may be commented out to ensure that the compiler -// deletes any "if (tracer.enabled()) {...}" codeblocks. This is useful to -// test the performance impact tracers have on the system. -// -// #define COMPILE_OUT_ALL_TRACERS_IN_OPT_BUILD -#ifdef COMPILE_OUT_ALL_TRACERS_IN_OPT_BUILD - bool enabled() { return false; } -#else +// Use the symbol GRPC_USE_TRACERS to determine if tracers will be enabled in +// opt builds (tracers are always on in dbg builds). The default in OSS is for +// tracers to be on since we support binary distributions of gRPC for the +// wrapped language (wr don't want to force recompilation to get tracing). +// Internally, however, for performance reasons, we compile them out by +// default, since internal build systems make recompiling trivial. +#define GRPC_USE_TRACERS // tracers on by default in OSS +#if defined(GRPC_USE_TRACERS) || !defined(NDEBUG) bool enabled() { #ifdef GRPC_THREADSAFE_TRACER return gpr_atm_no_barrier_load(&value_) != 0; @@ -72,7 +72,9 @@ class TraceFlag { return value_; #endif // GRPC_THREADSAFE_TRACER } -#endif // COMPILE_OUT_ALL_TRACERS_IN_OPT_BUILD +#else + bool enabled() { return false; } +#endif /* defined(GRPC_USE_TRACERS) || !defined(NDEBUG) */ private: friend void grpc_core::testing::grpc_tracer_enable_flag(TraceFlag* flag); diff --git a/src/core/lib/iomgr/ev_epollex_linux.cc b/src/core/lib/iomgr/ev_epollex_linux.cc index 4c6cff7fe2..12f23ea1d6 100644 --- a/src/core/lib/iomgr/ev_epollex_linux.cc +++ b/src/core/lib/iomgr/ev_epollex_linux.cc @@ -1066,7 +1066,7 @@ static grpc_error* pollset_work(grpc_pollset* pollset, #endif if (grpc_polling_trace.enabled()) { gpr_log(GPR_INFO, - "PS:%p work hdl=%p worker=%p now=%" PRIdPTR " deadline=%" PRIdPTR + "PS:%p work hdl=%p worker=%p now=%" PRId64 " deadline=%" PRId64 " kwp=%d pollable=%p", pollset, worker_hdl, WORKER_PTR, grpc_core::ExecCtx::Get()->Now(), deadline, pollset->kicked_without_poller, pollset->active_pollable); diff --git a/src/core/lib/iomgr/ev_posix.cc b/src/core/lib/iomgr/ev_posix.cc index 6bd1dc8e50..902dca934a 100644 --- a/src/core/lib/iomgr/ev_posix.cc +++ b/src/core/lib/iomgr/ev_posix.cc @@ -251,10 +251,10 @@ static void pollset_destroy(grpc_pollset* pollset) { static grpc_error* pollset_work(grpc_pollset* pollset, grpc_pollset_worker** worker, grpc_millis deadline) { - GRPC_POLLING_API_TRACE("pollset_work(%p, %" PRIdPTR ") begin", pollset, + GRPC_POLLING_API_TRACE("pollset_work(%p, %" PRId64 ") begin", pollset, deadline); grpc_error* err = g_event_engine->pollset_work(pollset, worker, deadline); - GRPC_POLLING_API_TRACE("pollset_work(%p, %" PRIdPTR ") end", pollset, + GRPC_POLLING_API_TRACE("pollset_work(%p, %" PRId64 ") end", pollset, deadline); return err; } diff --git a/src/core/lib/iomgr/exec_ctx.cc b/src/core/lib/iomgr/exec_ctx.cc index 2f544b20ab..5d5c355ff9 100644 --- a/src/core/lib/iomgr/exec_ctx.cc +++ b/src/core/lib/iomgr/exec_ctx.cc @@ -53,24 +53,24 @@ static void exec_ctx_sched(grpc_closure* closure, grpc_error* error) { static gpr_timespec g_start_time; -static gpr_atm timespec_to_atm_round_down(gpr_timespec ts) { +static grpc_millis timespec_to_millis_round_down(gpr_timespec ts) { ts = gpr_time_sub(ts, g_start_time); double x = GPR_MS_PER_SEC * static_cast<double>(ts.tv_sec) + static_cast<double>(ts.tv_nsec) / GPR_NS_PER_MS; if (x < 0) return 0; - if (x > GPR_ATM_MAX) return GPR_ATM_MAX; - return static_cast<gpr_atm>(x); + if (x > GRPC_MILLIS_INF_FUTURE) return GRPC_MILLIS_INF_FUTURE; + return static_cast<grpc_millis>(x); } -static gpr_atm timespec_to_atm_round_up(gpr_timespec ts) { +static grpc_millis timespec_to_millis_round_up(gpr_timespec ts) { ts = gpr_time_sub(ts, g_start_time); double x = GPR_MS_PER_SEC * static_cast<double>(ts.tv_sec) + static_cast<double>(ts.tv_nsec) / GPR_NS_PER_MS + static_cast<double>(GPR_NS_PER_SEC - 1) / static_cast<double>(GPR_NS_PER_SEC); if (x < 0) return 0; - if (x > GPR_ATM_MAX) return GPR_ATM_MAX; - return static_cast<gpr_atm>(x); + if (x > GRPC_MILLIS_INF_FUTURE) return GRPC_MILLIS_INF_FUTURE; + return static_cast<grpc_millis>(x); } gpr_timespec grpc_millis_to_timespec(grpc_millis millis, @@ -92,12 +92,12 @@ gpr_timespec grpc_millis_to_timespec(grpc_millis millis, } grpc_millis grpc_timespec_to_millis_round_down(gpr_timespec ts) { - return timespec_to_atm_round_down( + return timespec_to_millis_round_down( gpr_convert_clock_type(ts, g_start_time.clock_type)); } grpc_millis grpc_timespec_to_millis_round_up(gpr_timespec ts) { - return timespec_to_atm_round_up( + return timespec_to_millis_round_up( gpr_convert_clock_type(ts, g_start_time.clock_type)); } @@ -138,7 +138,7 @@ bool ExecCtx::Flush() { grpc_millis ExecCtx::Now() { if (!now_is_valid_) { - now_ = timespec_to_atm_round_down(gpr_now(GPR_CLOCK_MONOTONIC)); + now_ = timespec_to_millis_round_down(gpr_now(GPR_CLOCK_MONOTONIC)); now_is_valid_ = true; } return now_; diff --git a/src/core/lib/iomgr/exec_ctx.h b/src/core/lib/iomgr/exec_ctx.h index 8823dc4b51..cf1118a003 100644 --- a/src/core/lib/iomgr/exec_ctx.h +++ b/src/core/lib/iomgr/exec_ctx.h @@ -29,10 +29,10 @@ #include "src/core/lib/gprpp/fork.h" #include "src/core/lib/iomgr/closure.h" -typedef gpr_atm grpc_millis; +typedef int64_t grpc_millis; -#define GRPC_MILLIS_INF_FUTURE GPR_ATM_MAX -#define GRPC_MILLIS_INF_PAST GPR_ATM_MIN +#define GRPC_MILLIS_INF_FUTURE INT64_MAX +#define GRPC_MILLIS_INF_PAST INT64_MIN /** A workqueue represents a list of work to be executed asynchronously. Forward declared here to avoid a circular dependency with workqueue.h. */ diff --git a/src/core/lib/iomgr/pollset_custom.cc b/src/core/lib/iomgr/pollset_custom.cc index 04bd104055..70e8a4596f 100644 --- a/src/core/lib/iomgr/pollset_custom.cc +++ b/src/core/lib/iomgr/pollset_custom.cc @@ -69,7 +69,7 @@ static grpc_error* pollset_work(grpc_pollset* pollset, GRPC_CUSTOM_IOMGR_ASSERT_SAME_THREAD(); gpr_mu_unlock(&pollset->mu); grpc_millis now = grpc_core::ExecCtx::Get()->Now(); - size_t timeout = 0; + grpc_millis timeout = 0; if (deadline > now) { timeout = deadline - now; } @@ -77,7 +77,7 @@ static grpc_error* pollset_work(grpc_pollset* pollset, // control back to the application grpc_core::ExecCtx* curr = grpc_core::ExecCtx::Get(); grpc_core::ExecCtx::Set(nullptr); - poller_vtable->poll(timeout); + poller_vtable->poll(static_cast<size_t>(timeout)); grpc_core::ExecCtx::Set(curr); grpc_core::ExecCtx::Get()->InvalidateNow(); if (grpc_core::ExecCtx::Get()->HasWork()) { diff --git a/src/core/lib/iomgr/timer.h b/src/core/lib/iomgr/timer.h index 5ff10d3aee..7f534476df 100644 --- a/src/core/lib/iomgr/timer.h +++ b/src/core/lib/iomgr/timer.h @@ -28,7 +28,7 @@ #include "src/core/lib/iomgr/iomgr.h" typedef struct grpc_timer { - gpr_atm deadline; + grpc_millis deadline; uint32_t heap_index; /* INVALID_HEAP_INDEX if not in heap */ bool pending; struct grpc_timer* next; diff --git a/src/core/lib/iomgr/timer_generic.cc b/src/core/lib/iomgr/timer_generic.cc index de2256f7cb..4294162af7 100644 --- a/src/core/lib/iomgr/timer_generic.cc +++ b/src/core/lib/iomgr/timer_generic.cc @@ -34,6 +34,7 @@ #include "src/core/lib/gpr/spinlock.h" #include "src/core/lib/gpr/tls.h" #include "src/core/lib/gpr/useful.h" +#include "src/core/lib/iomgr/exec_ctx.h" #include "src/core/lib/iomgr/time_averaged_stats.h" #include "src/core/lib/iomgr/timer_heap.h" @@ -59,9 +60,9 @@ typedef struct { gpr_mu mu; grpc_time_averaged_stats stats; /* All and only timers with deadlines <= this will be in the heap. */ - gpr_atm queue_deadline_cap; + grpc_millis queue_deadline_cap; /* The deadline of the next timer due in this shard */ - gpr_atm min_deadline; + grpc_millis min_deadline; /* Index of this timer_shard in the g_shard_queue */ uint32_t shard_queue_index; /* This holds all timers with deadlines < queue_deadline_cap. Timers in this @@ -209,15 +210,23 @@ static void validate_non_pending_timer(grpc_timer* t) { #endif +#if GPR_ARCH_64 +/* NOTE: TODO(sreek) - Currently the thread local storage support in grpc is + for intptr_t which means on 32-bit machines it is not wide enough to hold + grpc_millis which is 64-bit. Adding thread local support for 64 bit values + is a lot of work for very little gain. So we are currently restricting this + optimization to only 64 bit machines */ + /* Thread local variable that stores the deadline of the next timer the thread * has last-seen. This is an optimization to prevent the thread from checking * shared_mutables.min_timer (which requires acquiring shared_mutables.mu lock, * an expensive operation) */ GPR_TLS_DECL(g_last_seen_min_timer); +#endif struct shared_mutables { /* The deadline of the next timer due across all timer shards */ - gpr_atm min_timer; + grpc_millis min_timer; /* Allow only one run_some_expired_timers at once */ gpr_spinlock checker_mu; bool initialized; @@ -227,18 +236,18 @@ struct shared_mutables { static struct shared_mutables g_shared_mutables; -static gpr_atm saturating_add(gpr_atm a, gpr_atm b) { - if (a > GPR_ATM_MAX - b) { - return GPR_ATM_MAX; +static grpc_millis saturating_add(grpc_millis a, grpc_millis b) { + if (a > GRPC_MILLIS_INF_FUTURE - b) { + return GRPC_MILLIS_INF_FUTURE; } return a + b; } -static grpc_timer_check_result run_some_expired_timers(gpr_atm now, - gpr_atm* next, +static grpc_timer_check_result run_some_expired_timers(grpc_millis now, + grpc_millis* next, grpc_error* error); -static gpr_atm compute_min_deadline(timer_shard* shard) { +static grpc_millis compute_min_deadline(timer_shard* shard) { return grpc_timer_heap_is_empty(&shard->heap) ? saturating_add(shard->queue_deadline_cap, 1) : grpc_timer_heap_top(&shard->heap)->deadline; @@ -257,8 +266,11 @@ static void timer_list_init() { g_shared_mutables.checker_mu = GPR_SPINLOCK_INITIALIZER; gpr_mu_init(&g_shared_mutables.mu); g_shared_mutables.min_timer = grpc_core::ExecCtx::Get()->Now(); + +#if GPR_ARCH_64 gpr_tls_init(&g_last_seen_min_timer); gpr_tls_set(&g_last_seen_min_timer, 0); +#endif for (i = 0; i < g_num_shards; i++) { timer_shard* shard = &g_shards[i]; @@ -287,7 +299,11 @@ static void timer_list_shutdown() { grpc_timer_heap_destroy(&shard->heap); } gpr_mu_destroy(&g_shared_mutables.mu); + +#if GPR_ARCH_64 gpr_tls_destroy(&g_last_seen_min_timer); +#endif + gpr_free(g_shards); gpr_free(g_shard_queue); g_shared_mutables.initialized = false; @@ -346,7 +362,7 @@ static void timer_init(grpc_timer* timer, grpc_millis deadline, #endif if (grpc_timer_trace.enabled()) { - gpr_log(GPR_INFO, "TIMER %p: SET %" PRIdPTR " now %" PRIdPTR " call %p[%p]", + gpr_log(GPR_INFO, "TIMER %p: SET %" PRId64 " now %" PRId64 " call %p[%p]", timer, deadline, grpc_core::ExecCtx::Get()->Now(), closure, closure->cb); } @@ -383,7 +399,7 @@ static void timer_init(grpc_timer* timer, grpc_millis deadline, } if (grpc_timer_trace.enabled()) { gpr_log(GPR_INFO, - " .. add to shard %d with queue_deadline_cap=%" PRIdPTR + " .. add to shard %d with queue_deadline_cap=%" PRId64 " => is_first_timer=%s", static_cast<int>(shard - g_shards), shard->queue_deadline_cap, is_first_timer ? "true" : "false"); @@ -404,15 +420,27 @@ static void timer_init(grpc_timer* timer, grpc_millis deadline, if (is_first_timer) { gpr_mu_lock(&g_shared_mutables.mu); if (grpc_timer_trace.enabled()) { - gpr_log(GPR_INFO, " .. old shard min_deadline=%" PRIdPTR, + gpr_log(GPR_INFO, " .. old shard min_deadline=%" PRId64, shard->min_deadline); } if (deadline < shard->min_deadline) { - gpr_atm old_min_deadline = g_shard_queue[0]->min_deadline; + grpc_millis old_min_deadline = g_shard_queue[0]->min_deadline; shard->min_deadline = deadline; note_deadline_change(shard); if (shard->shard_queue_index == 0 && deadline < old_min_deadline) { - gpr_atm_no_barrier_store(&g_shared_mutables.min_timer, deadline); +#if GPR_ARCH_64 + // TODO: sreek - Using c-style cast here. static_cast<> gives an error + // (on mac platforms complaining that gpr_atm* is (long *) while + // (&g_shared_mutables.min_timer) is a (long long *). The cast should be + // safe since we know that both are pointer types and 64-bit wide. + gpr_atm_no_barrier_store((gpr_atm*)(&g_shared_mutables.min_timer), + deadline); +#else + // On 32-bit systems, gpr_atm_no_barrier_store does not work on 64-bit + // types (like grpc_millis). So all reads and writes to + // g_shared_mutables.min_timer varialbe under g_shared_mutables.mu + g_shared_mutables.min_timer = deadline; +#endif grpc_kick_poller(); } } @@ -421,8 +449,10 @@ static void timer_init(grpc_timer* timer, grpc_millis deadline, } static void timer_consume_kick(void) { - /* force re-evaluation of last seeen min */ +#if GPR_ARCH_64 + /* Force re-evaluation of last seen min */ gpr_tls_set(&g_last_seen_min_timer, 0); +#endif } static void timer_cancel(grpc_timer* timer) { @@ -459,7 +489,7 @@ static void timer_cancel(grpc_timer* timer) { 'queue_deadline_cap') into into shard->heap. Returns 'true' if shard->heap has atleast ONE element REQUIRES: shard->mu locked */ -static int refill_heap(timer_shard* shard, gpr_atm now) { +static int refill_heap(timer_shard* shard, grpc_millis now) { /* Compute the new queue window width and bound by the limits: */ double computed_deadline_delta = grpc_time_averaged_stats_update_average(&shard->stats) * @@ -472,10 +502,10 @@ static int refill_heap(timer_shard* shard, gpr_atm now) { /* Compute the new cap and put all timers under it into the queue: */ shard->queue_deadline_cap = saturating_add(GPR_MAX(now, shard->queue_deadline_cap), - static_cast<gpr_atm>(deadline_delta * 1000.0)); + static_cast<grpc_millis>(deadline_delta * 1000.0)); if (grpc_timer_check_trace.enabled()) { - gpr_log(GPR_INFO, " .. shard[%d]->queue_deadline_cap --> %" PRIdPTR, + gpr_log(GPR_INFO, " .. shard[%d]->queue_deadline_cap --> %" PRId64, static_cast<int>(shard - g_shards), shard->queue_deadline_cap); } for (timer = shard->list.next; timer != &shard->list; timer = next) { @@ -483,7 +513,7 @@ static int refill_heap(timer_shard* shard, gpr_atm now) { if (timer->deadline < shard->queue_deadline_cap) { if (grpc_timer_check_trace.enabled()) { - gpr_log(GPR_INFO, " .. add timer with deadline %" PRIdPTR " to heap", + gpr_log(GPR_INFO, " .. add timer with deadline %" PRId64 " to heap", timer->deadline); } list_remove(timer); @@ -496,7 +526,7 @@ static int refill_heap(timer_shard* shard, gpr_atm now) { /* This pops the next non-cancelled timer with deadline <= now from the queue, or returns NULL if there isn't one. REQUIRES: shard->mu locked */ -static grpc_timer* pop_one(timer_shard* shard, gpr_atm now) { +static grpc_timer* pop_one(timer_shard* shard, grpc_millis now) { grpc_timer* timer; for (;;) { if (grpc_timer_check_trace.enabled()) { @@ -511,12 +541,12 @@ static grpc_timer* pop_one(timer_shard* shard, gpr_atm now) { timer = grpc_timer_heap_top(&shard->heap); if (grpc_timer_check_trace.enabled()) { gpr_log(GPR_INFO, - " .. check top timer deadline=%" PRIdPTR " now=%" PRIdPTR, + " .. check top timer deadline=%" PRId64 " now=%" PRId64, timer->deadline, now); } if (timer->deadline > now) return nullptr; if (grpc_timer_trace.enabled()) { - gpr_log(GPR_INFO, "TIMER %p: FIRE %" PRIdPTR "ms late via %s scheduler", + gpr_log(GPR_INFO, "TIMER %p: FIRE %" PRId64 "ms late via %s scheduler", timer, now - timer->deadline, timer->closure->scheduler->vtable->name); } @@ -527,8 +557,8 @@ static grpc_timer* pop_one(timer_shard* shard, gpr_atm now) { } /* REQUIRES: shard->mu unlocked */ -static size_t pop_timers(timer_shard* shard, gpr_atm now, - gpr_atm* new_min_deadline, grpc_error* error) { +static size_t pop_timers(timer_shard* shard, grpc_millis now, + grpc_millis* new_min_deadline, grpc_error* error) { size_t n = 0; grpc_timer* timer; gpr_mu_lock(&shard->mu); @@ -546,13 +576,27 @@ static size_t pop_timers(timer_shard* shard, gpr_atm now, return n; } -static grpc_timer_check_result run_some_expired_timers(gpr_atm now, - gpr_atm* next, +static grpc_timer_check_result run_some_expired_timers(grpc_millis now, + grpc_millis* next, grpc_error* error) { grpc_timer_check_result result = GRPC_TIMERS_NOT_CHECKED; - gpr_atm min_timer = gpr_atm_no_barrier_load(&g_shared_mutables.min_timer); +#if GPR_ARCH_64 + // TODO: sreek - Using c-style cast here. static_cast<> gives an error (on + // mac platforms complaining that gpr_atm* is (long *) while + // (&g_shared_mutables.min_timer) is a (long long *). The cast should be + // safe since we know that both are pointer types and 64-bit wide + grpc_millis min_timer = static_cast<grpc_millis>( + gpr_atm_no_barrier_load((gpr_atm*)(&g_shared_mutables.min_timer))); gpr_tls_set(&g_last_seen_min_timer, min_timer); +#else + // On 32-bit systems, gpr_atm_no_barrier_load does not work on 64-bit types + // (like grpc_millis). So all reads and writes to g_shared_mutables.min_timer + // are done under g_shared_mutables.mu + gpr_mu_lock(&g_shared_mutables.mu); + grpc_millis min_timer = g_shared_mutables.min_timer; + gpr_mu_unlock(&g_shared_mutables.mu); +#endif if (now < min_timer) { if (next != nullptr) *next = GPR_MIN(*next, min_timer); return GRPC_TIMERS_CHECKED_AND_EMPTY; @@ -563,14 +607,15 @@ static grpc_timer_check_result run_some_expired_timers(gpr_atm now, result = GRPC_TIMERS_CHECKED_AND_EMPTY; if (grpc_timer_check_trace.enabled()) { - gpr_log(GPR_INFO, " .. shard[%d]->min_deadline = %" PRIdPTR, + gpr_log(GPR_INFO, " .. shard[%d]->min_deadline = %" PRId64, static_cast<int>(g_shard_queue[0] - g_shards), g_shard_queue[0]->min_deadline); } while (g_shard_queue[0]->min_deadline < now || - (now != GPR_ATM_MAX && g_shard_queue[0]->min_deadline == now)) { - gpr_atm new_min_deadline; + (now != GRPC_MILLIS_INF_FUTURE && + g_shard_queue[0]->min_deadline == now)) { + grpc_millis new_min_deadline; /* For efficiency, we pop as many available timers as we can from the shard. This may violate perfect timer deadline ordering, but that @@ -582,8 +627,8 @@ static grpc_timer_check_result run_some_expired_timers(gpr_atm now, if (grpc_timer_check_trace.enabled()) { gpr_log(GPR_INFO, " .. result --> %d" - ", shard[%d]->min_deadline %" PRIdPTR " --> %" PRIdPTR - ", now=%" PRIdPTR, + ", shard[%d]->min_deadline %" PRId64 " --> %" PRId64 + ", now=%" PRId64, result, static_cast<int>(g_shard_queue[0] - g_shards), g_shard_queue[0]->min_deadline, new_min_deadline, now); } @@ -601,8 +646,19 @@ static grpc_timer_check_result run_some_expired_timers(gpr_atm now, *next = GPR_MIN(*next, g_shard_queue[0]->min_deadline); } - gpr_atm_no_barrier_store(&g_shared_mutables.min_timer, +#if GPR_ARCH_64 + // TODO: sreek - Using c-style cast here. static_cast<> gives an error (on + // mac platforms complaining that gpr_atm* is (long *) while + // (&g_shared_mutables.min_timer) is a (long long *). The cast should be + // safe since we know that both are pointer types and 64-bit wide + gpr_atm_no_barrier_store((gpr_atm*)(&g_shared_mutables.min_timer), g_shard_queue[0]->min_deadline); +#else + // On 32-bit systems, gpr_atm_no_barrier_store does not work on 64-bit + // types (like grpc_millis). So all reads and writes to + // g_shared_mutables.min_timer are done under g_shared_mutables.mu + g_shared_mutables.min_timer = g_shard_queue[0]->min_deadline; +#endif gpr_mu_unlock(&g_shared_mutables.mu); gpr_spinlock_unlock(&g_shared_mutables.checker_mu); } @@ -616,17 +672,28 @@ static grpc_timer_check_result timer_check(grpc_millis* next) { // prelude grpc_millis now = grpc_core::ExecCtx::Get()->Now(); +#if GPR_ARCH_64 /* fetch from a thread-local first: this avoids contention on a globally mutable cacheline in the common case */ grpc_millis min_timer = gpr_tls_get(&g_last_seen_min_timer); +#else + // On 32-bit systems, we currently do not have thread local support for 64-bit + // types. In this case, directly read from g_shared_mutables.min_timer. + // Also, note that on 32-bit systems, gpr_atm_no_barrier_store does not work + // on 64-bit types (like grpc_millis). So all reads and writes to + // g_shared_mutables.min_timer are done under g_shared_mutables.mu + gpr_mu_lock(&g_shared_mutables.mu); + grpc_millis min_timer = g_shared_mutables.min_timer; + gpr_mu_unlock(&g_shared_mutables.mu); +#endif + if (now < min_timer) { if (next != nullptr) { *next = GPR_MIN(*next, min_timer); } if (grpc_timer_check_trace.enabled()) { - gpr_log(GPR_INFO, - "TIMER CHECK SKIP: now=%" PRIdPTR " min_timer=%" PRIdPTR, now, - min_timer); + gpr_log(GPR_INFO, "TIMER CHECK SKIP: now=%" PRId64 " min_timer=%" PRId64, + now, min_timer); } return GRPC_TIMERS_CHECKED_AND_EMPTY; } @@ -642,13 +709,18 @@ static grpc_timer_check_result timer_check(grpc_millis* next) { if (next == nullptr) { next_str = gpr_strdup("NULL"); } else { - gpr_asprintf(&next_str, "%" PRIdPTR, *next); + gpr_asprintf(&next_str, "%" PRId64, *next); } +#if GPR_ARCH_64 gpr_log(GPR_INFO, - "TIMER CHECK BEGIN: now=%" PRIdPTR " next=%s tls_min=%" PRIdPTR + "TIMER CHECK BEGIN: now=%" PRId64 " next=%s tls_min=%" PRId64 " glob_min=%" PRIdPTR, - now, next_str, gpr_tls_get(&g_last_seen_min_timer), - gpr_atm_no_barrier_load(&g_shared_mutables.min_timer)); + now, next_str, min_timer, + gpr_atm_no_barrier_load((gpr_atm*)(&g_shared_mutables.min_timer))); +#else + gpr_log(GPR_INFO, "TIMER CHECK BEGIN: now=%" PRId64 " next=%s min=%" PRId64, + now, next_str, min_timer); +#endif gpr_free(next_str); } // actual code @@ -660,7 +732,7 @@ static grpc_timer_check_result timer_check(grpc_millis* next) { if (next == nullptr) { next_str = gpr_strdup("NULL"); } else { - gpr_asprintf(&next_str, "%" PRIdPTR, *next); + gpr_asprintf(&next_str, "%" PRId64, *next); } gpr_log(GPR_INFO, "TIMER CHECK END: r=%d; next=%s", r, next_str); gpr_free(next_str); diff --git a/src/core/lib/iomgr/timer_manager.cc b/src/core/lib/iomgr/timer_manager.cc index 35e7914568..9fdae17909 100644 --- a/src/core/lib/iomgr/timer_manager.cc +++ b/src/core/lib/iomgr/timer_manager.cc @@ -172,7 +172,7 @@ static bool wait_until(grpc_millis next) { if (grpc_timer_check_trace.enabled()) { grpc_millis wait_time = next - grpc_core::ExecCtx::Get()->Now(); - gpr_log(GPR_INFO, "sleep for a %" PRIdPTR " milliseconds", wait_time); + gpr_log(GPR_INFO, "sleep for a %" PRId64 " milliseconds", wait_time); } } else { // g_timed_waiter == true && next >= g_timed_waiter_deadline next = GRPC_MILLIS_INF_FUTURE; diff --git a/src/core/lib/surface/call.cc b/src/core/lib/surface/call.cc index 86e0afa6ee..8946a7bedf 100644 --- a/src/core/lib/surface/call.cc +++ b/src/core/lib/surface/call.cc @@ -67,9 +67,6 @@ #define MAX_SEND_EXTRA_METADATA_COUNT 3 -// Used to create arena for the first call. -#define ESTIMATED_MDELEM_COUNT 16 - /* Status data for a request can come from several sources; this enumerates them all, and acts as a priority sorting for which status to return to the application - earlier entries override @@ -326,11 +323,6 @@ static parent_call* get_parent_call(grpc_call* call) { return (parent_call*)gpr_atm_acq_load(&call->parent_call_atm); } -size_t grpc_call_get_initial_size_estimate() { - return sizeof(grpc_call) + sizeof(batch_control) * MAX_CONCURRENT_BATCHES + - sizeof(grpc_linked_mdelem) * ESTIMATED_MDELEM_COUNT; -} - grpc_error* grpc_call_create(const grpc_call_create_args* args, grpc_call** out_call) { GPR_TIMER_SCOPE("grpc_call_create", 0); diff --git a/src/core/lib/surface/call.h b/src/core/lib/surface/call.h index e000f13e7d..793cce4efa 100644 --- a/src/core/lib/surface/call.h +++ b/src/core/lib/surface/call.h @@ -98,11 +98,6 @@ void* grpc_call_context_get(grpc_call* call, grpc_context_index elem); uint8_t grpc_call_is_client(grpc_call* call); -/* Get the estimated memory size for a call BESIDES the call stack. Combined - * with the size of the call stack, it helps estimate the arena size for the - * initial call. */ -size_t grpc_call_get_initial_size_estimate(); - /* Return an appropriate compression algorithm for the requested compression \a * level in the context of \a call. */ grpc_compression_algorithm grpc_call_compression_for_level( diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc index a466b325be..0062d0d4d5 100644 --- a/src/core/lib/surface/channel.cc +++ b/src/core/lib/surface/channel.cc @@ -108,8 +108,7 @@ grpc_channel* grpc_channel_create_with_builder( gpr_atm_no_barrier_store( &channel->call_size_estimate, - (gpr_atm)CHANNEL_STACK_FROM_CHANNEL(channel)->call_stack_size + - grpc_call_get_initial_size_estimate()); + (gpr_atm)CHANNEL_STACK_FROM_CHANNEL(channel)->call_stack_size); grpc_compression_options_init(&channel->compression_options); for (size_t i = 0; i < args->num_args; i++) { diff --git a/src/core/lib/transport/transport_op_string.cc b/src/core/lib/transport/transport_op_string.cc index 99af7c1931..25ab492f3a 100644 --- a/src/core/lib/transport/transport_op_string.cc +++ b/src/core/lib/transport/transport_op_string.cc @@ -52,7 +52,7 @@ static void put_metadata_list(gpr_strvec* b, grpc_metadata_batch md) { } if (md.deadline != GRPC_MILLIS_INF_FUTURE) { char* tmp; - gpr_asprintf(&tmp, " deadline=%" PRIdPTR, md.deadline); + gpr_asprintf(&tmp, " deadline=%" PRId64, md.deadline); gpr_strvec_add(b, tmp); } } diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 75dda9df85..5b48d06158 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -268,8 +268,10 @@ static NSString *const kBearerPrefix = @"Bearer "; // method. // TODO(jcanizales): Rename to readResponseIfNotPaused. - (void)startNextRead { - if (self.state == GRXWriterStatePaused) { - return; + @synchronized(self) { + if (self.state == GRXWriterStatePaused) { + return; + } } dispatch_async(_callQueue, ^{ |