diff --git a/include/grpcpp/server.h b/include/grpcpp/server.h index 5bbbd704a02..df68cf31441 100644 --- a/include/grpcpp/server.h +++ b/include/grpcpp/server.h @@ -248,11 +248,11 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { /// the \a sync_server_cqs) std::vector> sync_req_mgrs_; - // Outstanding callback requests. The vector is indexed by method with a - // list per method. Each element should store its own iterator - // in the list and should erase it when the request is actually bound to - // an RPC. Synchronize this list with its own mu_ (not the server mu_) since - // these must be active at Shutdown when the server mu_ is locked + // Outstanding callback requests. The vector is indexed by method with a list + // per method. Each element should store its own iterator in the list and + // should erase it when the request is actually bound to an RPC. Synchronize + // this list with its own mu_ (not the server mu_) since these must be active + // at Shutdown when the server mu_ is locked. // TODO(vjpai): Merge with the core request matcher to avoid duplicate work struct MethodReqList { std::mutex reqs_mu; @@ -274,13 +274,12 @@ class Server : public ServerInterface, private GrpcLibraryCodegen { std::condition_variable shutdown_cv_; // It is ok (but not required) to nest callback_reqs_mu_ under mu_ . - // Incrementing callback_reqs_outstanding_ is ok without a lock - // but it should only be decremented under the lock in case it is the - // last request and enables the server shutdown. The increment is - // performance-critical since it happens during periods of increasing - // load; the decrement happens only when memory is maxed out, during server - // shutdown, or (possibly in a future version) during decreasing load, so - // it is less performance-critical. + // Incrementing callback_reqs_outstanding_ is ok without a lock but it must be + // decremented under the lock in case it is the last request and enables the + // server shutdown. The increment is performance-critical since it happens + // during periods of increasing load; the decrement happens only when memory + // is maxed out, during server shutdown, or (possibly in a future version) + // during decreasing load, so it is less performance-critical. std::mutex callback_reqs_mu_; std::condition_variable callback_reqs_done_cv_; std::atomic_int callback_reqs_outstanding_{0}; diff --git a/src/core/lib/iomgr/executor.cc b/src/core/lib/iomgr/executor.cc index 34683273cf6..1e7c6a907a2 100644 --- a/src/core/lib/iomgr/executor.cc +++ b/src/core/lib/iomgr/executor.cc @@ -111,11 +111,11 @@ size_t Executor::RunClosures(const char* executor_name, grpc_closure_list list) { size_t n = 0; - // In the executor, the ExecCtx for the thread is declared - // in the executor thread itself, but this is the point where we - // could start seeing application-level callbacks. No need to - // create a new ExecCtx, though, since there already is one and it is - // flushed (but not destructed) in this function itself + // In the executor, the ExecCtx for the thread is declared in the executor + // thread itself, but this is the point where we could start seeing + // application-level callbacks. No need to create a new ExecCtx, though, + // since there already is one and it is flushed (but not destructed) in this + // function itself. grpc_core::ApplicationCallbackExecCtx callback_exec_ctx; grpc_closure* c = list.head; diff --git a/src/cpp/server/server_cc.cc b/src/cpp/server/server_cc.cc index 12aa52ef704..1e642681467 100644 --- a/src/cpp/server/server_cc.cc +++ b/src/cpp/server/server_cc.cc @@ -62,11 +62,11 @@ namespace { #define DEFAULT_CALLBACK_REQS_PER_METHOD 512 // What is the (soft) limit for outstanding requests in the server -#define MAXIMUM_CALLBACK_REQS_OUTSTANDING 30000 +#define SOFT_MAXIMUM_CALLBACK_REQS_OUTSTANDING 30000 -// If the number of unmatched requests for a method drops below this amount, -// try to allocate extra unless it pushes the total number of callbacks above -// the soft maximum +// If the number of unmatched requests for a method drops below this amount, try +// to allocate extra unless it pushes the total number of callbacks above the +// soft maximum #define SOFT_MINIMUM_SPARE_CALLBACK_REQS_PER_METHOD 128 class DefaultGlobalCallbacks final : public Server::GlobalCallbacks { @@ -185,11 +185,10 @@ class Server::SyncRequest final : public internal::CompletionQueueTag { GPR_ASSERT(cq_ && !in_flight_); in_flight_ = true; if (method_tag_) { - if (GRPC_CALL_OK != - grpc_server_request_registered_call( + if (grpc_server_request_registered_call( server, method_tag_, &call_, &deadline_, &request_metadata_, has_request_payload_ ? &request_payload_ : nullptr, cq_, - notify_cq, this)) { + notify_cq, this) != GRPC_CALL_OK) { TeardownRequest(); return; } @@ -452,7 +451,7 @@ class Server::CallbackRequest final : public internal::CompletionQueueTag { (req_->req_list_->reqs_list_sz < SOFT_MINIMUM_SPARE_CALLBACK_REQS_PER_METHOD && req_->server_->callback_reqs_outstanding_ < - MAXIMUM_CALLBACK_REQS_OUTSTANDING)) { + SOFT_MAXIMUM_CALLBACK_REQS_OUTSTANDING)) { spawn_new = true; } } @@ -528,7 +527,7 @@ class Server::CallbackRequest final : public internal::CompletionQueueTag { // load no longer justifies it. Consider measuring // dynamic load and setting a target accordingly. if (req_->server_->callback_reqs_outstanding_ < - MAXIMUM_CALLBACK_REQS_OUTSTANDING) { + SOFT_MAXIMUM_CALLBACK_REQS_OUTSTANDING) { req_->Clear(); req_->Setup(); } else {