From cefa37803acfd59071d3e7bee2bcf8537c3ec8ae Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 14 May 2016 13:20:21 -0700 Subject: [PATCH 01/24] Add affinity to ev_poll_posix --- src/core/lib/iomgr/ev_poll_posix.c | 39 +++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/src/core/lib/iomgr/ev_poll_posix.c b/src/core/lib/iomgr/ev_poll_posix.c index d1752327a2b..ba62d36507b 100644 --- a/src/core/lib/iomgr/ev_poll_posix.c +++ b/src/core/lib/iomgr/ev_poll_posix.c @@ -113,6 +113,9 @@ struct grpc_fd { grpc_closure *on_done_closure; grpc_iomgr_object iomgr_object; + + /* The pollset that last noticed and notified that the fd is readable */ + grpc_pollset *read_notifier_pollset; }; /* Begin polling on an fd. @@ -134,7 +137,8 @@ static uint32_t fd_begin_poll(grpc_fd *fd, grpc_pollset *pollset, if got_read or got_write are 1, also does the become_{readable,writable} as appropriate. */ static void fd_end_poll(grpc_exec_ctx *exec_ctx, grpc_fd_watcher *rec, - int got_read, int got_write); + int got_read, int got_write, + grpc_pollset *read_notifier_pollset); /* Return 1 if this fd is orphaned, 0 otherwise */ static bool fd_is_orphaned(grpc_fd *fd); @@ -301,6 +305,7 @@ static grpc_fd *fd_create(int fd, const char *name) { r->on_done_closure = NULL; r->closed = 0; r->released = 0; + r->read_notifier_pollset = NULL; char *name2; gpr_asprintf(&name2, "%s fd=%d", name, fd); @@ -316,6 +321,18 @@ static bool fd_is_orphaned(grpc_fd *fd) { return (gpr_atm_acq_load(&fd->refst) & 1) == 0; } +/* Return the read-notifier pollset */ +static grpc_pollset *fd_get_read_notifier_pollset(grpc_exec_ctx *exec_ctx, + grpc_fd *fd) { + grpc_pollset *notifier = NULL; + + gpr_mu_lock(&fd->mu); + notifier = fd->read_notifier_pollset; + gpr_mu_unlock(&fd->mu); + + return notifier; +} + static void pollset_kick_locked(grpc_fd_watcher *watcher) { gpr_mu_lock(&watcher->pollset->mu); GPR_ASSERT(watcher->worker); @@ -444,6 +461,11 @@ static int set_ready_locked(grpc_exec_ctx *exec_ctx, grpc_fd *fd, } } +static void set_read_notifier_pollset_locked( + grpc_exec_ctx *exec_ctx, grpc_fd *fd, grpc_pollset *read_notifier_pollset) { + fd->read_notifier_pollset = read_notifier_pollset; +} + static void fd_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd) { gpr_mu_lock(&fd->mu); GPR_ASSERT(!fd->shutdown); @@ -519,7 +541,8 @@ static uint32_t fd_begin_poll(grpc_fd *fd, grpc_pollset *pollset, } static void fd_end_poll(grpc_exec_ctx *exec_ctx, grpc_fd_watcher *watcher, - int got_read, int got_write) { + int got_read, int got_write, + grpc_pollset *read_notifier_pollset) { int was_polling = 0; int kick = 0; grpc_fd *fd = watcher->fd; @@ -555,6 +578,9 @@ static void fd_end_poll(grpc_exec_ctx *exec_ctx, grpc_fd_watcher *watcher, if (set_ready_locked(exec_ctx, fd, &fd->read_closure)) { kick = 1; } + if (read_notifier_pollset != NULL) { + set_read_notifier_pollset_locked(exec_ctx, fd, read_notifier_pollset); + } } if (got_write) { if (set_ready_locked(exec_ctx, fd, &fd->write_closure)) { @@ -899,11 +925,11 @@ static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, gpr_log(GPR_ERROR, "poll() failed: %s", strerror(errno)); } for (i = 2; i < pfd_count; i++) { - fd_end_poll(exec_ctx, &watchers[i], 0, 0); + fd_end_poll(exec_ctx, &watchers[i], 0, 0, NULL); } } else if (r == 0) { for (i = 2; i < pfd_count; i++) { - fd_end_poll(exec_ctx, &watchers[i], 0, 0); + fd_end_poll(exec_ctx, &watchers[i], 0, 0, NULL); } } else { if (pfds[0].revents & POLLIN_CHECK) { @@ -914,10 +940,10 @@ static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, } for (i = 2; i < pfd_count; i++) { if (watchers[i].fd == NULL) { - fd_end_poll(exec_ctx, &watchers[i], 0, 0); + fd_end_poll(exec_ctx, &watchers[i], 0, 0, NULL); } else { fd_end_poll(exec_ctx, &watchers[i], pfds[i].revents & POLLIN_CHECK, - pfds[i].revents & POLLOUT_CHECK); + pfds[i].revents & POLLOUT_CHECK, pollset); } } } @@ -1181,6 +1207,7 @@ static const grpc_event_engine_vtable vtable = { .fd_shutdown = fd_shutdown, .fd_notify_on_read = fd_notify_on_read, .fd_notify_on_write = fd_notify_on_write, + .fd_get_read_notifier_pollset = fd_get_read_notifier_pollset, .pollset_init = pollset_init, .pollset_shutdown = pollset_shutdown, From 11e304a3b1311fcfaf27a8b1155c2e0eb7de39b7 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 16 May 2016 17:19:52 -0700 Subject: [PATCH 02/24] Fix the failing test. (Adding fd was caling 'kicked_without_pollers' flag to be set to true on the pollset in case of 'poll' strategy. To fix this I am calling grpc_pollset_work with a 0 timeout right after adding the fds) --- test/core/iomgr/fd_posix_test.c | 50 ++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/test/core/iomgr/fd_posix_test.c b/test/core/iomgr/fd_posix_test.c index 187720e1de2..740bba2a1e7 100644 --- a/test/core/iomgr/fd_posix_test.c +++ b/test/core/iomgr/fd_posix_test.c @@ -558,9 +558,21 @@ static void free_grpc_pollset(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset) { gpr_free(pollset); } +static void pollset_work(grpc_exec_ctx *exec_ctx, gpr_mu *pollset_mu, + grpc_pollset *pollset, gpr_timespec deadline) { + grpc_pollset_worker *worker = NULL; + + /* grpc_pollset_work requires the caller to hold the pollset mutex */ + gpr_mu_lock(pollset_mu); + grpc_pollset_work(exec_ctx, pollset, &worker, gpr_now(GPR_CLOCK_MONOTONIC), + deadline); + gpr_mu_unlock(pollset_mu); + + grpc_exec_ctx_flush(exec_ctx); +} + /* This tests that the read_notifier_pollset field of a grpc_fd is properly set when the grpc_fd becomes readable - - This tests both basic and multi pollsets - The parameter register_cb_after_read_event controls whether the on-read callback registration (i.e the one done by grpc_fd_notify_on_read()) is done either before or after the fd becomes readable @@ -569,34 +581,33 @@ static void test_grpc_fd_read_notifier_pollset( bool register_cb_after_read_event) { grpc_fd *em_fd[2]; int sv[2][2]; - gpr_mu *mu[2]; - grpc_pollset *pollset[2]; + gpr_mu *mu; + grpc_pollset *pollset; char data; ssize_t result; int i; - grpc_pollset_worker *worker; read_notifier_test_fd_context fd_context; grpc_closure on_read_closure; grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; + pollset = create_grpc_pollset(&mu); + for (i = 0; i < 2; i++) { - pollset[i] = create_grpc_pollset(&mu[i]); get_socket_pair(sv[i]); /* sv[i][0] & sv[i][1] will have the socket pair */ em_fd[i] = grpc_fd_create(sv[i][0], "test_grpc_fd_read_notifier_pollset"); - grpc_pollset_add_fd(&exec_ctx, pollset[i], em_fd[i]); + grpc_pollset_add_fd(&exec_ctx, pollset, em_fd[i]); } - /* At this point pollset[0] has em_fd[0] and pollset[1] has em_fd[1] and both - are basic pollsets. Make pollset[1] a multi-pollset by adding em_fd[0] to - it */ - grpc_pollset_add_fd(&exec_ctx, pollset[1], em_fd[0]); grpc_exec_ctx_flush(&exec_ctx); - /* The following tests that the read_notifier_pollset is correctly set on the - grpc_fd structure in both basic pollset and multi pollset cases. - pollset[0] is a basic pollset containing just em_fd[0] - pollset[1] is a multi pollset containing em_fd[0] and em_fd[1] */ + /* Call grpc_pollset_work with an immediate deadline (i.e a deadline in the + past like gpr_inf_past(GPR_CLOCK_MONOTONIC) so that any work that needs to + be done as a result of adding the above file descriptors will get done */ + pollset_work(&exec_ctx, mu, pollset, gpr_inf_past(GPR_CLOCK_MONOTONIC)); + /* At this point pollset contains two fds. em_fd[0] and em_fd[1]. The + following loop makes each of these fds readable (one at a time) and checks + that the read_notifier_pollset is correctly set on those fds */ for (i = 0; i < 2; i++) { on_read_closure.cb = read_notifier_test_callback; fd_context.fd = em_fd[i]; @@ -612,14 +623,7 @@ static void test_grpc_fd_read_notifier_pollset( result = write(sv[i][1], &data, sizeof(data)); GPR_ASSERT(result == 1); - /* grpc_pollset_work requires the caller to hold the pollset mutex */ - gpr_mu_lock(mu[i]); - worker = NULL; - grpc_pollset_work(&exec_ctx, pollset[i], &worker, - gpr_now(GPR_CLOCK_MONOTONIC), - gpr_inf_future(GPR_CLOCK_MONOTONIC)); - gpr_mu_unlock(mu[i]); - grpc_exec_ctx_flush(&exec_ctx); + pollset_work(&exec_ctx, mu, pollset, gpr_inf_future(GPR_CLOCK_MONOTONIC)); if (register_cb_after_read_event) { /* Registering the callback after the fd is readable. In this case, the @@ -640,9 +644,9 @@ static void test_grpc_fd_read_notifier_pollset( for (i = 0; i < 2; i++) { grpc_fd_orphan(&exec_ctx, em_fd[i], NULL, NULL, ""); close(sv[i][1]); - free_grpc_pollset(&exec_ctx, pollset[i]); } + free_grpc_pollset(&exec_ctx, pollset); grpc_exec_ctx_finish(&exec_ctx); } From 98a185a72eb069dada018bdb49dcd17bbaa94068 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 16 May 2016 18:45:15 -0700 Subject: [PATCH 03/24] Change error to warning since we do not have a good way to determine whether the server is sync or async --- src/cpp/server/server_builder.cc | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index a2d90c29740..f6c39289b0b 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -100,11 +100,8 @@ void ServerBuilder::AddListeningPort(const grpc::string& addr, std::unique_ptr ServerBuilder::BuildAndStart() { std::unique_ptr thread_pool; - // Does this server have atleast one sync method - bool has_sync_methods = false; for (auto it = services_.begin(); it != services_.end(); ++it) { if ((*it)->service->has_synchronous_methods()) { - has_sync_methods = true; if (thread_pool == nullptr) { thread_pool.reset(CreateDefaultThreadPool()); break; @@ -134,12 +131,7 @@ std::unique_ptr ServerBuilder::BuildAndStart() { ServerInitializer* initializer = server->initializer(); - // If the server has atleast one sync methods, we know that this is a Sync - // server or a Hybrid server. This means that the completion queue on the - // Server object (i.e server->cq_) will be frequently polled (which is why - // we initialize num_frequently_pollsed_cqs to 1 here) - int num_frequently_polled_cqs = has_sync_methods ? 1 : 0; - + int num_non_listening_cqs = 0; for (auto cq = cqs_.begin(); cq != cqs_.end(); ++cq) { // A completion queue that is not polled frequently (by calling Next() or // AsyncNext()) is not safe to use for listening to incoming channels. @@ -148,17 +140,19 @@ std::unique_ptr ServerBuilder::BuildAndStart() { if ((*cq)->IsFrequentlyPolled()) { grpc_server_register_completion_queue(server->server_, (*cq)->cq(), nullptr); - num_frequently_polled_cqs++; } else { grpc_server_register_non_listening_completion_queue(server->server_, (*cq)->cq(), nullptr); + num_non_listening_cqs++; } } - if (num_frequently_polled_cqs == 0) { - gpr_log(GPR_ERROR, - "Atleast one of the completion queues must be frequently polled"); - return nullptr; + // TODO: (sreek) - Find a good way to determine whether the server is a Sync + // server or an Async server. In case of Async server, return an error if all + // the completion queues are non-listening + if (num_non_listening_cqs >= 0) { + gpr_log(GPR_INFO, "Number of non listening completion queues: %d out of %d", + num_non_listening_cqs, cqs_.size()); } for (auto service = services_.begin(); service != services_.end(); From 8c34e7c3f12af273f3f0941ddcd6d31dc7593279 Mon Sep 17 00:00:00 2001 From: Sree Kuchibhotla Date: Mon, 16 May 2016 19:26:22 -0700 Subject: [PATCH 04/24] Fix a typo --- src/cpp/server/server_builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index f6c39289b0b..a5bcd3db311 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -150,7 +150,7 @@ std::unique_ptr ServerBuilder::BuildAndStart() { // TODO: (sreek) - Find a good way to determine whether the server is a Sync // server or an Async server. In case of Async server, return an error if all // the completion queues are non-listening - if (num_non_listening_cqs >= 0) { + if (num_non_listening_cqs > 0) { gpr_log(GPR_INFO, "Number of non listening completion queues: %d out of %d", num_non_listening_cqs, cqs_.size()); } From 2afdc935990a82d196ceb8f219f7bd0ae5e774ec Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 08:43:30 -0700 Subject: [PATCH 05/24] Unrevert protobuf version --- third_party/protobuf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/protobuf b/third_party/protobuf index d5fb408ddc2..a1938b2aa9c 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit d5fb408ddc281ffcadeb08699e65bb694656d0bd +Subproject commit a1938b2aa9ca86ce7ce50c27ff9737c1008d2a03 From 29dc490b94ad087e4c8b81a9610eb83ade9bbb16 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 08:43:48 -0700 Subject: [PATCH 06/24] clang-format --- test/core/client_config/set_initial_connect_string_test.c | 3 ++- test/core/iomgr/tcp_server_posix_test.c | 3 ++- test/core/surface/concurrent_connectivity_test.c | 3 ++- test/core/util/reconnect_server.c | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/test/core/client_config/set_initial_connect_string_test.c b/test/core/client_config/set_initial_connect_string_test.c index f21d651d46b..7eb5a01bf1e 100644 --- a/test/core/client_config/set_initial_connect_string_test.c +++ b/test/core/client_config/set_initial_connect_string_test.c @@ -79,7 +79,8 @@ static void handle_read(grpc_exec_ctx *exec_ctx, void *arg, bool success) { } } -static void on_connect(grpc_exec_ctx *exec_ctx, void *arg, grpc_endpoint *tcp,grpc_pollset*accepting_pollset, +static void on_connect(grpc_exec_ctx *exec_ctx, void *arg, grpc_endpoint *tcp, + grpc_pollset *accepting_pollset, grpc_tcp_server_acceptor *acceptor) { test_tcp_server *server = arg; grpc_closure_init(&on_read, handle_read, NULL); diff --git a/test/core/iomgr/tcp_server_posix_test.c b/test/core/iomgr/tcp_server_posix_test.c index 365bfbbaa84..222ae774fc7 100644 --- a/test/core/iomgr/tcp_server_posix_test.c +++ b/test/core/iomgr/tcp_server_posix_test.c @@ -112,7 +112,8 @@ static void server_weak_ref_set(server_weak_ref *weak_ref, weak_ref->server = server; } -static void on_connect(grpc_exec_ctx *exec_ctx, void *arg, grpc_endpoint *tcp, grpc_pollset *pollset, +static void on_connect(grpc_exec_ctx *exec_ctx, void *arg, grpc_endpoint *tcp, + grpc_pollset *pollset, grpc_tcp_server_acceptor *acceptor) { grpc_endpoint_shutdown(exec_ctx, tcp); grpc_endpoint_destroy(exec_ctx, tcp); diff --git a/test/core/surface/concurrent_connectivity_test.c b/test/core/surface/concurrent_connectivity_test.c index af23fba8f3b..de9ba8d27b6 100644 --- a/test/core/surface/concurrent_connectivity_test.c +++ b/test/core/surface/concurrent_connectivity_test.c @@ -95,7 +95,8 @@ void server_thread(void *vargs) { GPR_ASSERT(detag(ev.tag) == 0xd1e); } -static void on_connect(grpc_exec_ctx *exec_ctx, void *vargs, grpc_endpoint *tcp, grpc_pollset*accepting_pollset, +static void on_connect(grpc_exec_ctx *exec_ctx, void *vargs, grpc_endpoint *tcp, + grpc_pollset *accepting_pollset, grpc_tcp_server_acceptor *acceptor) { struct server_thread_args *args = (struct server_thread_args *)vargs; (void)acceptor; diff --git a/test/core/util/reconnect_server.c b/test/core/util/reconnect_server.c index d3d8f5a23b5..6509cc5b685 100644 --- a/test/core/util/reconnect_server.c +++ b/test/core/util/reconnect_server.c @@ -70,7 +70,8 @@ static void pretty_print_backoffs(reconnect_server *server) { } } -static void on_connect(grpc_exec_ctx *exec_ctx, void *arg, grpc_endpoint *tcp, grpc_pollset *accepting_pollset, +static void on_connect(grpc_exec_ctx *exec_ctx, void *arg, grpc_endpoint *tcp, + grpc_pollset *accepting_pollset, grpc_tcp_server_acceptor *acceptor) { char *peer; char *last_colon; From 8ad69bfab5d69987d9db7c9e85a7449d9708a914 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 08:48:22 -0700 Subject: [PATCH 07/24] Attempt to fix Windows --- src/core/lib/iomgr/tcp_server_windows.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/lib/iomgr/tcp_server_windows.c b/src/core/lib/iomgr/tcp_server_windows.c index 125f521d87e..87cacfe979d 100644 --- a/src/core/lib/iomgr/tcp_server_windows.c +++ b/src/core/lib/iomgr/tcp_server_windows.c @@ -379,9 +379,10 @@ static void on_accept(grpc_exec_ctx *exec_ctx, void *arg, bool from_iocp) { /* The only time we should call our callback, is where we successfully managed to accept a connection, and created an endpoint. */ - if (ep) + if (ep) { sp->server->on_accept_cb(exec_ctx, sp->server->on_accept_cb_arg, ep, - &acceptor); + NULL, &acceptor); + } /* As we were notified from the IOCP of one and exactly one accept, the former socked we created has now either been destroy or assigned to the new connection. We need to create a new one for the next From aae6c2cb961a3cbf8fe2681a7316a270af7bf718 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 08:58:30 -0700 Subject: [PATCH 08/24] Fix server plugin test --- src/cpp/server/server_builder.cc | 2 +- test/cpp/end2end/server_builder_plugin_test.cc | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index fbcb3cef1b8..dd7e86b12c0 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -155,7 +155,7 @@ std::unique_ptr ServerBuilder::BuildAndStart() { if (num_frequently_polled_cqs == 0) { gpr_log(GPR_ERROR, - "Atleast one of the completion queues must be frequently polled"); + "At least one of the completion queues must be frequently polled"); return nullptr; } diff --git a/test/cpp/end2end/server_builder_plugin_test.cc b/test/cpp/end2end/server_builder_plugin_test.cc index 87e3709d7d2..17fc1afbd8b 100644 --- a/test/cpp/end2end/server_builder_plugin_test.cc +++ b/test/cpp/end2end/server_builder_plugin_test.cc @@ -189,6 +189,7 @@ class ServerBuilderPluginTest : public ::testing::TestWithParam { void StartServer() { grpc::string server_address = "localhost:" + to_string(port_); builder_->AddListeningPort(server_address, InsecureServerCredentials()); + cq_ = builder_->AddCompletionQueue(); server_ = builder_->BuildAndStart(); EXPECT_TRUE(builder_->plugins_[PLUGIN_NAME] != nullptr); } @@ -219,6 +220,7 @@ class ServerBuilderPluginTest : public ::testing::TestWithParam { std::unique_ptr builder_; std::unique_ptr stub_; std::unique_ptr server_; + std::unique_ptr cq_; TestServiceImpl service_; int port_; }; From 20431a8618547df86551475510f1b60bab7ad8c7 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 10:02:07 -0700 Subject: [PATCH 09/24] Fix merge --- src/cpp/server/server_builder.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 391932c88e3..daa79e5ae99 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -133,7 +133,7 @@ std::unique_ptr ServerBuilder::BuildAndStart() { // If the server has atleast one sync methods, we know that this is a Sync // server or a Hybrid server and the completion queue (server->cq_) would be // frequently polled. - int num_frequently_polled_cqs = has_sync_methods ? 1 : 0; + int num_frequently_polled_cqs = (thread_pool != nullptr) ? 1 : 0; for (auto cq = cqs_.begin(); cq != cqs_.end(); ++cq) { // A completion queue that is not polled frequently (by calling Next() or @@ -143,10 +143,10 @@ std::unique_ptr ServerBuilder::BuildAndStart() { if ((*cq)->IsFrequentlyPolled()) { grpc_server_register_completion_queue(server->server_, (*cq)->cq(), nullptr); + num_frequently_polled_cqs++; } else { grpc_server_register_non_listening_completion_queue(server->server_, (*cq)->cq(), nullptr); - num_non_listening_cqs++; } } From e004958fd691ba0fa2b9f83df5da79919d4f0313 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 10:31:09 -0700 Subject: [PATCH 10/24] Fix formatting, mem leak, stall --- src/core/lib/iomgr/tcp_server_windows.c | 4 ++-- src/cpp/server/server.cc | 4 +++- test/cpp/end2end/hybrid_end2end_test.cc | 29 +++++++++++++------------ 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/core/lib/iomgr/tcp_server_windows.c b/src/core/lib/iomgr/tcp_server_windows.c index 87cacfe979d..e15f8b0cdf2 100644 --- a/src/core/lib/iomgr/tcp_server_windows.c +++ b/src/core/lib/iomgr/tcp_server_windows.c @@ -380,8 +380,8 @@ static void on_accept(grpc_exec_ctx *exec_ctx, void *arg, bool from_iocp) { /* The only time we should call our callback, is where we successfully managed to accept a connection, and created an endpoint. */ if (ep) { - sp->server->on_accept_cb(exec_ctx, sp->server->on_accept_cb_arg, ep, - NULL, &acceptor); + sp->server->on_accept_cb(exec_ctx, sp->server->on_accept_cb_arg, ep, NULL, + &acceptor); } /* As we were notified from the IOCP of one and exactly one accept, the former socked we created has now either been destroy or assigned diff --git a/src/cpp/server/server.cc b/src/cpp/server/server.cc index 854057efbc4..f6c3e5747c9 100644 --- a/src/cpp/server/server.cc +++ b/src/cpp/server/server.cc @@ -412,7 +412,9 @@ bool Server::Start(ServerCompletionQueue** cqs, size_t num_cqs) { sync_methods_->push_back(SyncRequest(unknown_method_.get(), nullptr)); } for (size_t i = 0; i < num_cqs; i++) { - new UnimplementedAsyncRequest(this, cqs[i]); + if (cqs[i]->IsFrequentlyPolled()) { + new UnimplementedAsyncRequest(this, cqs[i]); + } } } // Start processing rpcs. diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index 0423448154d..208e7d589fa 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -199,7 +199,8 @@ class HybridEnd2endTest : public ::testing::Test { HybridEnd2endTest() {} void SetUpServer(::grpc::Service* service1, ::grpc::Service* service2, - AsyncGenericService* generic_service) { + AsyncGenericService* generic_service, + int num_cqs_frequently_polled) { int port = grpc_pick_unused_port_or_die(); server_address_ << "localhost:" << port; @@ -216,7 +217,7 @@ class HybridEnd2endTest : public ::testing::Test { } // Create a separate cq for each potential handler. for (int i = 0; i < 5; i++) { - cqs_.push_back(builder.AddCompletionQueue(false)); + cqs_.push_back(builder.AddCompletionQueue(i < num_cqs_frequently_polled)); } server_ = builder.BuildAndStart(); } @@ -346,7 +347,7 @@ class HybridEnd2endTest : public ::testing::Test { TEST_F(HybridEnd2endTest, AsyncEcho) { EchoTestService::WithAsyncMethod_Echo service; - SetUpServer(&service, nullptr, nullptr); + SetUpServer(&service, nullptr, nullptr, 1); ResetStub(); std::thread echo_handler_thread( [this, &service] { HandleEcho(&service, cqs_[0].get(), false); }); @@ -358,7 +359,7 @@ TEST_F(HybridEnd2endTest, AsyncEchoRequestStream) { EchoTestService::WithAsyncMethod_RequestStream< EchoTestService::WithAsyncMethod_Echo > service; - SetUpServer(&service, nullptr, nullptr); + SetUpServer(&service, nullptr, nullptr, 2); ResetStub(); std::thread echo_handler_thread( [this, &service] { HandleEcho(&service, cqs_[0].get(), false); }); @@ -373,7 +374,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream) { EchoTestService::WithAsyncMethod_RequestStream< EchoTestService::WithAsyncMethod_ResponseStream > service; - SetUpServer(&service, nullptr, nullptr); + SetUpServer(&service, nullptr, nullptr, 2); ResetStub(); std::thread response_stream_handler_thread( [this, &service] { HandleServerStreaming(&service, cqs_[0].get()); }); @@ -390,7 +391,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream_SyncDupService) { EchoTestService::WithAsyncMethod_ResponseStream > service; TestServiceImplDupPkg dup_service; - SetUpServer(&service, &dup_service, nullptr); + SetUpServer(&service, &dup_service, nullptr, 2); ResetStub(); std::thread response_stream_handler_thread( [this, &service] { HandleServerStreaming(&service, cqs_[0].get()); }); @@ -408,7 +409,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream_AsyncDupService) { EchoTestService::WithAsyncMethod_ResponseStream > service; duplicate::EchoTestService::AsyncService dup_service; - SetUpServer(&service, &dup_service, nullptr); + SetUpServer(&service, &dup_service, nullptr, 3); ResetStub(); std::thread response_stream_handler_thread( [this, &service] { HandleServerStreaming(&service, cqs_[0].get()); }); @@ -426,7 +427,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream_AsyncDupService) { TEST_F(HybridEnd2endTest, GenericEcho) { EchoTestService::WithGenericMethod_Echo service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service); + SetUpServer(&service, nullptr, &generic_service, 1); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -440,7 +441,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStream) { EchoTestService::WithGenericMethod_Echo > service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service); + SetUpServer(&service, nullptr, &generic_service, 2); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -459,7 +460,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStream_SyncDupService) { service; AsyncGenericService generic_service; TestServiceImplDupPkg dup_service; - SetUpServer(&service, &dup_service, &generic_service); + SetUpServer(&service, &dup_service, &generic_service, 2); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -479,7 +480,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStream_AsyncDupService) { service; AsyncGenericService generic_service; duplicate::EchoTestService::AsyncService dup_service; - SetUpServer(&service, &dup_service, &generic_service); + SetUpServer(&service, &dup_service, &generic_service, 3); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -501,7 +502,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStreamResponseStream) { EchoTestService::WithAsyncMethod_ResponseStream > > service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service); + SetUpServer(&service, nullptr, &generic_service, 3); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -522,7 +523,7 @@ TEST_F(HybridEnd2endTest, GenericEchoRequestStreamAsyncResponseStream) { EchoTestService::WithAsyncMethod_ResponseStream > > service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service); + SetUpServer(&service, nullptr, &generic_service, 3); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -545,7 +546,7 @@ TEST_F(HybridEnd2endTest, GenericMethodWithoutGenericService) { EchoTestService::WithGenericMethod_Echo< EchoTestService::WithAsyncMethod_ResponseStream > > service; - SetUpServer(&service, nullptr, nullptr); + SetUpServer(&service, nullptr, nullptr, 0); EXPECT_EQ(nullptr, server_.get()); } From 718ce51af8a24dcf3c02f1f02c9ba7ed1e17c9ee Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 10:44:23 -0700 Subject: [PATCH 11/24] Fix server plugin test --- test/cpp/end2end/server_builder_plugin_test.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/cpp/end2end/server_builder_plugin_test.cc b/test/cpp/end2end/server_builder_plugin_test.cc index 17fc1afbd8b..8a74621e5ac 100644 --- a/test/cpp/end2end/server_builder_plugin_test.cc +++ b/test/cpp/end2end/server_builder_plugin_test.cc @@ -207,6 +207,12 @@ class ServerBuilderPluginTest : public ::testing::TestWithParam { EXPECT_TRUE(plugin != nullptr); EXPECT_TRUE(plugin->init_server_is_called()); EXPECT_TRUE(plugin->finish_is_called()); + server_->Shutdown(); + void* tag; + bool ok; + cq_->Shutdown(); + while (cq_->Next(&tag, &ok)) + ; } string to_string(const int number) { @@ -219,8 +225,8 @@ class ServerBuilderPluginTest : public ::testing::TestWithParam { std::shared_ptr channel_; std::unique_ptr builder_; std::unique_ptr stub_; - std::unique_ptr server_; std::unique_ptr cq_; + std::unique_ptr server_; TestServiceImpl service_; int port_; }; From bd24a4611f1a827f2da050c8bf6bf4f9216c9aae Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 11:15:58 -0700 Subject: [PATCH 12/24] Better guesses at benchmarks turned unit tests cpu cost --- test/cpp/qps/gen_build_yaml.py | 20 +++++++++++++++++++- tools/run_tests/tests.json | 32 ++++++++++++++++---------------- 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/test/cpp/qps/gen_build_yaml.py b/test/cpp/qps/gen_build_yaml.py index 6b3329b046b..e8ad74f0f63 100755 --- a/test/cpp/qps/gen_build_yaml.py +++ b/test/cpp/qps/gen_build_yaml.py @@ -46,6 +46,24 @@ import performance.scenario_config as scenario_config def _scenario_json_string(scenario_json): return json.dumps(scenario_config.remove_nonproto_fields(scenario_json)) +def threads_of_type(scenario_json, path): + d = scenario_json + for el in path.split('/'): + if el not in d: + return 0 + d = d[el] + return d + +def guess_cpu(scenario_json): + client = threads_of_type(scenario_json, 'client_config/async_client_threads') + server = threads_of_type(scenario_json, 'server_config/async_server_threads') + # make an arbitrary guess if set to auto-detect + # about the size of the jenkins instances we have for unit tests + if client == 0: client = 8 + if server == 0: server = 8 + return (scenario_json['num_clients'] * client + + scenario_json['num_servers'] * server) + print yaml.dump({ 'tests': [ { @@ -59,7 +77,7 @@ print yaml.dump({ 'language': 'c++', 'boringssl': True, 'defaults': 'boringssl', - 'cpu_cost': 1000.0, + 'cpu_cost': guess_cpu(scenario_json), 'exclude_configs': [] } for scenario_json in scenario_config.CXXLanguage().scenarios() diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 10674d22b31..da597effa75 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -23031,7 +23031,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23057,7 +23057,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23083,7 +23083,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23109,7 +23109,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23135,7 +23135,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 8, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23161,7 +23161,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 8, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23187,7 +23187,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 8, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23213,7 +23213,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 1, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23239,7 +23239,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23265,7 +23265,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23291,7 +23291,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23317,7 +23317,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 2, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23343,7 +23343,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 8, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23369,7 +23369,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 8, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23395,7 +23395,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 8, "defaults": "boringssl", "exclude_configs": [], "flaky": false, @@ -23421,7 +23421,7 @@ "posix", "windows" ], - "cpu_cost": 1000.0, + "cpu_cost": 1, "defaults": "boringssl", "exclude_configs": [], "flaky": false, From 8a7fe1a0eff0981a470596845ec23bf05825c3b4 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 11:17:20 -0700 Subject: [PATCH 13/24] Fix crash --- src/cpp/server/server_builder.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index daa79e5ae99..5966e548b09 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -100,10 +100,12 @@ void ServerBuilder::AddListeningPort(const grpc::string& addr, std::unique_ptr ServerBuilder::BuildAndStart() { std::unique_ptr thread_pool; + bool has_sync_methods = false; for (auto it = services_.begin(); it != services_.end(); ++it) { if ((*it)->service->has_synchronous_methods()) { if (thread_pool == nullptr) { thread_pool.reset(CreateDefaultThreadPool()); + has_sync_methods = true; break; } } @@ -133,7 +135,7 @@ std::unique_ptr ServerBuilder::BuildAndStart() { // If the server has atleast one sync methods, we know that this is a Sync // server or a Hybrid server and the completion queue (server->cq_) would be // frequently polled. - int num_frequently_polled_cqs = (thread_pool != nullptr) ? 1 : 0; + int num_frequently_polled_cqs = has_sync_methods ? 1 : 0; for (auto cq = cqs_.begin(); cq != cqs_.end(); ++cq) { // A completion queue that is not polled frequently (by calling Next() or From f1cde58049332ef1522452f84a1dc736eba662e3 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 12:13:23 -0700 Subject: [PATCH 14/24] Disable legacy poller testing: seems like there is a bug, its legacy --- tools/run_tests/run_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 0a5625c3f5b..5f0943b440f 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -157,7 +157,7 @@ class CLanguage(object): 'windows': ['all'], 'mac': ['all'], 'posix': ['all'], - 'linux': ['poll', 'legacy'] + 'linux': ['poll'], # DISABLED DUE TO BUGS: 'legacy' } for target in binaries: polling_strategies = (POLLING_STRATEGIES[self.platform] From d88e15cee750cd647a900098d82f87cc25aa8dbe Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 12:22:37 -0700 Subject: [PATCH 15/24] Remove legacy poller --- BUILD | 6 - Makefile | 2 - binding.gyp | 1 - build.yaml | 2 - config.m4 | 1 - gRPC.podspec | 3 - grpc.gemspec | 2 - package.xml | 2 - src/core/lib/iomgr/ev_poll_and_epoll_posix.c | 1978 ----------------- src/core/lib/iomgr/ev_poll_and_epoll_posix.h | 41 - src/core/lib/iomgr/ev_poll_posix.c | 2 + src/core/lib/iomgr/ev_posix.c | 3 +- src/python/grpcio/grpc_core_dependencies.py | 1 - third_party/protobuf | 2 +- tools/doxygen/Doxyfile.core.internal | 2 - tools/run_tests/run_tests.py | 2 +- tools/run_tests/sources_and_headers.json | 3 - vsprojects/vcxproj/grpc/grpc.vcxproj | 3 - vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 6 - .../grpc_unsecure/grpc_unsecure.vcxproj | 3 - .../grpc_unsecure.vcxproj.filters | 6 - 21 files changed, 5 insertions(+), 2066 deletions(-) delete mode 100644 src/core/lib/iomgr/ev_poll_and_epoll_posix.c delete mode 100644 src/core/lib/iomgr/ev_poll_and_epoll_posix.h diff --git a/BUILD b/BUILD index 793c1c714de..0f8d8c77109 100644 --- a/BUILD +++ b/BUILD @@ -178,7 +178,6 @@ cc_library( "src/core/lib/iomgr/closure.h", "src/core/lib/iomgr/endpoint.h", "src/core/lib/iomgr/endpoint_pair.h", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.h", "src/core/lib/iomgr/ev_poll_posix.h", "src/core/lib/iomgr/ev_posix.h", "src/core/lib/iomgr/exec_ctx.h", @@ -313,7 +312,6 @@ cc_library( "src/core/lib/iomgr/endpoint.c", "src/core/lib/iomgr/endpoint_pair_posix.c", "src/core/lib/iomgr/endpoint_pair_windows.c", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.c", "src/core/lib/iomgr/ev_poll_posix.c", "src/core/lib/iomgr/ev_posix.c", "src/core/lib/iomgr/exec_ctx.c", @@ -531,7 +529,6 @@ cc_library( "src/core/lib/iomgr/closure.h", "src/core/lib/iomgr/endpoint.h", "src/core/lib/iomgr/endpoint_pair.h", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.h", "src/core/lib/iomgr/ev_poll_posix.h", "src/core/lib/iomgr/ev_posix.h", "src/core/lib/iomgr/exec_ctx.h", @@ -652,7 +649,6 @@ cc_library( "src/core/lib/iomgr/endpoint.c", "src/core/lib/iomgr/endpoint_pair_posix.c", "src/core/lib/iomgr/endpoint_pair_windows.c", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.c", "src/core/lib/iomgr/ev_poll_posix.c", "src/core/lib/iomgr/ev_posix.c", "src/core/lib/iomgr/exec_ctx.c", @@ -1345,7 +1341,6 @@ objc_library( "src/core/lib/iomgr/endpoint.c", "src/core/lib/iomgr/endpoint_pair_posix.c", "src/core/lib/iomgr/endpoint_pair_windows.c", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.c", "src/core/lib/iomgr/ev_poll_posix.c", "src/core/lib/iomgr/ev_posix.c", "src/core/lib/iomgr/exec_ctx.c", @@ -1542,7 +1537,6 @@ objc_library( "src/core/lib/iomgr/closure.h", "src/core/lib/iomgr/endpoint.h", "src/core/lib/iomgr/endpoint_pair.h", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.h", "src/core/lib/iomgr/ev_poll_posix.h", "src/core/lib/iomgr/ev_posix.h", "src/core/lib/iomgr/exec_ctx.h", diff --git a/Makefile b/Makefile index c93c9a42412..949f8669c7a 100644 --- a/Makefile +++ b/Makefile @@ -2510,7 +2510,6 @@ LIBGRPC_SRC = \ src/core/lib/iomgr/endpoint.c \ src/core/lib/iomgr/endpoint_pair_posix.c \ src/core/lib/iomgr/endpoint_pair_windows.c \ - src/core/lib/iomgr/ev_poll_and_epoll_posix.c \ src/core/lib/iomgr/ev_poll_posix.c \ src/core/lib/iomgr/ev_posix.c \ src/core/lib/iomgr/exec_ctx.c \ @@ -2857,7 +2856,6 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/iomgr/endpoint.c \ src/core/lib/iomgr/endpoint_pair_posix.c \ src/core/lib/iomgr/endpoint_pair_windows.c \ - src/core/lib/iomgr/ev_poll_and_epoll_posix.c \ src/core/lib/iomgr/ev_poll_posix.c \ src/core/lib/iomgr/ev_posix.c \ src/core/lib/iomgr/exec_ctx.c \ diff --git a/binding.gyp b/binding.gyp index 760bb24d72a..442a14762c2 100644 --- a/binding.gyp +++ b/binding.gyp @@ -581,7 +581,6 @@ 'src/core/lib/iomgr/endpoint.c', 'src/core/lib/iomgr/endpoint_pair_posix.c', 'src/core/lib/iomgr/endpoint_pair_windows.c', - 'src/core/lib/iomgr/ev_poll_and_epoll_posix.c', 'src/core/lib/iomgr/ev_poll_posix.c', 'src/core/lib/iomgr/ev_posix.c', 'src/core/lib/iomgr/exec_ctx.c', diff --git a/build.yaml b/build.yaml index 68e814f76c8..acf2f9307f7 100644 --- a/build.yaml +++ b/build.yaml @@ -165,7 +165,6 @@ filegroups: - src/core/lib/iomgr/closure.h - src/core/lib/iomgr/endpoint.h - src/core/lib/iomgr/endpoint_pair.h - - src/core/lib/iomgr/ev_poll_and_epoll_posix.h - src/core/lib/iomgr/ev_poll_posix.h - src/core/lib/iomgr/ev_posix.h - src/core/lib/iomgr/exec_ctx.h @@ -240,7 +239,6 @@ filegroups: - src/core/lib/iomgr/endpoint.c - src/core/lib/iomgr/endpoint_pair_posix.c - src/core/lib/iomgr/endpoint_pair_windows.c - - src/core/lib/iomgr/ev_poll_and_epoll_posix.c - src/core/lib/iomgr/ev_poll_posix.c - src/core/lib/iomgr/ev_posix.c - src/core/lib/iomgr/exec_ctx.c diff --git a/config.m4 b/config.m4 index 6ed1887fef3..8f2cfa24a4e 100644 --- a/config.m4 +++ b/config.m4 @@ -100,7 +100,6 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/iomgr/endpoint.c \ src/core/lib/iomgr/endpoint_pair_posix.c \ src/core/lib/iomgr/endpoint_pair_windows.c \ - src/core/lib/iomgr/ev_poll_and_epoll_posix.c \ src/core/lib/iomgr/ev_poll_posix.c \ src/core/lib/iomgr/ev_posix.c \ src/core/lib/iomgr/exec_ctx.c \ diff --git a/gRPC.podspec b/gRPC.podspec index 67e7a8174f8..ea02aa34870 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -181,7 +181,6 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/closure.h', 'src/core/lib/iomgr/endpoint.h', 'src/core/lib/iomgr/endpoint_pair.h', - 'src/core/lib/iomgr/ev_poll_and_epoll_posix.h', 'src/core/lib/iomgr/ev_poll_posix.h', 'src/core/lib/iomgr/ev_posix.h', 'src/core/lib/iomgr/exec_ctx.h', @@ -350,7 +349,6 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/endpoint.c', 'src/core/lib/iomgr/endpoint_pair_posix.c', 'src/core/lib/iomgr/endpoint_pair_windows.c', - 'src/core/lib/iomgr/ev_poll_and_epoll_posix.c', 'src/core/lib/iomgr/ev_poll_posix.c', 'src/core/lib/iomgr/ev_posix.c', 'src/core/lib/iomgr/exec_ctx.c', @@ -531,7 +529,6 @@ Pod::Spec.new do |s| 'src/core/lib/iomgr/closure.h', 'src/core/lib/iomgr/endpoint.h', 'src/core/lib/iomgr/endpoint_pair.h', - 'src/core/lib/iomgr/ev_poll_and_epoll_posix.h', 'src/core/lib/iomgr/ev_poll_posix.h', 'src/core/lib/iomgr/ev_posix.h', 'src/core/lib/iomgr/exec_ctx.h', diff --git a/grpc.gemspec b/grpc.gemspec index 13aed6b61c2..72f044258b8 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -190,7 +190,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/iomgr/closure.h ) s.files += %w( src/core/lib/iomgr/endpoint.h ) s.files += %w( src/core/lib/iomgr/endpoint_pair.h ) - s.files += %w( src/core/lib/iomgr/ev_poll_and_epoll_posix.h ) s.files += %w( src/core/lib/iomgr/ev_poll_posix.h ) s.files += %w( src/core/lib/iomgr/ev_posix.h ) s.files += %w( src/core/lib/iomgr/exec_ctx.h ) @@ -329,7 +328,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/iomgr/endpoint.c ) s.files += %w( src/core/lib/iomgr/endpoint_pair_posix.c ) s.files += %w( src/core/lib/iomgr/endpoint_pair_windows.c ) - s.files += %w( src/core/lib/iomgr/ev_poll_and_epoll_posix.c ) s.files += %w( src/core/lib/iomgr/ev_poll_posix.c ) s.files += %w( src/core/lib/iomgr/ev_posix.c ) s.files += %w( src/core/lib/iomgr/exec_ctx.c ) diff --git a/package.xml b/package.xml index a169ad24e7e..a9b0ee4be2e 100644 --- a/package.xml +++ b/package.xml @@ -197,7 +197,6 @@ - @@ -336,7 +335,6 @@ - diff --git a/src/core/lib/iomgr/ev_poll_and_epoll_posix.c b/src/core/lib/iomgr/ev_poll_and_epoll_posix.c deleted file mode 100644 index 943c404f917..00000000000 --- a/src/core/lib/iomgr/ev_poll_and_epoll_posix.c +++ /dev/null @@ -1,1978 +0,0 @@ -/* - * - * Copyright 2015, Google Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - */ - -/* This file will be removed shortly: it's here to keep refactoring - * steps simple and auditable. - * It's the combination of the old files: - * - fd_posix.{h,c} - * - pollset_posix.{h,c} - * - pullset_multipoller_with_{poll,epoll}.{h,c} - * The new version will be split into: - * - ev_poll_posix.{h,c} - * - ev_epoll_posix.{h,c} - */ - -#include - -#ifdef GPR_POSIX_SOCKET - -#include "src/core/lib/iomgr/ev_poll_and_epoll_posix.h" - -#include -#include -#include -#include -#include -#include - -#include -#include -#include -#include -#include - -#include "src/core/lib/iomgr/iomgr_internal.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" - -/******************************************************************************* - * FD declarations - */ - -typedef struct grpc_fd_watcher { - struct grpc_fd_watcher *next; - struct grpc_fd_watcher *prev; - grpc_pollset *pollset; - grpc_pollset_worker *worker; - grpc_fd *fd; -} grpc_fd_watcher; - -struct grpc_fd { - int fd; - /* refst format: - bit0: 1=active/0=orphaned - bit1-n: refcount - meaning that mostly we ref by two to avoid altering the orphaned bit, - and just unref by 1 when we're ready to flag the object as orphaned */ - gpr_atm refst; - - gpr_mu mu; - int shutdown; - int closed; - int released; - - /* The watcher list. - - The following watcher related fields are protected by watcher_mu. - - An fd_watcher is an ephemeral object created when an fd wants to - begin polling, and destroyed after the poll. - - It denotes the fd's interest in whether to read poll or write poll - or both or neither on this fd. - - If a watcher is asked to poll for reads or writes, the read_watcher - or write_watcher fields are set respectively. A watcher may be asked - to poll for both, in which case both fields will be set. - - read_watcher and write_watcher may be NULL if no watcher has been - asked to poll for reads or writes. - - If an fd_watcher is not asked to poll for reads or writes, it's added - to a linked list of inactive watchers, rooted at inactive_watcher_root. - If at a later time there becomes need of a poller to poll, one of - the inactive pollers may be kicked out of their poll loops to take - that responsibility. */ - grpc_fd_watcher inactive_watcher_root; - grpc_fd_watcher *read_watcher; - grpc_fd_watcher *write_watcher; - - grpc_closure *read_closure; - grpc_closure *write_closure; - - struct grpc_fd *freelist_next; - - grpc_closure *on_done_closure; - - grpc_iomgr_object iomgr_object; - - /* The pollset that last noticed and notified that the fd is readable */ - grpc_pollset *read_notifier_pollset; -}; - -/* Begin polling on an fd. - Registers that the given pollset is interested in this fd - so that if read - or writability interest changes, the pollset can be kicked to pick up that - new interest. - Return value is: - (fd_needs_read? read_mask : 0) | (fd_needs_write? write_mask : 0) - i.e. a combination of read_mask and write_mask determined by the fd's current - interest in said events. - Polling strategies that do not need to alter their behavior depending on the - fd's current interest (such as epoll) do not need to call this function. - MUST NOT be called with a pollset lock taken */ -static uint32_t fd_begin_poll(grpc_fd *fd, grpc_pollset *pollset, - grpc_pollset_worker *worker, uint32_t read_mask, - uint32_t write_mask, grpc_fd_watcher *rec); -/* Complete polling previously started with fd_begin_poll - MUST NOT be called with a pollset lock taken - if got_read or got_write are 1, also does the become_{readable,writable} as - appropriate. */ -static void fd_end_poll(grpc_exec_ctx *exec_ctx, grpc_fd_watcher *rec, - int got_read, int got_write, - grpc_pollset *read_notifier_pollset); - -/* Return 1 if this fd is orphaned, 0 otherwise */ -static bool fd_is_orphaned(grpc_fd *fd); - -/* Reference counting for fds */ -/*#define GRPC_FD_REF_COUNT_DEBUG*/ -#ifdef GRPC_FD_REF_COUNT_DEBUG -static void fd_ref(grpc_fd *fd, const char *reason, const char *file, int line); -static void fd_unref(grpc_fd *fd, const char *reason, const char *file, - int line); -#define GRPC_FD_REF(fd, reason) fd_ref(fd, reason, __FILE__, __LINE__) -#define GRPC_FD_UNREF(fd, reason) fd_unref(fd, reason, __FILE__, __LINE__) -#else -static void fd_ref(grpc_fd *fd); -static void fd_unref(grpc_fd *fd); -#define GRPC_FD_REF(fd, reason) fd_ref(fd) -#define GRPC_FD_UNREF(fd, reason) fd_unref(fd) -#endif - -static void fd_global_init(void); -static void fd_global_shutdown(void); - -#define CLOSURE_NOT_READY ((grpc_closure *)0) -#define CLOSURE_READY ((grpc_closure *)1) - -/******************************************************************************* - * pollset declarations - */ - -typedef struct grpc_pollset_vtable grpc_pollset_vtable; - -typedef struct grpc_cached_wakeup_fd { - grpc_wakeup_fd fd; - struct grpc_cached_wakeup_fd *next; -} grpc_cached_wakeup_fd; - -struct grpc_pollset_worker { - grpc_cached_wakeup_fd *wakeup_fd; - int reevaluate_polling_on_wakeup; - int kicked_specifically; - struct grpc_pollset_worker *next; - struct grpc_pollset_worker *prev; -}; - -struct grpc_pollset { - /* pollsets under posix can mutate representation as fds are added and - removed. - For example, we may choose a poll() based implementation on linux for - few fds, and an epoll() based implementation for many fds */ - const grpc_pollset_vtable *vtable; - gpr_mu mu; - grpc_pollset_worker root_worker; - int in_flight_cbs; - int shutting_down; - int called_shutdown; - int kicked_without_pollers; - grpc_closure *shutdown_done; - grpc_closure_list idle_jobs; - union { - int fd; - void *ptr; - } data; - /* Local cache of eventfds for workers */ - grpc_cached_wakeup_fd *local_wakeup_cache; -}; - -struct grpc_pollset_vtable { - void (*add_fd)(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - struct grpc_fd *fd, int and_unlock_pollset); - void (*maybe_work_and_unlock)(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - grpc_pollset_worker *worker, - gpr_timespec deadline, gpr_timespec now); - void (*finish_shutdown)(grpc_pollset *pollset); - void (*destroy)(grpc_pollset *pollset); -}; - -/* Add an fd to a pollset */ -static void pollset_add_fd(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - struct grpc_fd *fd); - -static void pollset_set_add_fd(grpc_exec_ctx *exec_ctx, - grpc_pollset_set *pollset_set, grpc_fd *fd); - -/* Convert a timespec to milliseconds: - - very small or negative poll times are clamped to zero to do a - non-blocking poll (which becomes spin polling) - - other small values are rounded up to one millisecond - - longer than a millisecond polls are rounded up to the next nearest - millisecond to avoid spinning - - infinite timeouts are converted to -1 */ -static int poll_deadline_to_millis_timeout(gpr_timespec deadline, - gpr_timespec now); - -/* Allow kick to wakeup the currently polling worker */ -#define GRPC_POLLSET_CAN_KICK_SELF 1 -/* Force the wakee to repoll when awoken */ -#define GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP 2 -/* As per pollset_kick, with an extended set of flags (defined above) - -- mostly for fd_posix's use. */ -static void pollset_kick_ext(grpc_pollset *p, - grpc_pollset_worker *specific_worker, - uint32_t flags); - -/* turn a pollset into a multipoller: platform specific */ -typedef void (*platform_become_multipoller_type)(grpc_exec_ctx *exec_ctx, - grpc_pollset *pollset, - struct grpc_fd **fds, - size_t fd_count); -static platform_become_multipoller_type platform_become_multipoller; - -/* Return 1 if the pollset has active threads in pollset_work (pollset must - * be locked) */ -static int pollset_has_workers(grpc_pollset *pollset); - -static void remove_fd_from_all_epoll_sets(int fd); - -/******************************************************************************* - * pollset_set definitions - */ - -struct grpc_pollset_set { - gpr_mu mu; - - size_t pollset_count; - size_t pollset_capacity; - grpc_pollset **pollsets; - - size_t pollset_set_count; - size_t pollset_set_capacity; - struct grpc_pollset_set **pollset_sets; - - size_t fd_count; - size_t fd_capacity; - grpc_fd **fds; -}; - -/******************************************************************************* - * fd_posix.c - */ - -/* We need to keep a freelist not because of any concerns of malloc performance - * but instead so that implementations with multiple threads in (for example) - * epoll_wait deal with the race between pollset removal and incoming poll - * notifications. - * - * The problem is that the poller ultimately holds a reference to this - * object, so it is very difficult to know when is safe to free it, at least - * without some expensive synchronization. - * - * If we keep the object freelisted, in the worst case losing this race just - * becomes a spurious read notification on a reused fd. - */ -/* TODO(klempner): We could use some form of polling generation count to know - * when these are safe to free. */ -/* TODO(klempner): Consider disabling freelisting if we don't have multiple - * threads in poll on the same fd */ -/* TODO(klempner): Batch these allocations to reduce fragmentation */ -static grpc_fd *fd_freelist = NULL; -static gpr_mu fd_freelist_mu; - -static void freelist_fd(grpc_fd *fd) { - gpr_mu_lock(&fd_freelist_mu); - fd->freelist_next = fd_freelist; - fd_freelist = fd; - grpc_iomgr_unregister_object(&fd->iomgr_object); - gpr_mu_unlock(&fd_freelist_mu); -} - -static grpc_fd *alloc_fd(int fd) { - grpc_fd *r = NULL; - gpr_mu_lock(&fd_freelist_mu); - if (fd_freelist != NULL) { - r = fd_freelist; - fd_freelist = fd_freelist->freelist_next; - } - gpr_mu_unlock(&fd_freelist_mu); - if (r == NULL) { - r = gpr_malloc(sizeof(grpc_fd)); - gpr_mu_init(&r->mu); - } - - gpr_mu_lock(&r->mu); - gpr_atm_rel_store(&r->refst, 1); - r->shutdown = 0; - r->read_closure = CLOSURE_NOT_READY; - r->write_closure = CLOSURE_NOT_READY; - r->fd = fd; - r->inactive_watcher_root.next = r->inactive_watcher_root.prev = - &r->inactive_watcher_root; - r->freelist_next = NULL; - r->read_watcher = r->write_watcher = NULL; - r->on_done_closure = NULL; - r->closed = 0; - r->released = 0; - r->read_notifier_pollset = NULL; - gpr_mu_unlock(&r->mu); - return r; -} - -static void destroy(grpc_fd *fd) { - gpr_mu_destroy(&fd->mu); - gpr_free(fd); -} - -#ifdef GRPC_FD_REF_COUNT_DEBUG -#define REF_BY(fd, n, reason) ref_by(fd, n, reason, __FILE__, __LINE__) -#define UNREF_BY(fd, n, reason) unref_by(fd, n, reason, __FILE__, __LINE__) -static void ref_by(grpc_fd *fd, int n, const char *reason, const char *file, - int line) { - gpr_log(GPR_DEBUG, "FD %d %p ref %d %d -> %d [%s; %s:%d]", fd->fd, fd, n, - gpr_atm_no_barrier_load(&fd->refst), - gpr_atm_no_barrier_load(&fd->refst) + n, reason, file, line); -#else -#define REF_BY(fd, n, reason) ref_by(fd, n) -#define UNREF_BY(fd, n, reason) unref_by(fd, n) -static void ref_by(grpc_fd *fd, int n) { -#endif - GPR_ASSERT(gpr_atm_no_barrier_fetch_add(&fd->refst, n) > 0); -} - -#ifdef GRPC_FD_REF_COUNT_DEBUG -static void unref_by(grpc_fd *fd, int n, const char *reason, const char *file, - int line) { - gpr_atm old; - gpr_log(GPR_DEBUG, "FD %d %p unref %d %d -> %d [%s; %s:%d]", fd->fd, fd, n, - gpr_atm_no_barrier_load(&fd->refst), - gpr_atm_no_barrier_load(&fd->refst) - n, reason, file, line); -#else -static void unref_by(grpc_fd *fd, int n) { - gpr_atm old; -#endif - old = gpr_atm_full_fetch_add(&fd->refst, -n); - if (old == n) { - freelist_fd(fd); - } else { - GPR_ASSERT(old > n); - } -} - -static void fd_global_init(void) { gpr_mu_init(&fd_freelist_mu); } - -static void fd_global_shutdown(void) { - gpr_mu_lock(&fd_freelist_mu); - gpr_mu_unlock(&fd_freelist_mu); - while (fd_freelist != NULL) { - grpc_fd *fd = fd_freelist; - fd_freelist = fd_freelist->freelist_next; - destroy(fd); - } - gpr_mu_destroy(&fd_freelist_mu); -} - -static grpc_fd *fd_create(int fd, const char *name) { - grpc_fd *r = alloc_fd(fd); - char *name2; - gpr_asprintf(&name2, "%s fd=%d", name, fd); - grpc_iomgr_register_object(&r->iomgr_object, name2); - gpr_free(name2); -#ifdef GRPC_FD_REF_COUNT_DEBUG - gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, r, name); -#endif - return r; -} - -static bool fd_is_orphaned(grpc_fd *fd) { - return (gpr_atm_acq_load(&fd->refst) & 1) == 0; -} - -static void pollset_kick_locked(grpc_fd_watcher *watcher) { - gpr_mu_lock(&watcher->pollset->mu); - GPR_ASSERT(watcher->worker); - pollset_kick_ext(watcher->pollset, watcher->worker, - GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP); - gpr_mu_unlock(&watcher->pollset->mu); -} - -static void maybe_wake_one_watcher_locked(grpc_fd *fd) { - if (fd->inactive_watcher_root.next != &fd->inactive_watcher_root) { - pollset_kick_locked(fd->inactive_watcher_root.next); - } else if (fd->read_watcher) { - pollset_kick_locked(fd->read_watcher); - } else if (fd->write_watcher) { - pollset_kick_locked(fd->write_watcher); - } -} - -static void wake_all_watchers_locked(grpc_fd *fd) { - grpc_fd_watcher *watcher; - for (watcher = fd->inactive_watcher_root.next; - watcher != &fd->inactive_watcher_root; watcher = watcher->next) { - pollset_kick_locked(watcher); - } - if (fd->read_watcher) { - pollset_kick_locked(fd->read_watcher); - } - if (fd->write_watcher && fd->write_watcher != fd->read_watcher) { - pollset_kick_locked(fd->write_watcher); - } -} - -static int has_watchers(grpc_fd *fd) { - return fd->read_watcher != NULL || fd->write_watcher != NULL || - fd->inactive_watcher_root.next != &fd->inactive_watcher_root; -} - -static void close_fd_locked(grpc_exec_ctx *exec_ctx, grpc_fd *fd) { - fd->closed = 1; - if (!fd->released) { - close(fd->fd); - } else { - remove_fd_from_all_epoll_sets(fd->fd); - } - grpc_exec_ctx_enqueue(exec_ctx, fd->on_done_closure, true, NULL); -} - -static int fd_wrapped_fd(grpc_fd *fd) { - if (fd->released || fd->closed) { - return -1; - } else { - return fd->fd; - } -} - -static void fd_orphan(grpc_exec_ctx *exec_ctx, grpc_fd *fd, - grpc_closure *on_done, int *release_fd, - const char *reason) { - fd->on_done_closure = on_done; - fd->released = release_fd != NULL; - if (!fd->released) { - shutdown(fd->fd, SHUT_RDWR); - } else { - *release_fd = fd->fd; - } - gpr_mu_lock(&fd->mu); - REF_BY(fd, 1, reason); /* remove active status, but keep referenced */ - if (!has_watchers(fd)) { - close_fd_locked(exec_ctx, fd); - } else { - wake_all_watchers_locked(fd); - } - gpr_mu_unlock(&fd->mu); - UNREF_BY(fd, 2, reason); /* drop the reference */ -} - -/* increment refcount by two to avoid changing the orphan bit */ -#ifdef GRPC_FD_REF_COUNT_DEBUG -static void fd_ref(grpc_fd *fd, const char *reason, const char *file, - int line) { - ref_by(fd, 2, reason, file, line); -} - -static void fd_unref(grpc_fd *fd, const char *reason, const char *file, - int line) { - unref_by(fd, 2, reason, file, line); -} -#else -static void fd_ref(grpc_fd *fd) { ref_by(fd, 2); } - -static void fd_unref(grpc_fd *fd) { unref_by(fd, 2); } -#endif - -static void notify_on_locked(grpc_exec_ctx *exec_ctx, grpc_fd *fd, - grpc_closure **st, grpc_closure *closure) { - if (*st == CLOSURE_NOT_READY) { - /* not ready ==> switch to a waiting state by setting the closure */ - *st = closure; - } else if (*st == CLOSURE_READY) { - /* already ready ==> queue the closure to run immediately */ - *st = CLOSURE_NOT_READY; - grpc_exec_ctx_enqueue(exec_ctx, closure, !fd->shutdown, NULL); - maybe_wake_one_watcher_locked(fd); - } else { - /* upcallptr was set to a different closure. This is an error! */ - gpr_log(GPR_ERROR, - "User called a notify_on function with a previous callback still " - "pending"); - abort(); - } -} - -/* returns 1 if state becomes not ready */ -static int set_ready_locked(grpc_exec_ctx *exec_ctx, grpc_fd *fd, - grpc_closure **st) { - if (*st == CLOSURE_READY) { - /* duplicate ready ==> ignore */ - return 0; - } else if (*st == CLOSURE_NOT_READY) { - /* not ready, and not waiting ==> flag ready */ - *st = CLOSURE_READY; - return 0; - } else { - /* waiting ==> queue closure */ - grpc_exec_ctx_enqueue(exec_ctx, *st, !fd->shutdown, NULL); - *st = CLOSURE_NOT_READY; - return 1; - } -} - -static void set_read_notifier_pollset_locked( - grpc_exec_ctx *exec_ctx, grpc_fd *fd, grpc_pollset *read_notifier_pollset) { - fd->read_notifier_pollset = read_notifier_pollset; -} - -static void fd_shutdown(grpc_exec_ctx *exec_ctx, grpc_fd *fd) { - gpr_mu_lock(&fd->mu); - GPR_ASSERT(!fd->shutdown); - fd->shutdown = 1; - set_ready_locked(exec_ctx, fd, &fd->read_closure); - set_ready_locked(exec_ctx, fd, &fd->write_closure); - gpr_mu_unlock(&fd->mu); -} - -static void fd_notify_on_read(grpc_exec_ctx *exec_ctx, grpc_fd *fd, - grpc_closure *closure) { - gpr_mu_lock(&fd->mu); - notify_on_locked(exec_ctx, fd, &fd->read_closure, closure); - gpr_mu_unlock(&fd->mu); -} - -static void fd_notify_on_write(grpc_exec_ctx *exec_ctx, grpc_fd *fd, - grpc_closure *closure) { - gpr_mu_lock(&fd->mu); - notify_on_locked(exec_ctx, fd, &fd->write_closure, closure); - gpr_mu_unlock(&fd->mu); -} - -/* Return the read-notifier pollset */ -static grpc_pollset *fd_get_read_notifier_pollset(grpc_exec_ctx *exec_ctx, - grpc_fd *fd) { - grpc_pollset *notifier = NULL; - - gpr_mu_lock(&fd->mu); - notifier = fd->read_notifier_pollset; - gpr_mu_unlock(&fd->mu); - - return notifier; -} - -static uint32_t fd_begin_poll(grpc_fd *fd, grpc_pollset *pollset, - grpc_pollset_worker *worker, uint32_t read_mask, - uint32_t write_mask, grpc_fd_watcher *watcher) { - uint32_t mask = 0; - grpc_closure *cur; - int requested; - /* keep track of pollers that have requested our events, in case they change - */ - GRPC_FD_REF(fd, "poll"); - - gpr_mu_lock(&fd->mu); - - /* if we are shutdown, then don't add to the watcher set */ - if (fd->shutdown) { - watcher->fd = NULL; - watcher->pollset = NULL; - watcher->worker = NULL; - gpr_mu_unlock(&fd->mu); - GRPC_FD_UNREF(fd, "poll"); - return 0; - } - - /* if there is nobody polling for read, but we need to, then start doing so */ - cur = fd->read_closure; - requested = cur != CLOSURE_READY; - if (read_mask && fd->read_watcher == NULL && requested) { - fd->read_watcher = watcher; - mask |= read_mask; - } - /* if there is nobody polling for write, but we need to, then start doing so - */ - cur = fd->write_closure; - requested = cur != CLOSURE_READY; - if (write_mask && fd->write_watcher == NULL && requested) { - fd->write_watcher = watcher; - mask |= write_mask; - } - /* if not polling, remember this watcher in case we need someone to later */ - if (mask == 0 && worker != NULL) { - watcher->next = &fd->inactive_watcher_root; - watcher->prev = watcher->next->prev; - watcher->next->prev = watcher->prev->next = watcher; - } - watcher->pollset = pollset; - watcher->worker = worker; - watcher->fd = fd; - gpr_mu_unlock(&fd->mu); - - return mask; -} - -static void fd_end_poll(grpc_exec_ctx *exec_ctx, grpc_fd_watcher *watcher, - int got_read, int got_write, - grpc_pollset *read_notifier_pollset) { - int was_polling = 0; - int kick = 0; - grpc_fd *fd = watcher->fd; - - if (fd == NULL) { - return; - } - - gpr_mu_lock(&fd->mu); - - if (watcher == fd->read_watcher) { - /* remove read watcher, kick if we still need a read */ - was_polling = 1; - if (!got_read) { - kick = 1; - } - fd->read_watcher = NULL; - } - if (watcher == fd->write_watcher) { - /* remove write watcher, kick if we still need a write */ - was_polling = 1; - if (!got_write) { - kick = 1; - } - fd->write_watcher = NULL; - } - if (!was_polling && watcher->worker != NULL) { - /* remove from inactive list */ - watcher->next->prev = watcher->prev; - watcher->prev->next = watcher->next; - } - if (got_read) { - if (set_ready_locked(exec_ctx, fd, &fd->read_closure)) { - kick = 1; - } - - if (read_notifier_pollset != NULL) { - set_read_notifier_pollset_locked(exec_ctx, fd, read_notifier_pollset); - } - } - if (got_write) { - if (set_ready_locked(exec_ctx, fd, &fd->write_closure)) { - kick = 1; - } - } - if (kick) { - maybe_wake_one_watcher_locked(fd); - } - if (fd_is_orphaned(fd) && !has_watchers(fd) && !fd->closed) { - close_fd_locked(exec_ctx, fd); - } - gpr_mu_unlock(&fd->mu); - - GRPC_FD_UNREF(fd, "poll"); -} - -/******************************************************************************* - * pollset_posix.c - */ - -GPR_TLS_DECL(g_current_thread_poller); -GPR_TLS_DECL(g_current_thread_worker); - -/** The alarm system needs to be able to wakeup 'some poller' sometimes - * (specifically when a new alarm needs to be triggered earlier than the next - * alarm 'epoch'). - * This wakeup_fd gives us something to alert on when such a case occurs. */ -grpc_wakeup_fd grpc_global_wakeup_fd; - -static void remove_worker(grpc_pollset *p, grpc_pollset_worker *worker) { - worker->prev->next = worker->next; - worker->next->prev = worker->prev; -} - -static int pollset_has_workers(grpc_pollset *p) { - return p->root_worker.next != &p->root_worker; -} - -static grpc_pollset_worker *pop_front_worker(grpc_pollset *p) { - if (pollset_has_workers(p)) { - grpc_pollset_worker *w = p->root_worker.next; - remove_worker(p, w); - return w; - } else { - return NULL; - } -} - -static void push_back_worker(grpc_pollset *p, grpc_pollset_worker *worker) { - worker->next = &p->root_worker; - worker->prev = worker->next->prev; - worker->prev->next = worker->next->prev = worker; -} - -static void push_front_worker(grpc_pollset *p, grpc_pollset_worker *worker) { - worker->prev = &p->root_worker; - worker->next = worker->prev->next; - worker->prev->next = worker->next->prev = worker; -} - -static void pollset_kick_ext(grpc_pollset *p, - grpc_pollset_worker *specific_worker, - uint32_t flags) { - GPR_TIMER_BEGIN("pollset_kick_ext", 0); - - /* pollset->mu already held */ - if (specific_worker != NULL) { - if (specific_worker == GRPC_POLLSET_KICK_BROADCAST) { - GPR_TIMER_BEGIN("pollset_kick_ext.broadcast", 0); - GPR_ASSERT((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) == 0); - for (specific_worker = p->root_worker.next; - specific_worker != &p->root_worker; - specific_worker = specific_worker->next) { - grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd->fd); - } - p->kicked_without_pollers = 1; - GPR_TIMER_END("pollset_kick_ext.broadcast", 0); - } else if (gpr_tls_get(&g_current_thread_worker) != - (intptr_t)specific_worker) { - GPR_TIMER_MARK("different_thread_worker", 0); - if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) { - specific_worker->reevaluate_polling_on_wakeup = 1; - } - specific_worker->kicked_specifically = 1; - grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd->fd); - } else if ((flags & GRPC_POLLSET_CAN_KICK_SELF) != 0) { - GPR_TIMER_MARK("kick_yoself", 0); - if ((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) != 0) { - specific_worker->reevaluate_polling_on_wakeup = 1; - } - specific_worker->kicked_specifically = 1; - grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd->fd); - } - } else if (gpr_tls_get(&g_current_thread_poller) != (intptr_t)p) { - GPR_ASSERT((flags & GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) == 0); - GPR_TIMER_MARK("kick_anonymous", 0); - specific_worker = pop_front_worker(p); - if (specific_worker != NULL) { - if (gpr_tls_get(&g_current_thread_worker) == (intptr_t)specific_worker) { - /* Prefer not to kick self. Push the worker to the end of the list and - * pop the one from front */ - GPR_TIMER_MARK("kick_anonymous_not_self", 0); - push_back_worker(p, specific_worker); - specific_worker = pop_front_worker(p); - /* If there was only one worker on the pollset, we would get the same - * worker we pushed (the one set on current thread local) back. If so, - * kick it only if GRPC_POLLSET_CAN_KICK_SELF flag is set */ - if ((flags & GRPC_POLLSET_CAN_KICK_SELF) == 0 && - gpr_tls_get(&g_current_thread_worker) == - (intptr_t)specific_worker) { - push_back_worker(p, specific_worker); - specific_worker = NULL; - } - } - if (specific_worker != NULL) { - GPR_TIMER_MARK("finally_kick", 0); - push_back_worker(p, specific_worker); - grpc_wakeup_fd_wakeup(&specific_worker->wakeup_fd->fd); - } - } else { - GPR_TIMER_MARK("kicked_no_pollers", 0); - p->kicked_without_pollers = 1; - } - } - - GPR_TIMER_END("pollset_kick_ext", 0); -} - -static void pollset_kick(grpc_pollset *p, - grpc_pollset_worker *specific_worker) { - pollset_kick_ext(p, specific_worker, 0); -} - -/* global state management */ - -static void pollset_global_init(void) { - gpr_tls_init(&g_current_thread_poller); - gpr_tls_init(&g_current_thread_worker); - grpc_wakeup_fd_init(&grpc_global_wakeup_fd); -} - -static void pollset_global_shutdown(void) { - grpc_wakeup_fd_destroy(&grpc_global_wakeup_fd); - gpr_tls_destroy(&g_current_thread_poller); - gpr_tls_destroy(&g_current_thread_worker); -} - -static void kick_poller(void) { grpc_wakeup_fd_wakeup(&grpc_global_wakeup_fd); } - -/* main interface */ - -static void become_basic_pollset(grpc_pollset *pollset, grpc_fd *fd_or_null); - -static void pollset_init(grpc_pollset *pollset, gpr_mu **mu) { - gpr_mu_init(&pollset->mu); - *mu = &pollset->mu; - pollset->root_worker.next = pollset->root_worker.prev = &pollset->root_worker; - pollset->in_flight_cbs = 0; - pollset->shutting_down = 0; - pollset->called_shutdown = 0; - pollset->kicked_without_pollers = 0; - pollset->idle_jobs.head = pollset->idle_jobs.tail = NULL; - pollset->local_wakeup_cache = NULL; - pollset->kicked_without_pollers = 0; - become_basic_pollset(pollset, NULL); -} - -static void pollset_destroy(grpc_pollset *pollset) { - GPR_ASSERT(pollset->in_flight_cbs == 0); - GPR_ASSERT(!pollset_has_workers(pollset)); - GPR_ASSERT(pollset->idle_jobs.head == pollset->idle_jobs.tail); - pollset->vtable->destroy(pollset); - while (pollset->local_wakeup_cache) { - grpc_cached_wakeup_fd *next = pollset->local_wakeup_cache->next; - grpc_wakeup_fd_destroy(&pollset->local_wakeup_cache->fd); - gpr_free(pollset->local_wakeup_cache); - pollset->local_wakeup_cache = next; - } - gpr_mu_destroy(&pollset->mu); -} - -static void pollset_reset(grpc_pollset *pollset) { - GPR_ASSERT(pollset->shutting_down); - GPR_ASSERT(pollset->in_flight_cbs == 0); - GPR_ASSERT(!pollset_has_workers(pollset)); - GPR_ASSERT(pollset->idle_jobs.head == pollset->idle_jobs.tail); - pollset->vtable->destroy(pollset); - pollset->shutting_down = 0; - pollset->called_shutdown = 0; - pollset->kicked_without_pollers = 0; - become_basic_pollset(pollset, NULL); -} - -static void pollset_add_fd(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - grpc_fd *fd) { - gpr_mu_lock(&pollset->mu); - pollset->vtable->add_fd(exec_ctx, pollset, fd, 1); -/* the following (enabled only in debug) will reacquire and then release - our lock - meaning that if the unlocking flag passed to add_fd above is - not respected, the code will deadlock (in a way that we have a chance of - debugging) */ -#ifndef NDEBUG - gpr_mu_lock(&pollset->mu); - gpr_mu_unlock(&pollset->mu); -#endif -} - -static void finish_shutdown(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset) { - GPR_ASSERT(grpc_closure_list_empty(pollset->idle_jobs)); - pollset->vtable->finish_shutdown(pollset); - grpc_exec_ctx_enqueue(exec_ctx, pollset->shutdown_done, true, NULL); -} - -static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - grpc_pollset_worker **worker_hdl, gpr_timespec now, - gpr_timespec deadline) { - grpc_pollset_worker worker; - *worker_hdl = &worker; - - /* pollset->mu already held */ - int added_worker = 0; - int locked = 1; - int queued_work = 0; - int keep_polling = 0; - GPR_TIMER_BEGIN("pollset_work", 0); - /* this must happen before we (potentially) drop pollset->mu */ - worker.next = worker.prev = NULL; - worker.reevaluate_polling_on_wakeup = 0; - if (pollset->local_wakeup_cache != NULL) { - worker.wakeup_fd = pollset->local_wakeup_cache; - pollset->local_wakeup_cache = worker.wakeup_fd->next; - } else { - worker.wakeup_fd = gpr_malloc(sizeof(*worker.wakeup_fd)); - grpc_wakeup_fd_init(&worker.wakeup_fd->fd); - } - worker.kicked_specifically = 0; - /* If there's work waiting for the pollset to be idle, and the - pollset is idle, then do that work */ - if (!pollset_has_workers(pollset) && - !grpc_closure_list_empty(pollset->idle_jobs)) { - GPR_TIMER_MARK("pollset_work.idle_jobs", 0); - grpc_exec_ctx_enqueue_list(exec_ctx, &pollset->idle_jobs, NULL); - goto done; - } - /* If we're shutting down then we don't execute any extended work */ - if (pollset->shutting_down) { - GPR_TIMER_MARK("pollset_work.shutting_down", 0); - goto done; - } - /* Give do_promote priority so we don't starve it out */ - if (pollset->in_flight_cbs) { - GPR_TIMER_MARK("pollset_work.in_flight_cbs", 0); - gpr_mu_unlock(&pollset->mu); - locked = 0; - goto done; - } - /* Start polling, and keep doing so while we're being asked to - re-evaluate our pollers (this allows poll() based pollers to - ensure they don't miss wakeups) */ - keep_polling = 1; - while (keep_polling) { - keep_polling = 0; - if (!pollset->kicked_without_pollers) { - if (!added_worker) { - push_front_worker(pollset, &worker); - added_worker = 1; - gpr_tls_set(&g_current_thread_worker, (intptr_t)&worker); - } - gpr_tls_set(&g_current_thread_poller, (intptr_t)pollset); - GPR_TIMER_BEGIN("maybe_work_and_unlock", 0); - pollset->vtable->maybe_work_and_unlock(exec_ctx, pollset, &worker, - deadline, now); - GPR_TIMER_END("maybe_work_and_unlock", 0); - locked = 0; - gpr_tls_set(&g_current_thread_poller, 0); - } else { - GPR_TIMER_MARK("pollset_work.kicked_without_pollers", 0); - pollset->kicked_without_pollers = 0; - } - /* Finished execution - start cleaning up. - Note that we may arrive here from outside the enclosing while() loop. - In that case we won't loop though as we haven't added worker to the - worker list, which means nobody could ask us to re-evaluate polling). */ - done: - if (!locked) { - queued_work |= grpc_exec_ctx_flush(exec_ctx); - gpr_mu_lock(&pollset->mu); - locked = 1; - } - /* If we're forced to re-evaluate polling (via pollset_kick with - GRPC_POLLSET_REEVALUATE_POLLING_ON_WAKEUP) then we land here and force - a loop */ - if (worker.reevaluate_polling_on_wakeup) { - worker.reevaluate_polling_on_wakeup = 0; - pollset->kicked_without_pollers = 0; - if (queued_work || worker.kicked_specifically) { - /* If there's queued work on the list, then set the deadline to be - immediate so we get back out of the polling loop quickly */ - deadline = gpr_inf_past(GPR_CLOCK_MONOTONIC); - } - keep_polling = 1; - } - } - if (added_worker) { - remove_worker(pollset, &worker); - gpr_tls_set(&g_current_thread_worker, 0); - } - /* release wakeup fd to the local pool */ - worker.wakeup_fd->next = pollset->local_wakeup_cache; - pollset->local_wakeup_cache = worker.wakeup_fd; - /* check shutdown conditions */ - if (pollset->shutting_down) { - if (pollset_has_workers(pollset)) { - pollset_kick(pollset, NULL); - } else if (!pollset->called_shutdown && pollset->in_flight_cbs == 0) { - pollset->called_shutdown = 1; - gpr_mu_unlock(&pollset->mu); - finish_shutdown(exec_ctx, pollset); - grpc_exec_ctx_flush(exec_ctx); - /* Continuing to access pollset here is safe -- it is the caller's - * responsibility to not destroy when it has outstanding calls to - * pollset_work. - * TODO(dklempner): Can we refactor the shutdown logic to avoid this? */ - gpr_mu_lock(&pollset->mu); - } else if (!grpc_closure_list_empty(pollset->idle_jobs)) { - grpc_exec_ctx_enqueue_list(exec_ctx, &pollset->idle_jobs, NULL); - gpr_mu_unlock(&pollset->mu); - grpc_exec_ctx_flush(exec_ctx); - gpr_mu_lock(&pollset->mu); - } - } - *worker_hdl = NULL; - GPR_TIMER_END("pollset_work", 0); -} - -static void pollset_shutdown(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - grpc_closure *closure) { - GPR_ASSERT(!pollset->shutting_down); - pollset->shutting_down = 1; - pollset->shutdown_done = closure; - pollset_kick(pollset, GRPC_POLLSET_KICK_BROADCAST); - if (!pollset_has_workers(pollset)) { - grpc_exec_ctx_enqueue_list(exec_ctx, &pollset->idle_jobs, NULL); - } - if (!pollset->called_shutdown && pollset->in_flight_cbs == 0 && - !pollset_has_workers(pollset)) { - pollset->called_shutdown = 1; - finish_shutdown(exec_ctx, pollset); - } -} - -static int poll_deadline_to_millis_timeout(gpr_timespec deadline, - gpr_timespec now) { - gpr_timespec timeout; - static const int64_t max_spin_polling_us = 10; - if (gpr_time_cmp(deadline, gpr_inf_future(deadline.clock_type)) == 0) { - return -1; - } - if (gpr_time_cmp(deadline, gpr_time_add(now, gpr_time_from_micros( - max_spin_polling_us, - GPR_TIMESPAN))) <= 0) { - return 0; - } - timeout = gpr_time_sub(deadline, now); - return gpr_time_to_millis(gpr_time_add( - timeout, gpr_time_from_nanos(GPR_NS_PER_MS - 1, GPR_TIMESPAN))); -} - -/* - * basic_pollset - a vtable that provides polling for zero or one file - * descriptor via poll() - */ - -typedef struct grpc_unary_promote_args { - const grpc_pollset_vtable *original_vtable; - grpc_pollset *pollset; - grpc_fd *fd; - grpc_closure promotion_closure; -} grpc_unary_promote_args; - -static void basic_do_promote(grpc_exec_ctx *exec_ctx, void *args, - bool success) { - grpc_unary_promote_args *up_args = args; - const grpc_pollset_vtable *original_vtable = up_args->original_vtable; - grpc_pollset *pollset = up_args->pollset; - grpc_fd *fd = up_args->fd; - - /* - * This is quite tricky. There are a number of cases to keep in mind here: - * 1. fd may have been orphaned - * 2. The pollset may no longer be a unary poller (and we can't let case #1 - * leak to other pollset types!) - * 3. pollset's fd (which may have changed) may have been orphaned - * 4. The pollset may be shutting down. - */ - - gpr_mu_lock(&pollset->mu); - /* First we need to ensure that nobody is polling concurrently */ - GPR_ASSERT(!pollset_has_workers(pollset)); - - gpr_free(up_args); - /* At this point the pollset may no longer be a unary poller. In that case - * we should just call the right add function and be done. */ - /* TODO(klempner): If we're not careful this could cause infinite recursion. - * That's not a problem for now because empty_pollset has a trivial poller - * and we don't have any mechanism to unbecome multipoller. */ - pollset->in_flight_cbs--; - if (pollset->shutting_down) { - /* We don't care about this pollset anymore. */ - if (pollset->in_flight_cbs == 0 && !pollset->called_shutdown) { - pollset->called_shutdown = 1; - finish_shutdown(exec_ctx, pollset); - } - } else if (fd_is_orphaned(fd)) { - /* Don't try to add it to anything, we'll drop our ref on it below */ - } else if (pollset->vtable != original_vtable) { - pollset->vtable->add_fd(exec_ctx, pollset, fd, 0); - } else if (fd != pollset->data.ptr) { - grpc_fd *fds[2]; - fds[0] = pollset->data.ptr; - fds[1] = fd; - - if (fds[0] && !fd_is_orphaned(fds[0])) { - platform_become_multipoller(exec_ctx, pollset, fds, GPR_ARRAY_SIZE(fds)); - GRPC_FD_UNREF(fds[0], "basicpoll"); - } else { - /* old fd is orphaned and we haven't cleaned it up until now, so remain a - * unary poller */ - /* Note that it is possible that fds[1] is also orphaned at this point. - * That's okay, we'll correct it at the next add or poll. */ - if (fds[0]) GRPC_FD_UNREF(fds[0], "basicpoll"); - pollset->data.ptr = fd; - GRPC_FD_REF(fd, "basicpoll"); - } - } - - gpr_mu_unlock(&pollset->mu); - - /* Matching ref in basic_pollset_add_fd */ - GRPC_FD_UNREF(fd, "basicpoll_add"); -} - -static void basic_pollset_add_fd(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - grpc_fd *fd, int and_unlock_pollset) { - grpc_unary_promote_args *up_args; - GPR_ASSERT(fd); - if (fd == pollset->data.ptr) goto exit; - - if (!pollset_has_workers(pollset)) { - /* Fast path -- no in flight cbs */ - /* TODO(klempner): Comment this out and fix any test failures or establish - * they are due to timing issues */ - grpc_fd *fds[2]; - fds[0] = pollset->data.ptr; - fds[1] = fd; - - if (fds[0] == NULL) { - pollset->data.ptr = fd; - GRPC_FD_REF(fd, "basicpoll"); - } else if (!fd_is_orphaned(fds[0])) { - platform_become_multipoller(exec_ctx, pollset, fds, GPR_ARRAY_SIZE(fds)); - GRPC_FD_UNREF(fds[0], "basicpoll"); - } else { - /* old fd is orphaned and we haven't cleaned it up until now, so remain a - * unary poller */ - GRPC_FD_UNREF(fds[0], "basicpoll"); - pollset->data.ptr = fd; - GRPC_FD_REF(fd, "basicpoll"); - } - goto exit; - } - - /* Now we need to promote. This needs to happen when we're not polling. Since - * this may be called from poll, the wait needs to happen asynchronously. */ - GRPC_FD_REF(fd, "basicpoll_add"); - pollset->in_flight_cbs++; - up_args = gpr_malloc(sizeof(*up_args)); - up_args->fd = fd; - up_args->original_vtable = pollset->vtable; - up_args->pollset = pollset; - up_args->promotion_closure.cb = basic_do_promote; - up_args->promotion_closure.cb_arg = up_args; - - grpc_closure_list_add(&pollset->idle_jobs, &up_args->promotion_closure, 1); - pollset_kick(pollset, GRPC_POLLSET_KICK_BROADCAST); - -exit: - if (and_unlock_pollset) { - gpr_mu_unlock(&pollset->mu); - } -} - -static void basic_pollset_maybe_work_and_unlock(grpc_exec_ctx *exec_ctx, - grpc_pollset *pollset, - grpc_pollset_worker *worker, - gpr_timespec deadline, - gpr_timespec now) { -#define POLLOUT_CHECK (POLLOUT | POLLHUP | POLLERR) -#define POLLIN_CHECK (POLLIN | POLLHUP | POLLERR) - - struct pollfd pfd[3]; - grpc_fd *fd; - grpc_fd_watcher fd_watcher; - int timeout; - int r; - nfds_t nfds; - - fd = pollset->data.ptr; - if (fd && fd_is_orphaned(fd)) { - GRPC_FD_UNREF(fd, "basicpoll"); - fd = pollset->data.ptr = NULL; - } - timeout = poll_deadline_to_millis_timeout(deadline, now); - pfd[0].fd = GRPC_WAKEUP_FD_GET_READ_FD(&grpc_global_wakeup_fd); - pfd[0].events = POLLIN; - pfd[0].revents = 0; - pfd[1].fd = GRPC_WAKEUP_FD_GET_READ_FD(&worker->wakeup_fd->fd); - pfd[1].events = POLLIN; - pfd[1].revents = 0; - nfds = 2; - if (fd) { - pfd[2].fd = fd->fd; - pfd[2].revents = 0; - GRPC_FD_REF(fd, "basicpoll_begin"); - gpr_mu_unlock(&pollset->mu); - pfd[2].events = - (short)fd_begin_poll(fd, pollset, worker, POLLIN, POLLOUT, &fd_watcher); - if (pfd[2].events != 0) { - nfds++; - } - } else { - gpr_mu_unlock(&pollset->mu); - } - - /* TODO(vpai): Consider first doing a 0 timeout poll here to avoid - even going into the blocking annotation if possible */ - /* poll fd count (argument 2) is shortened by one if we have no events - to poll on - such that it only includes the kicker */ - GPR_TIMER_BEGIN("poll", 0); - GRPC_SCHEDULING_START_BLOCKING_REGION; - r = grpc_poll_function(pfd, nfds, timeout); - GRPC_SCHEDULING_END_BLOCKING_REGION; - GPR_TIMER_END("poll", 0); - - if (r < 0) { - if (errno != EINTR) { - gpr_log(GPR_ERROR, "poll() failed: %s", strerror(errno)); - } - if (fd) { - fd_end_poll(exec_ctx, &fd_watcher, 0, 0, NULL); - } - } else if (r == 0) { - if (fd) { - fd_end_poll(exec_ctx, &fd_watcher, 0, 0, NULL); - } - } else { - if (pfd[0].revents & POLLIN_CHECK) { - grpc_wakeup_fd_consume_wakeup(&grpc_global_wakeup_fd); - } - if (pfd[1].revents & POLLIN_CHECK) { - grpc_wakeup_fd_consume_wakeup(&worker->wakeup_fd->fd); - } - if (nfds > 2) { - fd_end_poll(exec_ctx, &fd_watcher, pfd[2].revents & POLLIN_CHECK, - pfd[2].revents & POLLOUT_CHECK, pollset); - } else if (fd) { - fd_end_poll(exec_ctx, &fd_watcher, 0, 0, NULL); - } - } - - if (fd) { - GRPC_FD_UNREF(fd, "basicpoll_begin"); - } -} - -static void basic_pollset_destroy(grpc_pollset *pollset) { - if (pollset->data.ptr != NULL) { - GRPC_FD_UNREF(pollset->data.ptr, "basicpoll"); - pollset->data.ptr = NULL; - } -} - -static const grpc_pollset_vtable basic_pollset = { - basic_pollset_add_fd, basic_pollset_maybe_work_and_unlock, - basic_pollset_destroy, basic_pollset_destroy}; - -static void become_basic_pollset(grpc_pollset *pollset, grpc_fd *fd_or_null) { - pollset->vtable = &basic_pollset; - pollset->data.ptr = fd_or_null; - if (fd_or_null != NULL) { - GRPC_FD_REF(fd_or_null, "basicpoll"); - } -} - -/******************************************************************************* - * pollset_multipoller_with_poll_posix.c - */ - -#ifndef GPR_LINUX_MULTIPOLL_WITH_EPOLL - -typedef struct { - /* all polled fds */ - size_t fd_count; - size_t fd_capacity; - grpc_fd **fds; - /* fds that have been removed from the pollset explicitly */ - size_t del_count; - size_t del_capacity; - grpc_fd **dels; -} poll_hdr; - -static void multipoll_with_poll_pollset_add_fd(grpc_exec_ctx *exec_ctx, - grpc_pollset *pollset, - grpc_fd *fd, - int and_unlock_pollset) { - size_t i; - poll_hdr *h = pollset->data.ptr; - /* TODO(ctiller): this is O(num_fds^2); maybe switch to a hash set here */ - for (i = 0; i < h->fd_count; i++) { - if (h->fds[i] == fd) goto exit; - } - if (h->fd_count == h->fd_capacity) { - h->fd_capacity = GPR_MAX(h->fd_capacity + 8, h->fd_count * 3 / 2); - h->fds = gpr_realloc(h->fds, sizeof(grpc_fd *) * h->fd_capacity); - } - h->fds[h->fd_count++] = fd; - GRPC_FD_REF(fd, "multipoller"); -exit: - if (and_unlock_pollset) { - gpr_mu_unlock(&pollset->mu); - } -} - -static void multipoll_with_poll_pollset_maybe_work_and_unlock( - grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, grpc_pollset_worker *worker, - gpr_timespec deadline, gpr_timespec now) { -#define POLLOUT_CHECK (POLLOUT | POLLHUP | POLLERR) -#define POLLIN_CHECK (POLLIN | POLLHUP | POLLERR) - - int timeout; - int r; - size_t i, j, fd_count; - nfds_t pfd_count; - poll_hdr *h; - /* TODO(ctiller): inline some elements to avoid an allocation */ - grpc_fd_watcher *watchers; - struct pollfd *pfds; - - h = pollset->data.ptr; - timeout = poll_deadline_to_millis_timeout(deadline, now); - /* TODO(ctiller): perform just one malloc here if we exceed the inline case */ - pfds = gpr_malloc(sizeof(*pfds) * (h->fd_count + 2)); - watchers = gpr_malloc(sizeof(*watchers) * (h->fd_count + 2)); - fd_count = 0; - pfd_count = 2; - pfds[0].fd = GRPC_WAKEUP_FD_GET_READ_FD(&grpc_global_wakeup_fd); - pfds[0].events = POLLIN; - pfds[0].revents = 0; - pfds[1].fd = GRPC_WAKEUP_FD_GET_READ_FD(&worker->wakeup_fd->fd); - pfds[1].events = POLLIN; - pfds[1].revents = 0; - for (i = 0; i < h->fd_count; i++) { - int remove = fd_is_orphaned(h->fds[i]); - for (j = 0; !remove && j < h->del_count; j++) { - if (h->fds[i] == h->dels[j]) remove = 1; - } - if (remove) { - GRPC_FD_UNREF(h->fds[i], "multipoller"); - } else { - h->fds[fd_count++] = h->fds[i]; - watchers[pfd_count].fd = h->fds[i]; - GRPC_FD_REF(watchers[pfd_count].fd, "multipoller_start"); - pfds[pfd_count].fd = h->fds[i]->fd; - pfds[pfd_count].revents = 0; - pfd_count++; - } - } - for (j = 0; j < h->del_count; j++) { - GRPC_FD_UNREF(h->dels[j], "multipoller_del"); - } - h->del_count = 0; - h->fd_count = fd_count; - gpr_mu_unlock(&pollset->mu); - - for (i = 2; i < pfd_count; i++) { - grpc_fd *fd = watchers[i].fd; - pfds[i].events = (short)fd_begin_poll(fd, pollset, worker, POLLIN, POLLOUT, - &watchers[i]); - GRPC_FD_UNREF(fd, "multipoller_start"); - } - - /* TODO(vpai): Consider first doing a 0 timeout poll here to avoid - even going into the blocking annotation if possible */ - GRPC_SCHEDULING_START_BLOCKING_REGION; - r = grpc_poll_function(pfds, pfd_count, timeout); - GRPC_SCHEDULING_END_BLOCKING_REGION; - - if (r < 0) { - if (errno != EINTR) { - gpr_log(GPR_ERROR, "poll() failed: %s", strerror(errno)); - } - for (i = 2; i < pfd_count; i++) { - fd_end_poll(exec_ctx, &watchers[i], 0, 0, NULL); - } - } else if (r == 0) { - for (i = 2; i < pfd_count; i++) { - fd_end_poll(exec_ctx, &watchers[i], 0, 0, NULL); - } - } else { - if (pfds[0].revents & POLLIN_CHECK) { - grpc_wakeup_fd_consume_wakeup(&grpc_global_wakeup_fd); - } - if (pfds[1].revents & POLLIN_CHECK) { - grpc_wakeup_fd_consume_wakeup(&worker->wakeup_fd->fd); - } - for (i = 2; i < pfd_count; i++) { - if (watchers[i].fd == NULL) { - fd_end_poll(exec_ctx, &watchers[i], 0, 0, NULL); - continue; - } - fd_end_poll(exec_ctx, &watchers[i], pfds[i].revents & POLLIN_CHECK, - pfds[i].revents & POLLOUT_CHECK, pollset); - } - } - - gpr_free(pfds); - gpr_free(watchers); -} - -static void multipoll_with_poll_pollset_finish_shutdown(grpc_pollset *pollset) { - size_t i; - poll_hdr *h = pollset->data.ptr; - for (i = 0; i < h->fd_count; i++) { - GRPC_FD_UNREF(h->fds[i], "multipoller"); - } - for (i = 0; i < h->del_count; i++) { - GRPC_FD_UNREF(h->dels[i], "multipoller_del"); - } - h->fd_count = 0; - h->del_count = 0; -} - -static void multipoll_with_poll_pollset_destroy(grpc_pollset *pollset) { - poll_hdr *h = pollset->data.ptr; - multipoll_with_poll_pollset_finish_shutdown(pollset); - gpr_free(h->fds); - gpr_free(h->dels); - gpr_free(h); -} - -static const grpc_pollset_vtable multipoll_with_poll_pollset = { - multipoll_with_poll_pollset_add_fd, - multipoll_with_poll_pollset_maybe_work_and_unlock, - multipoll_with_poll_pollset_finish_shutdown, - multipoll_with_poll_pollset_destroy}; - -static void poll_become_multipoller(grpc_exec_ctx *exec_ctx, - grpc_pollset *pollset, grpc_fd **fds, - size_t nfds) { - size_t i; - poll_hdr *h = gpr_malloc(sizeof(poll_hdr)); - pollset->vtable = &multipoll_with_poll_pollset; - pollset->data.ptr = h; - h->fd_count = nfds; - h->fd_capacity = nfds; - h->fds = gpr_malloc(nfds * sizeof(grpc_fd *)); - h->del_count = 0; - h->del_capacity = 0; - h->dels = NULL; - for (i = 0; i < nfds; i++) { - h->fds[i] = fds[i]; - GRPC_FD_REF(fds[i], "multipoller"); - } -} - -#endif /* !GPR_LINUX_MULTIPOLL_WITH_EPOLL */ - -/******************************************************************************* - * pollset_multipoller_with_epoll_posix.c - */ - -#ifdef GPR_LINUX_MULTIPOLL_WITH_EPOLL - -#include -#include -#include -#include -#include - -#include -#include -#include - -#include "src/core/lib/iomgr/ev_posix.h" -#include "src/core/lib/profiling/timers.h" -#include "src/core/lib/support/block_annotate.h" - -static void set_ready(grpc_exec_ctx *exec_ctx, grpc_fd *fd, grpc_closure **st, - grpc_pollset *read_notifier_pollset) { - /* only one set_ready can be active at once (but there may be a racing - notify_on) */ - gpr_mu_lock(&fd->mu); - set_ready_locked(exec_ctx, fd, st); - - /* A non-NULL read_notifier_pollset means that the fd is readable. */ - if (read_notifier_pollset != NULL) { - /* Note: Since the fd might be a part of multiple pollsets, this might be - * called multiple times (for each time the fd becomes readable) and it is - * okay to set the fd's read-notifier pollset to anyone of these pollsets */ - set_read_notifier_pollset_locked(exec_ctx, fd, read_notifier_pollset); - } - - gpr_mu_unlock(&fd->mu); -} - -static void fd_become_readable(grpc_exec_ctx *exec_ctx, grpc_fd *fd, - grpc_pollset *notifier_pollset) { - set_ready(exec_ctx, fd, &fd->read_closure, notifier_pollset); -} - -static void fd_become_writable(grpc_exec_ctx *exec_ctx, grpc_fd *fd) { - set_ready(exec_ctx, fd, &fd->write_closure, NULL); -} - -struct epoll_fd_list { - int *epoll_fds; - size_t count; - size_t capacity; -}; - -static struct epoll_fd_list epoll_fd_global_list; -static gpr_once init_epoll_fd_list_mu = GPR_ONCE_INIT; -static gpr_mu epoll_fd_list_mu; - -static void init_mu(void) { gpr_mu_init(&epoll_fd_list_mu); } - -static void add_epoll_fd_to_global_list(int epoll_fd) { - gpr_once_init(&init_epoll_fd_list_mu, init_mu); - - gpr_mu_lock(&epoll_fd_list_mu); - if (epoll_fd_global_list.count == epoll_fd_global_list.capacity) { - epoll_fd_global_list.capacity = - GPR_MAX((size_t)8, epoll_fd_global_list.capacity * 2); - epoll_fd_global_list.epoll_fds = - gpr_realloc(epoll_fd_global_list.epoll_fds, - epoll_fd_global_list.capacity * sizeof(int)); - } - epoll_fd_global_list.epoll_fds[epoll_fd_global_list.count++] = epoll_fd; - gpr_mu_unlock(&epoll_fd_list_mu); -} - -static void remove_epoll_fd_from_global_list(int epoll_fd) { - gpr_mu_lock(&epoll_fd_list_mu); - GPR_ASSERT(epoll_fd_global_list.count > 0); - for (size_t i = 0; i < epoll_fd_global_list.count; i++) { - if (epoll_fd == epoll_fd_global_list.epoll_fds[i]) { - epoll_fd_global_list.epoll_fds[i] = - epoll_fd_global_list.epoll_fds[--(epoll_fd_global_list.count)]; - break; - } - } - gpr_mu_unlock(&epoll_fd_list_mu); -} - -static void remove_fd_from_all_epoll_sets(int fd) { - int err; - gpr_once_init(&init_epoll_fd_list_mu, init_mu); - gpr_mu_lock(&epoll_fd_list_mu); - if (epoll_fd_global_list.count == 0) { - gpr_mu_unlock(&epoll_fd_list_mu); - return; - } - for (size_t i = 0; i < epoll_fd_global_list.count; i++) { - err = epoll_ctl(epoll_fd_global_list.epoll_fds[i], EPOLL_CTL_DEL, fd, NULL); - if (err < 0 && errno != ENOENT) { - gpr_log(GPR_ERROR, "epoll_ctl del for %d failed: %s", fd, - strerror(errno)); - } - } - gpr_mu_unlock(&epoll_fd_list_mu); -} - -typedef struct { - grpc_pollset *pollset; - grpc_fd *fd; - grpc_closure closure; -} delayed_add; - -typedef struct { int epoll_fd; } epoll_hdr; - -static void finally_add_fd(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, - grpc_fd *fd) { - epoll_hdr *h = pollset->data.ptr; - struct epoll_event ev; - int err; - grpc_fd_watcher watcher; - - /* We pretend to be polling whilst adding an fd to keep the fd from being - closed during the add. This may result in a spurious wakeup being assigned - to this pollset whilst adding, but that should be benign. */ - GPR_ASSERT(fd_begin_poll(fd, pollset, NULL, 0, 0, &watcher) == 0); - if (watcher.fd != NULL) { - ev.events = (uint32_t)(EPOLLIN | EPOLLOUT | EPOLLET); - ev.data.ptr = fd; - err = epoll_ctl(h->epoll_fd, EPOLL_CTL_ADD, fd->fd, &ev); - if (err < 0) { - /* FDs may be added to a pollset multiple times, so EEXIST is normal. */ - if (errno != EEXIST) { - gpr_log(GPR_ERROR, "epoll_ctl add for %d failed: %s", fd->fd, - strerror(errno)); - } - } - } - fd_end_poll(exec_ctx, &watcher, 0, 0, NULL); -} - -static void perform_delayed_add(grpc_exec_ctx *exec_ctx, void *arg, - bool iomgr_status) { - delayed_add *da = arg; - - if (!fd_is_orphaned(da->fd)) { - finally_add_fd(exec_ctx, da->pollset, da->fd); - } - - gpr_mu_lock(&da->pollset->mu); - da->pollset->in_flight_cbs--; - if (da->pollset->shutting_down) { - /* We don't care about this pollset anymore. */ - if (da->pollset->in_flight_cbs == 0 && !da->pollset->called_shutdown) { - da->pollset->called_shutdown = 1; - grpc_exec_ctx_enqueue(exec_ctx, da->pollset->shutdown_done, true, NULL); - } - } - gpr_mu_unlock(&da->pollset->mu); - - GRPC_FD_UNREF(da->fd, "delayed_add"); - - gpr_free(da); -} - -static void multipoll_with_epoll_pollset_add_fd(grpc_exec_ctx *exec_ctx, - grpc_pollset *pollset, - grpc_fd *fd, - int and_unlock_pollset) { - if (and_unlock_pollset) { - gpr_mu_unlock(&pollset->mu); - finally_add_fd(exec_ctx, pollset, fd); - } else { - delayed_add *da = gpr_malloc(sizeof(*da)); - da->pollset = pollset; - da->fd = fd; - GRPC_FD_REF(fd, "delayed_add"); - grpc_closure_init(&da->closure, perform_delayed_add, da); - pollset->in_flight_cbs++; - grpc_exec_ctx_enqueue(exec_ctx, &da->closure, true, NULL); - } -} - -/* TODO(klempner): We probably want to turn this down a bit */ -#define GRPC_EPOLL_MAX_EVENTS 1000 - -static void multipoll_with_epoll_pollset_maybe_work_and_unlock( - grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, grpc_pollset_worker *worker, - gpr_timespec deadline, gpr_timespec now) { - struct epoll_event ep_ev[GRPC_EPOLL_MAX_EVENTS]; - int ep_rv; - int poll_rv; - epoll_hdr *h = pollset->data.ptr; - int timeout_ms; - struct pollfd pfds[2]; - - /* If you want to ignore epoll's ability to sanely handle parallel pollers, - * for a more apples-to-apples performance comparison with poll, add a - * if (pollset->counter != 0) { return 0; } - * here. - */ - - gpr_mu_unlock(&pollset->mu); - - timeout_ms = poll_deadline_to_millis_timeout(deadline, now); - - pfds[0].fd = GRPC_WAKEUP_FD_GET_READ_FD(&worker->wakeup_fd->fd); - pfds[0].events = POLLIN; - pfds[0].revents = 0; - pfds[1].fd = h->epoll_fd; - pfds[1].events = POLLIN; - pfds[1].revents = 0; - - /* TODO(vpai): Consider first doing a 0 timeout poll here to avoid - even going into the blocking annotation if possible */ - GPR_TIMER_BEGIN("poll", 0); - GRPC_SCHEDULING_START_BLOCKING_REGION; - poll_rv = grpc_poll_function(pfds, 2, timeout_ms); - GRPC_SCHEDULING_END_BLOCKING_REGION; - GPR_TIMER_END("poll", 0); - - if (poll_rv < 0) { - if (errno != EINTR) { - gpr_log(GPR_ERROR, "poll() failed: %s", strerror(errno)); - } - } else if (poll_rv == 0) { - /* do nothing */ - } else { - if (pfds[0].revents) { - grpc_wakeup_fd_consume_wakeup(&worker->wakeup_fd->fd); - } - if (pfds[1].revents) { - do { - /* The following epoll_wait never blocks; it has a timeout of 0 */ - ep_rv = epoll_wait(h->epoll_fd, ep_ev, GRPC_EPOLL_MAX_EVENTS, 0); - if (ep_rv < 0) { - if (errno != EINTR) { - gpr_log(GPR_ERROR, "epoll_wait() failed: %s", strerror(errno)); - } - } else { - int i; - for (i = 0; i < ep_rv; ++i) { - grpc_fd *fd = ep_ev[i].data.ptr; - /* TODO(klempner): We might want to consider making err and pri - * separate events */ - int cancel = ep_ev[i].events & (EPOLLERR | EPOLLHUP); - int read_ev = ep_ev[i].events & (EPOLLIN | EPOLLPRI); - int write_ev = ep_ev[i].events & EPOLLOUT; - if (fd == NULL) { - grpc_wakeup_fd_consume_wakeup(&grpc_global_wakeup_fd); - } else { - if (read_ev || cancel) { - fd_become_readable(exec_ctx, fd, pollset); - } - if (write_ev || cancel) { - fd_become_writable(exec_ctx, fd); - } - } - } - } - } while (ep_rv == GRPC_EPOLL_MAX_EVENTS); - } - } -} - -static void multipoll_with_epoll_pollset_finish_shutdown( - grpc_pollset *pollset) {} - -static void multipoll_with_epoll_pollset_destroy(grpc_pollset *pollset) { - epoll_hdr *h = pollset->data.ptr; - close(h->epoll_fd); - remove_epoll_fd_from_global_list(h->epoll_fd); - gpr_free(h); -} - -static const grpc_pollset_vtable multipoll_with_epoll_pollset = { - multipoll_with_epoll_pollset_add_fd, - multipoll_with_epoll_pollset_maybe_work_and_unlock, - multipoll_with_epoll_pollset_finish_shutdown, - multipoll_with_epoll_pollset_destroy}; - -static void epoll_become_multipoller(grpc_exec_ctx *exec_ctx, - grpc_pollset *pollset, grpc_fd **fds, - size_t nfds) { - size_t i; - epoll_hdr *h = gpr_malloc(sizeof(epoll_hdr)); - struct epoll_event ev; - int err; - - pollset->vtable = &multipoll_with_epoll_pollset; - pollset->data.ptr = h; - h->epoll_fd = epoll_create1(EPOLL_CLOEXEC); - if (h->epoll_fd < 0) { - /* TODO(klempner): Fall back to poll here, especially on ENOSYS */ - gpr_log(GPR_ERROR, "epoll_create1 failed: %s", strerror(errno)); - abort(); - } - add_epoll_fd_to_global_list(h->epoll_fd); - - ev.events = (uint32_t)(EPOLLIN | EPOLLET); - ev.data.ptr = NULL; - err = epoll_ctl(h->epoll_fd, EPOLL_CTL_ADD, - GRPC_WAKEUP_FD_GET_READ_FD(&grpc_global_wakeup_fd), &ev); - if (err < 0) { - gpr_log(GPR_ERROR, "epoll_ctl add for %d failed: %s", - GRPC_WAKEUP_FD_GET_READ_FD(&grpc_global_wakeup_fd), - strerror(errno)); - } - - for (i = 0; i < nfds; i++) { - multipoll_with_epoll_pollset_add_fd(exec_ctx, pollset, fds[i], 0); - } -} - -#else /* GPR_LINUX_MULTIPOLL_WITH_EPOLL */ - -static void remove_fd_from_all_epoll_sets(int fd) {} - -#endif /* GPR_LINUX_MULTIPOLL_WITH_EPOLL */ - -/******************************************************************************* - * pollset_set_posix.c - */ - -static grpc_pollset_set *pollset_set_create(void) { - grpc_pollset_set *pollset_set = gpr_malloc(sizeof(*pollset_set)); - memset(pollset_set, 0, sizeof(*pollset_set)); - gpr_mu_init(&pollset_set->mu); - return pollset_set; -} - -static void pollset_set_destroy(grpc_pollset_set *pollset_set) { - size_t i; - gpr_mu_destroy(&pollset_set->mu); - for (i = 0; i < pollset_set->fd_count; i++) { - GRPC_FD_UNREF(pollset_set->fds[i], "pollset_set"); - } - gpr_free(pollset_set->pollsets); - gpr_free(pollset_set->pollset_sets); - gpr_free(pollset_set->fds); - gpr_free(pollset_set); -} - -static void pollset_set_add_pollset(grpc_exec_ctx *exec_ctx, - grpc_pollset_set *pollset_set, - grpc_pollset *pollset) { - size_t i, j; - gpr_mu_lock(&pollset_set->mu); - if (pollset_set->pollset_count == pollset_set->pollset_capacity) { - pollset_set->pollset_capacity = - GPR_MAX(8, 2 * pollset_set->pollset_capacity); - pollset_set->pollsets = - gpr_realloc(pollset_set->pollsets, pollset_set->pollset_capacity * - sizeof(*pollset_set->pollsets)); - } - pollset_set->pollsets[pollset_set->pollset_count++] = pollset; - for (i = 0, j = 0; i < pollset_set->fd_count; i++) { - if (fd_is_orphaned(pollset_set->fds[i])) { - GRPC_FD_UNREF(pollset_set->fds[i], "pollset_set"); - } else { - pollset_add_fd(exec_ctx, pollset, pollset_set->fds[i]); - pollset_set->fds[j++] = pollset_set->fds[i]; - } - } - pollset_set->fd_count = j; - gpr_mu_unlock(&pollset_set->mu); -} - -static void pollset_set_del_pollset(grpc_exec_ctx *exec_ctx, - grpc_pollset_set *pollset_set, - grpc_pollset *pollset) { - size_t i; - gpr_mu_lock(&pollset_set->mu); - for (i = 0; i < pollset_set->pollset_count; i++) { - if (pollset_set->pollsets[i] == pollset) { - pollset_set->pollset_count--; - GPR_SWAP(grpc_pollset *, pollset_set->pollsets[i], - pollset_set->pollsets[pollset_set->pollset_count]); - break; - } - } - gpr_mu_unlock(&pollset_set->mu); -} - -static void pollset_set_add_pollset_set(grpc_exec_ctx *exec_ctx, - grpc_pollset_set *bag, - grpc_pollset_set *item) { - size_t i, j; - gpr_mu_lock(&bag->mu); - if (bag->pollset_set_count == bag->pollset_set_capacity) { - bag->pollset_set_capacity = GPR_MAX(8, 2 * bag->pollset_set_capacity); - bag->pollset_sets = - gpr_realloc(bag->pollset_sets, - bag->pollset_set_capacity * sizeof(*bag->pollset_sets)); - } - bag->pollset_sets[bag->pollset_set_count++] = item; - for (i = 0, j = 0; i < bag->fd_count; i++) { - if (fd_is_orphaned(bag->fds[i])) { - GRPC_FD_UNREF(bag->fds[i], "pollset_set"); - } else { - pollset_set_add_fd(exec_ctx, item, bag->fds[i]); - bag->fds[j++] = bag->fds[i]; - } - } - bag->fd_count = j; - gpr_mu_unlock(&bag->mu); -} - -static void pollset_set_del_pollset_set(grpc_exec_ctx *exec_ctx, - grpc_pollset_set *bag, - grpc_pollset_set *item) { - size_t i; - gpr_mu_lock(&bag->mu); - for (i = 0; i < bag->pollset_set_count; i++) { - if (bag->pollset_sets[i] == item) { - bag->pollset_set_count--; - GPR_SWAP(grpc_pollset_set *, bag->pollset_sets[i], - bag->pollset_sets[bag->pollset_set_count]); - break; - } - } - gpr_mu_unlock(&bag->mu); -} - -static void pollset_set_add_fd(grpc_exec_ctx *exec_ctx, - grpc_pollset_set *pollset_set, grpc_fd *fd) { - size_t i; - gpr_mu_lock(&pollset_set->mu); - if (pollset_set->fd_count == pollset_set->fd_capacity) { - pollset_set->fd_capacity = GPR_MAX(8, 2 * pollset_set->fd_capacity); - pollset_set->fds = gpr_realloc( - pollset_set->fds, pollset_set->fd_capacity * sizeof(*pollset_set->fds)); - } - GRPC_FD_REF(fd, "pollset_set"); - pollset_set->fds[pollset_set->fd_count++] = fd; - for (i = 0; i < pollset_set->pollset_count; i++) { - pollset_add_fd(exec_ctx, pollset_set->pollsets[i], fd); - } - for (i = 0; i < pollset_set->pollset_set_count; i++) { - pollset_set_add_fd(exec_ctx, pollset_set->pollset_sets[i], fd); - } - gpr_mu_unlock(&pollset_set->mu); -} - -static void pollset_set_del_fd(grpc_exec_ctx *exec_ctx, - grpc_pollset_set *pollset_set, grpc_fd *fd) { - size_t i; - gpr_mu_lock(&pollset_set->mu); - for (i = 0; i < pollset_set->fd_count; i++) { - if (pollset_set->fds[i] == fd) { - pollset_set->fd_count--; - GPR_SWAP(grpc_fd *, pollset_set->fds[i], - pollset_set->fds[pollset_set->fd_count]); - GRPC_FD_UNREF(fd, "pollset_set"); - break; - } - } - for (i = 0; i < pollset_set->pollset_set_count; i++) { - pollset_set_del_fd(exec_ctx, pollset_set->pollset_sets[i], fd); - } - gpr_mu_unlock(&pollset_set->mu); -} - -/******************************************************************************* - * event engine binding - */ - -static void shutdown_engine(void) { - fd_global_shutdown(); - pollset_global_shutdown(); -} - -static const grpc_event_engine_vtable vtable = { - .pollset_size = sizeof(grpc_pollset), - - .fd_create = fd_create, - .fd_wrapped_fd = fd_wrapped_fd, - .fd_orphan = fd_orphan, - .fd_shutdown = fd_shutdown, - .fd_notify_on_read = fd_notify_on_read, - .fd_notify_on_write = fd_notify_on_write, - .fd_get_read_notifier_pollset = fd_get_read_notifier_pollset, - - .pollset_init = pollset_init, - .pollset_shutdown = pollset_shutdown, - .pollset_reset = pollset_reset, - .pollset_destroy = pollset_destroy, - .pollset_work = pollset_work, - .pollset_kick = pollset_kick, - .pollset_add_fd = pollset_add_fd, - - .pollset_set_create = pollset_set_create, - .pollset_set_destroy = pollset_set_destroy, - .pollset_set_add_pollset = pollset_set_add_pollset, - .pollset_set_del_pollset = pollset_set_del_pollset, - .pollset_set_add_pollset_set = pollset_set_add_pollset_set, - .pollset_set_del_pollset_set = pollset_set_del_pollset_set, - .pollset_set_add_fd = pollset_set_add_fd, - .pollset_set_del_fd = pollset_set_del_fd, - - .kick_poller = kick_poller, - - .shutdown_engine = shutdown_engine, -}; - -const grpc_event_engine_vtable *grpc_init_poll_and_epoll_posix(void) { -#ifdef GPR_LINUX_MULTIPOLL_WITH_EPOLL - platform_become_multipoller = epoll_become_multipoller; -#else - platform_become_multipoller = poll_become_multipoller; -#endif - fd_global_init(); - pollset_global_init(); - return &vtable; -} - -#endif diff --git a/src/core/lib/iomgr/ev_poll_and_epoll_posix.h b/src/core/lib/iomgr/ev_poll_and_epoll_posix.h deleted file mode 100644 index 06d6dbf29dd..00000000000 --- a/src/core/lib/iomgr/ev_poll_and_epoll_posix.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * - * Copyright 2015, Google Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - */ - -#ifndef GRPC_CORE_LIB_IOMGR_EV_POLL_AND_EPOLL_POSIX_H -#define GRPC_CORE_LIB_IOMGR_EV_POLL_AND_EPOLL_POSIX_H - -#include "src/core/lib/iomgr/ev_posix.h" - -const grpc_event_engine_vtable *grpc_init_poll_and_epoll_posix(void); - -#endif /* GRPC_CORE_LIB_IOMGR_EV_POLL_AND_EPOLL_POSIX_H */ diff --git a/src/core/lib/iomgr/ev_poll_posix.c b/src/core/lib/iomgr/ev_poll_posix.c index fafb3b4b6f3..4d2ec5eb986 100644 --- a/src/core/lib/iomgr/ev_poll_posix.c +++ b/src/core/lib/iomgr/ev_poll_posix.c @@ -59,6 +59,8 @@ * FD declarations */ +grpc_wakeup_fd grpc_global_wakeup_fd; + typedef struct grpc_fd_watcher { struct grpc_fd_watcher *next; struct grpc_fd_watcher *prev; diff --git a/src/core/lib/iomgr/ev_posix.c b/src/core/lib/iomgr/ev_posix.c index 6477b05dcd8..95520b01d3a 100644 --- a/src/core/lib/iomgr/ev_posix.c +++ b/src/core/lib/iomgr/ev_posix.c @@ -44,7 +44,6 @@ #include #include -#include "src/core/lib/iomgr/ev_poll_and_epoll_posix.h" #include "src/core/lib/iomgr/ev_poll_posix.h" #include "src/core/lib/support/env.h" @@ -62,7 +61,7 @@ typedef struct { } event_engine_factory; static const event_engine_factory g_factories[] = { - {"poll", grpc_init_poll_posix}, {"legacy", grpc_init_poll_and_epoll_posix}, + {"poll", grpc_init_poll_posix}, }; static void add(const char *beg, const char *end, char ***ss, size_t *ns) { diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 162191b06db..aa79c8c2a89 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -94,7 +94,6 @@ CORE_SOURCE_FILES = [ 'src/core/lib/iomgr/endpoint.c', 'src/core/lib/iomgr/endpoint_pair_posix.c', 'src/core/lib/iomgr/endpoint_pair_windows.c', - 'src/core/lib/iomgr/ev_poll_and_epoll_posix.c', 'src/core/lib/iomgr/ev_poll_posix.c', 'src/core/lib/iomgr/ev_posix.c', 'src/core/lib/iomgr/exec_ctx.c', diff --git a/third_party/protobuf b/third_party/protobuf index 3470b6895aa..a1938b2aa9c 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit 3470b6895aa659b7559ed678e029a5338e535f14 +Subproject commit a1938b2aa9ca86ce7ce50c27ff9737c1008d2a03 diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 212dfc3160d..5afed4201a7 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -807,7 +807,6 @@ src/core/lib/http/parser.h \ src/core/lib/iomgr/closure.h \ src/core/lib/iomgr/endpoint.h \ src/core/lib/iomgr/endpoint_pair.h \ -src/core/lib/iomgr/ev_poll_and_epoll_posix.h \ src/core/lib/iomgr/ev_poll_posix.h \ src/core/lib/iomgr/ev_posix.h \ src/core/lib/iomgr/exec_ctx.h \ @@ -946,7 +945,6 @@ src/core/lib/iomgr/closure.c \ src/core/lib/iomgr/endpoint.c \ src/core/lib/iomgr/endpoint_pair_posix.c \ src/core/lib/iomgr/endpoint_pair_windows.c \ -src/core/lib/iomgr/ev_poll_and_epoll_posix.c \ src/core/lib/iomgr/ev_poll_posix.c \ src/core/lib/iomgr/ev_posix.c \ src/core/lib/iomgr/exec_ctx.c \ diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 5f0943b440f..0538dce4198 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -157,7 +157,7 @@ class CLanguage(object): 'windows': ['all'], 'mac': ['all'], 'posix': ['all'], - 'linux': ['poll'], # DISABLED DUE TO BUGS: 'legacy' + 'linux': ['poll'], } for target in binaries: polling_strategies = (POLLING_STRATEGIES[self.platform] diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 3866ebb0e55..64a49f5f766 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -5645,7 +5645,6 @@ "src/core/lib/iomgr/closure.h", "src/core/lib/iomgr/endpoint.h", "src/core/lib/iomgr/endpoint_pair.h", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.h", "src/core/lib/iomgr/ev_poll_posix.h", "src/core/lib/iomgr/ev_posix.h", "src/core/lib/iomgr/exec_ctx.h", @@ -5745,8 +5744,6 @@ "src/core/lib/iomgr/endpoint_pair.h", "src/core/lib/iomgr/endpoint_pair_posix.c", "src/core/lib/iomgr/endpoint_pair_windows.c", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.c", - "src/core/lib/iomgr/ev_poll_and_epoll_posix.h", "src/core/lib/iomgr/ev_poll_posix.c", "src/core/lib/iomgr/ev_poll_posix.h", "src/core/lib/iomgr/ev_posix.c", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index a20d386fa32..2dba1de384c 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -316,7 +316,6 @@ - @@ -475,8 +474,6 @@ - - diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index d5465176a28..1c789193705 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -55,9 +55,6 @@ src\core\lib\iomgr - - src\core\lib\iomgr - src\core\lib\iomgr @@ -653,9 +650,6 @@ src\core\lib\iomgr - - src\core\lib\iomgr - src\core\lib\iomgr diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index 09748f082c4..90ad80f2fc2 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -304,7 +304,6 @@ - @@ -450,8 +449,6 @@ - - diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index a85bfeefe6b..2b19c0fb341 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -58,9 +58,6 @@ src\core\lib\iomgr - - src\core\lib\iomgr - src\core\lib\iomgr @@ -575,9 +572,6 @@ src\core\lib\iomgr - - src\core\lib\iomgr - src\core\lib\iomgr From ae09d9dca9ac0f6d6c6e877e2935ad8cfba9da05 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 20 May 2016 22:23:37 -0700 Subject: [PATCH 16/24] Fixes and code simplification --- src/core/lib/iomgr/ev_poll_posix.c | 40 +++---------------------- src/core/lib/surface/server.c | 9 ++++++ test/cpp/end2end/hybrid_end2end_test.cc | 21 +++++++++++-- 3 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/core/lib/iomgr/ev_poll_posix.c b/src/core/lib/iomgr/ev_poll_posix.c index 4d2ec5eb986..4f64d31c971 100644 --- a/src/core/lib/iomgr/ev_poll_posix.c +++ b/src/core/lib/iomgr/ev_poll_posix.c @@ -183,7 +183,6 @@ struct grpc_pollset_worker { struct grpc_pollset { gpr_mu mu; grpc_pollset_worker root_worker; - int in_flight_cbs; int shutting_down; int called_shutdown; int kicked_without_pollers; @@ -193,10 +192,6 @@ struct grpc_pollset { size_t fd_count; size_t fd_capacity; grpc_fd **fds; - /* fds that have been removed from the pollset explicitly */ - size_t del_count; - size_t del_capacity; - grpc_fd **dels; /* Local cache of eventfds for workers */ grpc_cached_wakeup_fd *local_wakeup_cache; }; @@ -728,7 +723,6 @@ static void pollset_init(grpc_pollset *pollset, gpr_mu **mu) { gpr_mu_init(&pollset->mu); *mu = &pollset->mu; pollset->root_worker.next = pollset->root_worker.prev = &pollset->root_worker; - pollset->in_flight_cbs = 0; pollset->shutting_down = 0; pollset->called_shutdown = 0; pollset->kicked_without_pollers = 0; @@ -737,14 +731,10 @@ static void pollset_init(grpc_pollset *pollset, gpr_mu **mu) { pollset->kicked_without_pollers = 0; pollset->fd_count = 0; pollset->fd_capacity = 0; - pollset->del_count = 0; - pollset->del_capacity = 0; pollset->fds = NULL; - pollset->dels = NULL; } static void pollset_destroy(grpc_pollset *pollset) { - GPR_ASSERT(pollset->in_flight_cbs == 0); GPR_ASSERT(!pollset_has_workers(pollset)); GPR_ASSERT(pollset->idle_jobs.head == pollset->idle_jobs.tail); while (pollset->local_wakeup_cache) { @@ -754,17 +744,14 @@ static void pollset_destroy(grpc_pollset *pollset) { pollset->local_wakeup_cache = next; } gpr_free(pollset->fds); - gpr_free(pollset->dels); gpr_mu_destroy(&pollset->mu); } static void pollset_reset(grpc_pollset *pollset) { GPR_ASSERT(pollset->shutting_down); - GPR_ASSERT(pollset->in_flight_cbs == 0); GPR_ASSERT(!pollset_has_workers(pollset)); GPR_ASSERT(pollset->idle_jobs.head == pollset->idle_jobs.tail); GPR_ASSERT(pollset->fd_count == 0); - GPR_ASSERT(pollset->del_count == 0); pollset->shutting_down = 0; pollset->called_shutdown = 0; pollset->kicked_without_pollers = 0; @@ -797,11 +784,7 @@ static void finish_shutdown(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset) { for (i = 0; i < pollset->fd_count; i++) { GRPC_FD_UNREF(pollset->fds[i], "multipoller"); } - for (i = 0; i < pollset->del_count; i++) { - GRPC_FD_UNREF(pollset->dels[i], "multipoller_del"); - } pollset->fd_count = 0; - pollset->del_count = 0; grpc_exec_ctx_enqueue(exec_ctx, pollset->shutdown_done, true, NULL); } @@ -841,13 +824,6 @@ static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, GPR_TIMER_MARK("pollset_work.shutting_down", 0); goto done; } - /* Give do_promote priority so we don't starve it out */ - if (pollset->in_flight_cbs) { - GPR_TIMER_MARK("pollset_work.in_flight_cbs", 0); - gpr_mu_unlock(&pollset->mu); - locked = 0; - goto done; - } /* Start polling, and keep doing so while we're being asked to re-evaluate our pollers (this allows poll() based pollers to ensure they don't miss wakeups) */ @@ -867,7 +843,7 @@ static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, int timeout; int r; - size_t i, j, fd_count; + size_t i, fd_count; nfds_t pfd_count; /* TODO(ctiller): inline some elements to avoid an allocation */ grpc_fd_watcher *watchers; @@ -887,11 +863,7 @@ static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, pfds[1].events = POLLIN; pfds[1].revents = 0; for (i = 0; i < pollset->fd_count; i++) { - int remove = fd_is_orphaned(pollset->fds[i]); - for (j = 0; !remove && j < pollset->del_count; j++) { - if (pollset->fds[i] == pollset->dels[j]) remove = 1; - } - if (remove) { + if (fd_is_orphaned(pollset->fds[i])) { GRPC_FD_UNREF(pollset->fds[i], "multipoller"); } else { pollset->fds[fd_count++] = pollset->fds[i]; @@ -902,10 +874,6 @@ static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, pfd_count++; } } - for (j = 0; j < pollset->del_count; j++) { - GRPC_FD_UNREF(pollset->dels[j], "multipoller_del"); - } - pollset->del_count = 0; pollset->fd_count = fd_count; gpr_mu_unlock(&pollset->mu); @@ -997,7 +965,7 @@ static void pollset_work(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, if (pollset->shutting_down) { if (pollset_has_workers(pollset)) { pollset_kick(pollset, NULL); - } else if (!pollset->called_shutdown && pollset->in_flight_cbs == 0) { + } else if (!pollset->called_shutdown) { pollset->called_shutdown = 1; gpr_mu_unlock(&pollset->mu); finish_shutdown(exec_ctx, pollset); @@ -1027,7 +995,7 @@ static void pollset_shutdown(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, if (!pollset_has_workers(pollset)) { grpc_exec_ctx_enqueue_list(exec_ctx, &pollset->idle_jobs, NULL); } - if (!pollset->called_shutdown && pollset->in_flight_cbs == 0 && + if (!pollset->called_shutdown && !pollset_has_workers(pollset)) { pollset->called_shutdown = 1; finish_shutdown(exec_ctx, pollset); diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 54b76d8aa5b..165e20a0622 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -527,6 +527,8 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { if (request_id == -1) { continue; } else { + gpr_log(GPR_DEBUG, "queue lockfree, retries=%d chose=%d", i, cq_idx); + gpr_mu_lock(&calld->mu_state); calld->state = ACTIVATED; gpr_mu_unlock(&calld->mu_state); @@ -537,6 +539,7 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { } /* no cq to take the request found: queue it on the slow list */ + gpr_log(GPR_DEBUG, "queue slowpath"); gpr_mu_lock(&server->mu_call); gpr_mu_lock(&calld->mu_state); calld->state = PENDING; @@ -1298,12 +1301,14 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, server->requested_calls[request_id] = *rc; gpr_free(rc); if (gpr_stack_lockfree_push(rm->requests_per_cq[cq_idx], request_id)) { + gpr_log(GPR_DEBUG, "request against empty"); /* this was the first queued request: we need to lock and start matching calls */ gpr_mu_lock(&server->mu_call); while ((calld = rm->pending_head) != NULL) { request_id = gpr_stack_lockfree_pop(rm->requests_per_cq[cq_idx]); if (request_id == -1) break; + gpr_log(GPR_DEBUG, "drain1"); rm->pending_head = calld->pending_next; gpr_mu_unlock(&server->mu_call); gpr_mu_lock(&calld->mu_state); @@ -1324,6 +1329,8 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, gpr_mu_lock(&server->mu_call); } gpr_mu_unlock(&server->mu_call); + } else { + gpr_log(GPR_DEBUG, "request lockfree"); } return GRPC_CALL_OK; } @@ -1377,6 +1384,7 @@ grpc_call_error grpc_server_request_registered_call( grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; requested_call *rc = gpr_malloc(sizeof(*rc)); registered_method *rm = rmp; + gpr_log(GPR_DEBUG, "method: %s", rm->method); GRPC_API_TRACE( "grpc_server_request_registered_call(" "server=%p, rmp=%p, call=%p, deadline=%p, initial_metadata=%p, " @@ -1391,6 +1399,7 @@ grpc_call_error grpc_server_request_registered_call( break; } } + gpr_log(GPR_DEBUG, "cq_idx=%d, cq_count=%d", cq_idx, server->cq_count); if (cq_idx == server->cq_count) { gpr_free(rc); error = GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE; diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index 208e7d589fa..b4270070e2f 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -217,7 +217,7 @@ class HybridEnd2endTest : public ::testing::Test { } // Create a separate cq for each potential handler. for (int i = 0; i < 5; i++) { - cqs_.push_back(builder.AddCompletionQueue(i < num_cqs_frequently_polled)); + cqs_.push_back(builder.AddCompletionQueue(i == num_cqs_frequently_polled - 1)); } server_ = builder.BuildAndStart(); } @@ -253,6 +253,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest send_request; EchoResponse recv_response; ClientContext cli_ctx; + cli_ctx.set_fail_fast(false); send_request.set_message("Hello"); Status recv_status = stub_->Echo(&cli_ctx, send_request, &recv_response); EXPECT_EQ(send_request.message(), recv_response.message()); @@ -266,6 +267,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest send_request; EchoResponse recv_response; ClientContext cli_ctx; + cli_ctx.set_fail_fast(false); send_request.set_message("Hello"); Status recv_status = stub->Echo(&cli_ctx, send_request, &recv_response); EXPECT_EQ(send_request.message() + "_dup", recv_response.message()); @@ -277,6 +279,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoResponse recv_response; grpc::string expected_message; ClientContext cli_ctx; + cli_ctx.set_fail_fast(false); send_request.set_message("Hello"); auto stream = stub_->RequestStream(&cli_ctx, &recv_response); for (int i = 0; i < 5; i++) { @@ -293,6 +296,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest request; EchoResponse response; ClientContext context; + context.set_fail_fast(false); request.set_message("hello"); auto stream = stub_->ResponseStream(&context, request); @@ -312,6 +316,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest request; EchoResponse response; ClientContext context; + context.set_fail_fast(false); grpc::string msg("hello"); auto stream = stub_->BidiStream(&context); @@ -505,12 +510,22 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStreamResponseStream) { SetUpServer(&service, nullptr, &generic_service, 3); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { + gpr_log(GPR_DEBUG, "t0 start"); HandleGenericCall(&generic_service, cqs_[0].get()); + gpr_log(GPR_DEBUG, "t0 done"); }); std::thread request_stream_handler_thread( - [this, &service] { HandleClientStreaming(&service, cqs_[1].get()); }); + [this, &service] { + gpr_log(GPR_DEBUG, "t1 start"); + HandleClientStreaming(&service, cqs_[1].get()); + gpr_log(GPR_DEBUG, "t1 done"); + }); std::thread response_stream_handler_thread( - [this, &service] { HandleServerStreaming(&service, cqs_[2].get()); }); + [this, &service] { + gpr_log(GPR_DEBUG, "t2 start"); + HandleServerStreaming(&service, cqs_[2].get()); + gpr_log(GPR_DEBUG, "t2 done"); + }); TestAllMethods(); generic_handler_thread.join(); request_stream_handler_thread.join(); From 509b30e7396b617693a3c93f4c2fd4ec417a96a1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 12:32:39 -0700 Subject: [PATCH 17/24] Fix non-listening cq registration so that calls can be queued against them --- src/core/lib/surface/server.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 165e20a0622..b1d8b575a72 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -933,16 +933,15 @@ static void register_completion_queue(grpc_server *server, grpc_cq_mark_server_cq(cq); - /* Non-listening completion queues are not added to server->cqs */ if (is_non_listening) { grpc_cq_mark_non_listening_server_cq(cq); - } else { - GRPC_CQ_INTERNAL_REF(cq, "server"); - n = server->cq_count++; - server->cqs = gpr_realloc( - server->cqs, server->cq_count * sizeof(grpc_completion_queue *)); - server->cqs[n] = cq; } + + GRPC_CQ_INTERNAL_REF(cq, "server"); + n = server->cq_count++; + server->cqs = gpr_realloc(server->cqs, + server->cq_count * sizeof(grpc_completion_queue *)); + server->cqs[n] = cq; } void grpc_server_register_completion_queue(grpc_server *server, @@ -1049,9 +1048,12 @@ void grpc_server_start(grpc_server *server) { GRPC_API_TRACE("grpc_server_start(server=%p)", 1, (server)); server->started = true; + size_t pollset_count = 0; server->pollsets = gpr_malloc(sizeof(grpc_pollset *) * server->cq_count); for (i = 0; i < server->cq_count; i++) { - server->pollsets[i] = grpc_cq_pollset(server->cqs[i]); + if (!grpc_cq_is_non_listening_server_cq(server->cqs[i])) { + server->pollsets[pollset_count++] = grpc_cq_pollset(server->cqs[i]); + } } request_matcher_init(&server->unregistered_request_matcher, server->max_requested_calls, server); @@ -1061,7 +1063,7 @@ void grpc_server_start(grpc_server *server) { } for (l = server->listeners; l; l = l->next) { - l->start(&exec_ctx, server, l->arg, server->pollsets, server->cq_count); + l->start(&exec_ctx, server, l->arg, server->pollsets, pollset_count); } grpc_exec_ctx_finish(&exec_ctx); From 3f3312e7e92892c6625feecded6fbf09815689f0 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 12:35:29 -0700 Subject: [PATCH 18/24] Remove spam --- src/core/lib/surface/server.c | 9 --------- test/cpp/end2end/hybrid_end2end_test.cc | 17 ++++------------- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index b1d8b575a72..6be65f70336 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -527,8 +527,6 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { if (request_id == -1) { continue; } else { - gpr_log(GPR_DEBUG, "queue lockfree, retries=%d chose=%d", i, cq_idx); - gpr_mu_lock(&calld->mu_state); calld->state = ACTIVATED; gpr_mu_unlock(&calld->mu_state); @@ -539,7 +537,6 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { } /* no cq to take the request found: queue it on the slow list */ - gpr_log(GPR_DEBUG, "queue slowpath"); gpr_mu_lock(&server->mu_call); gpr_mu_lock(&calld->mu_state); calld->state = PENDING; @@ -1303,14 +1300,12 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, server->requested_calls[request_id] = *rc; gpr_free(rc); if (gpr_stack_lockfree_push(rm->requests_per_cq[cq_idx], request_id)) { - gpr_log(GPR_DEBUG, "request against empty"); /* this was the first queued request: we need to lock and start matching calls */ gpr_mu_lock(&server->mu_call); while ((calld = rm->pending_head) != NULL) { request_id = gpr_stack_lockfree_pop(rm->requests_per_cq[cq_idx]); if (request_id == -1) break; - gpr_log(GPR_DEBUG, "drain1"); rm->pending_head = calld->pending_next; gpr_mu_unlock(&server->mu_call); gpr_mu_lock(&calld->mu_state); @@ -1331,8 +1326,6 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, gpr_mu_lock(&server->mu_call); } gpr_mu_unlock(&server->mu_call); - } else { - gpr_log(GPR_DEBUG, "request lockfree"); } return GRPC_CALL_OK; } @@ -1386,7 +1379,6 @@ grpc_call_error grpc_server_request_registered_call( grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; requested_call *rc = gpr_malloc(sizeof(*rc)); registered_method *rm = rmp; - gpr_log(GPR_DEBUG, "method: %s", rm->method); GRPC_API_TRACE( "grpc_server_request_registered_call(" "server=%p, rmp=%p, call=%p, deadline=%p, initial_metadata=%p, " @@ -1401,7 +1393,6 @@ grpc_call_error grpc_server_request_registered_call( break; } } - gpr_log(GPR_DEBUG, "cq_idx=%d, cq_count=%d", cq_idx, server->cq_count); if (cq_idx == server->cq_count) { gpr_free(rc); error = GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE; diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index b4270070e2f..a19fccbb6ba 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -217,7 +217,8 @@ class HybridEnd2endTest : public ::testing::Test { } // Create a separate cq for each potential handler. for (int i = 0; i < 5; i++) { - cqs_.push_back(builder.AddCompletionQueue(i == num_cqs_frequently_polled - 1)); + cqs_.push_back( + builder.AddCompletionQueue(i == num_cqs_frequently_polled - 1)); } server_ = builder.BuildAndStart(); } @@ -510,22 +511,12 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStreamResponseStream) { SetUpServer(&service, nullptr, &generic_service, 3); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { - gpr_log(GPR_DEBUG, "t0 start"); HandleGenericCall(&generic_service, cqs_[0].get()); - gpr_log(GPR_DEBUG, "t0 done"); }); std::thread request_stream_handler_thread( - [this, &service] { - gpr_log(GPR_DEBUG, "t1 start"); - HandleClientStreaming(&service, cqs_[1].get()); - gpr_log(GPR_DEBUG, "t1 done"); - }); + [this, &service] { HandleClientStreaming(&service, cqs_[1].get()); }); std::thread response_stream_handler_thread( - [this, &service] { - gpr_log(GPR_DEBUG, "t2 start"); - HandleServerStreaming(&service, cqs_[2].get()); - gpr_log(GPR_DEBUG, "t2 done"); - }); + [this, &service] { HandleServerStreaming(&service, cqs_[2].get()); }); TestAllMethods(); generic_handler_thread.join(); request_stream_handler_thread.join(); From fa96d86a99137fc5a3581413c752603ffa731b93 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 12:39:56 -0700 Subject: [PATCH 19/24] Fix comments --- src/core/lib/surface/server.c | 4 ++-- src/cpp/server/server_builder.cc | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 6be65f70336..505b5019683 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -175,7 +175,7 @@ struct registered_method { char *host; grpc_server_register_method_payload_handling payload_handling; uint32_t flags; - /* one request matcher per method per cq */ + /* one request matcher per method */ request_matcher request_matcher; registered_method *next; }; @@ -204,7 +204,7 @@ struct grpc_server { gpr_mu mu_call; /* mutex for call-specific state */ registered_method *registered_methods; - /** one request matcher for unregistered methods per cq */ + /** one request matcher for unregistered methods */ request_matcher unregistered_request_matcher; /** free list of available requested_calls indices */ gpr_stack_lockfree *request_freelist; diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 5966e548b09..54feac39825 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -119,6 +119,7 @@ std::unique_ptr ServerBuilder::BuildAndStart() { for (auto plugin = plugins_.begin(); plugin != plugins_.end(); plugin++) { if ((*plugin).second->has_sync_methods()) { thread_pool.reset(CreateDefaultThreadPool()); + has_sync_methods = true; break; } } From 4265fa1e66b72c2ccf2e3c5fecc4b2012b0637c3 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 12:40:53 -0700 Subject: [PATCH 20/24] clang-format --- src/core/lib/iomgr/ev_poll_posix.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/lib/iomgr/ev_poll_posix.c b/src/core/lib/iomgr/ev_poll_posix.c index 4f64d31c971..e2a21230b9f 100644 --- a/src/core/lib/iomgr/ev_poll_posix.c +++ b/src/core/lib/iomgr/ev_poll_posix.c @@ -995,8 +995,7 @@ static void pollset_shutdown(grpc_exec_ctx *exec_ctx, grpc_pollset *pollset, if (!pollset_has_workers(pollset)) { grpc_exec_ctx_enqueue_list(exec_ctx, &pollset->idle_jobs, NULL); } - if (!pollset->called_shutdown && - !pollset_has_workers(pollset)) { + if (!pollset->called_shutdown && !pollset_has_workers(pollset)) { pollset->called_shutdown = 1; finish_shutdown(exec_ctx, pollset); } From e76528ce267e06024224ad52d2874384df26d0a1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 12:43:17 -0700 Subject: [PATCH 21/24] Revert "Remove spam" This reverts commit 3f3312e7e92892c6625feecded6fbf09815689f0. --- src/core/lib/surface/server.c | 9 +++++++++ test/cpp/end2end/hybrid_end2end_test.cc | 17 +++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 505b5019683..7a1f3a2e54a 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -527,6 +527,8 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { if (request_id == -1) { continue; } else { + gpr_log(GPR_DEBUG, "queue lockfree, retries=%d chose=%d", i, cq_idx); + gpr_mu_lock(&calld->mu_state); calld->state = ACTIVATED; gpr_mu_unlock(&calld->mu_state); @@ -537,6 +539,7 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { } /* no cq to take the request found: queue it on the slow list */ + gpr_log(GPR_DEBUG, "queue slowpath"); gpr_mu_lock(&server->mu_call); gpr_mu_lock(&calld->mu_state); calld->state = PENDING; @@ -1300,12 +1303,14 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, server->requested_calls[request_id] = *rc; gpr_free(rc); if (gpr_stack_lockfree_push(rm->requests_per_cq[cq_idx], request_id)) { + gpr_log(GPR_DEBUG, "request against empty"); /* this was the first queued request: we need to lock and start matching calls */ gpr_mu_lock(&server->mu_call); while ((calld = rm->pending_head) != NULL) { request_id = gpr_stack_lockfree_pop(rm->requests_per_cq[cq_idx]); if (request_id == -1) break; + gpr_log(GPR_DEBUG, "drain1"); rm->pending_head = calld->pending_next; gpr_mu_unlock(&server->mu_call); gpr_mu_lock(&calld->mu_state); @@ -1326,6 +1331,8 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, gpr_mu_lock(&server->mu_call); } gpr_mu_unlock(&server->mu_call); + } else { + gpr_log(GPR_DEBUG, "request lockfree"); } return GRPC_CALL_OK; } @@ -1379,6 +1386,7 @@ grpc_call_error grpc_server_request_registered_call( grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; requested_call *rc = gpr_malloc(sizeof(*rc)); registered_method *rm = rmp; + gpr_log(GPR_DEBUG, "method: %s", rm->method); GRPC_API_TRACE( "grpc_server_request_registered_call(" "server=%p, rmp=%p, call=%p, deadline=%p, initial_metadata=%p, " @@ -1393,6 +1401,7 @@ grpc_call_error grpc_server_request_registered_call( break; } } + gpr_log(GPR_DEBUG, "cq_idx=%d, cq_count=%d", cq_idx, server->cq_count); if (cq_idx == server->cq_count) { gpr_free(rc); error = GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE; diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index a19fccbb6ba..b4270070e2f 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -217,8 +217,7 @@ class HybridEnd2endTest : public ::testing::Test { } // Create a separate cq for each potential handler. for (int i = 0; i < 5; i++) { - cqs_.push_back( - builder.AddCompletionQueue(i == num_cqs_frequently_polled - 1)); + cqs_.push_back(builder.AddCompletionQueue(i == num_cqs_frequently_polled - 1)); } server_ = builder.BuildAndStart(); } @@ -511,12 +510,22 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStreamResponseStream) { SetUpServer(&service, nullptr, &generic_service, 3); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { + gpr_log(GPR_DEBUG, "t0 start"); HandleGenericCall(&generic_service, cqs_[0].get()); + gpr_log(GPR_DEBUG, "t0 done"); }); std::thread request_stream_handler_thread( - [this, &service] { HandleClientStreaming(&service, cqs_[1].get()); }); + [this, &service] { + gpr_log(GPR_DEBUG, "t1 start"); + HandleClientStreaming(&service, cqs_[1].get()); + gpr_log(GPR_DEBUG, "t1 done"); + }); std::thread response_stream_handler_thread( - [this, &service] { HandleServerStreaming(&service, cqs_[2].get()); }); + [this, &service] { + gpr_log(GPR_DEBUG, "t2 start"); + HandleServerStreaming(&service, cqs_[2].get()); + gpr_log(GPR_DEBUG, "t2 done"); + }); TestAllMethods(); generic_handler_thread.join(); request_stream_handler_thread.join(); From 34c6e87598dd2c8c68f25793a56bb5d6ffae679d Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 13:05:01 -0700 Subject: [PATCH 22/24] Simpler trick to force a listening cq --- test/cpp/end2end/hybrid_end2end_test.cc | 33 ++++++++++++++----------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index b4270070e2f..38c6ba9c94a 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -199,8 +199,7 @@ class HybridEnd2endTest : public ::testing::Test { HybridEnd2endTest() {} void SetUpServer(::grpc::Service* service1, ::grpc::Service* service2, - AsyncGenericService* generic_service, - int num_cqs_frequently_polled) { + AsyncGenericService* generic_service) { int port = grpc_pick_unused_port_or_die(); server_address_ << "localhost:" << port; @@ -208,6 +207,9 @@ class HybridEnd2endTest : public ::testing::Test { ServerBuilder builder; builder.AddListeningPort(server_address_.str(), grpc::InsecureServerCredentials()); + // Always add a sync unimplemented service: we rely on having at least one + // synchronous method to get a listening cq + builder.RegisterService(&unimplemented_service_); builder.RegisterService(service1); if (service2) { builder.RegisterService(service2); @@ -217,7 +219,7 @@ class HybridEnd2endTest : public ::testing::Test { } // Create a separate cq for each potential handler. for (int i = 0; i < 5; i++) { - cqs_.push_back(builder.AddCompletionQueue(i == num_cqs_frequently_polled - 1)); + cqs_.push_back(builder.AddCompletionQueue(false)); } server_ = builder.BuildAndStart(); } @@ -344,6 +346,7 @@ class HybridEnd2endTest : public ::testing::Test { EXPECT_TRUE(s.ok()); } + grpc::testing::UnimplementedService::Service unimplemented_service_; std::vector > cqs_; std::unique_ptr stub_; std::unique_ptr server_; @@ -352,7 +355,7 @@ class HybridEnd2endTest : public ::testing::Test { TEST_F(HybridEnd2endTest, AsyncEcho) { EchoTestService::WithAsyncMethod_Echo service; - SetUpServer(&service, nullptr, nullptr, 1); + SetUpServer(&service, nullptr, nullptr); ResetStub(); std::thread echo_handler_thread( [this, &service] { HandleEcho(&service, cqs_[0].get(), false); }); @@ -364,7 +367,7 @@ TEST_F(HybridEnd2endTest, AsyncEchoRequestStream) { EchoTestService::WithAsyncMethod_RequestStream< EchoTestService::WithAsyncMethod_Echo > service; - SetUpServer(&service, nullptr, nullptr, 2); + SetUpServer(&service, nullptr, nullptr); ResetStub(); std::thread echo_handler_thread( [this, &service] { HandleEcho(&service, cqs_[0].get(), false); }); @@ -379,7 +382,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream) { EchoTestService::WithAsyncMethod_RequestStream< EchoTestService::WithAsyncMethod_ResponseStream > service; - SetUpServer(&service, nullptr, nullptr, 2); + SetUpServer(&service, nullptr, nullptr); ResetStub(); std::thread response_stream_handler_thread( [this, &service] { HandleServerStreaming(&service, cqs_[0].get()); }); @@ -396,7 +399,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream_SyncDupService) { EchoTestService::WithAsyncMethod_ResponseStream > service; TestServiceImplDupPkg dup_service; - SetUpServer(&service, &dup_service, nullptr, 2); + SetUpServer(&service, &dup_service, nullptr); ResetStub(); std::thread response_stream_handler_thread( [this, &service] { HandleServerStreaming(&service, cqs_[0].get()); }); @@ -414,7 +417,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream_AsyncDupService) { EchoTestService::WithAsyncMethod_ResponseStream > service; duplicate::EchoTestService::AsyncService dup_service; - SetUpServer(&service, &dup_service, nullptr, 3); + SetUpServer(&service, &dup_service, nullptr); ResetStub(); std::thread response_stream_handler_thread( [this, &service] { HandleServerStreaming(&service, cqs_[0].get()); }); @@ -432,7 +435,7 @@ TEST_F(HybridEnd2endTest, AsyncRequestStreamResponseStream_AsyncDupService) { TEST_F(HybridEnd2endTest, GenericEcho) { EchoTestService::WithGenericMethod_Echo service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service, 1); + SetUpServer(&service, nullptr, &generic_service); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -446,7 +449,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStream) { EchoTestService::WithGenericMethod_Echo > service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service, 2); + SetUpServer(&service, nullptr, &generic_service); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -465,7 +468,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStream_SyncDupService) { service; AsyncGenericService generic_service; TestServiceImplDupPkg dup_service; - SetUpServer(&service, &dup_service, &generic_service, 2); + SetUpServer(&service, &dup_service, &generic_service); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -485,7 +488,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStream_AsyncDupService) { service; AsyncGenericService generic_service; duplicate::EchoTestService::AsyncService dup_service; - SetUpServer(&service, &dup_service, &generic_service, 3); + SetUpServer(&service, &dup_service, &generic_service); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -507,7 +510,7 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStreamResponseStream) { EchoTestService::WithAsyncMethod_ResponseStream > > service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service, 3); + SetUpServer(&service, nullptr, &generic_service); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { gpr_log(GPR_DEBUG, "t0 start"); @@ -538,7 +541,7 @@ TEST_F(HybridEnd2endTest, GenericEchoRequestStreamAsyncResponseStream) { EchoTestService::WithAsyncMethod_ResponseStream > > service; AsyncGenericService generic_service; - SetUpServer(&service, nullptr, &generic_service, 3); + SetUpServer(&service, nullptr, &generic_service); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { HandleGenericCall(&generic_service, cqs_[0].get()); @@ -561,7 +564,7 @@ TEST_F(HybridEnd2endTest, GenericMethodWithoutGenericService) { EchoTestService::WithGenericMethod_Echo< EchoTestService::WithAsyncMethod_ResponseStream > > service; - SetUpServer(&service, nullptr, nullptr, 0); + SetUpServer(&service, nullptr, nullptr); EXPECT_EQ(nullptr, server_.get()); } From bc7593de7a58fdf5b3e8d59fee40edfaa75785f4 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 13:05:44 -0700 Subject: [PATCH 23/24] Revert "Revert "Remove spam"" This reverts commit e76528ce267e06024224ad52d2874384df26d0a1. --- src/core/lib/surface/server.c | 9 --------- test/cpp/end2end/hybrid_end2end_test.cc | 14 ++------------ 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 7a1f3a2e54a..505b5019683 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -527,8 +527,6 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { if (request_id == -1) { continue; } else { - gpr_log(GPR_DEBUG, "queue lockfree, retries=%d chose=%d", i, cq_idx); - gpr_mu_lock(&calld->mu_state); calld->state = ACTIVATED; gpr_mu_unlock(&calld->mu_state); @@ -539,7 +537,6 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, bool success) { } /* no cq to take the request found: queue it on the slow list */ - gpr_log(GPR_DEBUG, "queue slowpath"); gpr_mu_lock(&server->mu_call); gpr_mu_lock(&calld->mu_state); calld->state = PENDING; @@ -1303,14 +1300,12 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, server->requested_calls[request_id] = *rc; gpr_free(rc); if (gpr_stack_lockfree_push(rm->requests_per_cq[cq_idx], request_id)) { - gpr_log(GPR_DEBUG, "request against empty"); /* this was the first queued request: we need to lock and start matching calls */ gpr_mu_lock(&server->mu_call); while ((calld = rm->pending_head) != NULL) { request_id = gpr_stack_lockfree_pop(rm->requests_per_cq[cq_idx]); if (request_id == -1) break; - gpr_log(GPR_DEBUG, "drain1"); rm->pending_head = calld->pending_next; gpr_mu_unlock(&server->mu_call); gpr_mu_lock(&calld->mu_state); @@ -1331,8 +1326,6 @@ static grpc_call_error queue_call_request(grpc_exec_ctx *exec_ctx, gpr_mu_lock(&server->mu_call); } gpr_mu_unlock(&server->mu_call); - } else { - gpr_log(GPR_DEBUG, "request lockfree"); } return GRPC_CALL_OK; } @@ -1386,7 +1379,6 @@ grpc_call_error grpc_server_request_registered_call( grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; requested_call *rc = gpr_malloc(sizeof(*rc)); registered_method *rm = rmp; - gpr_log(GPR_DEBUG, "method: %s", rm->method); GRPC_API_TRACE( "grpc_server_request_registered_call(" "server=%p, rmp=%p, call=%p, deadline=%p, initial_metadata=%p, " @@ -1401,7 +1393,6 @@ grpc_call_error grpc_server_request_registered_call( break; } } - gpr_log(GPR_DEBUG, "cq_idx=%d, cq_count=%d", cq_idx, server->cq_count); if (cq_idx == server->cq_count) { gpr_free(rc); error = GRPC_CALL_ERROR_NOT_SERVER_COMPLETION_QUEUE; diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index 38c6ba9c94a..2c05db345b4 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -513,22 +513,12 @@ TEST_F(HybridEnd2endTest, GenericEchoAsyncRequestStreamResponseStream) { SetUpServer(&service, nullptr, &generic_service); ResetStub(); std::thread generic_handler_thread([this, &generic_service] { - gpr_log(GPR_DEBUG, "t0 start"); HandleGenericCall(&generic_service, cqs_[0].get()); - gpr_log(GPR_DEBUG, "t0 done"); }); std::thread request_stream_handler_thread( - [this, &service] { - gpr_log(GPR_DEBUG, "t1 start"); - HandleClientStreaming(&service, cqs_[1].get()); - gpr_log(GPR_DEBUG, "t1 done"); - }); + [this, &service] { HandleClientStreaming(&service, cqs_[1].get()); }); std::thread response_stream_handler_thread( - [this, &service] { - gpr_log(GPR_DEBUG, "t2 start"); - HandleServerStreaming(&service, cqs_[2].get()); - gpr_log(GPR_DEBUG, "t2 done"); - }); + [this, &service] { HandleServerStreaming(&service, cqs_[2].get()); }); TestAllMethods(); generic_handler_thread.join(); request_stream_handler_thread.join(); From 5f045384119eb07e84f93acb617ad77d0ad762a5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Sat, 21 May 2016 19:59:27 -0700 Subject: [PATCH 24/24] Fix protobuf --- third_party/protobuf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third_party/protobuf b/third_party/protobuf index a1938b2aa9c..3470b6895aa 160000 --- a/third_party/protobuf +++ b/third_party/protobuf @@ -1 +1 @@ -Subproject commit a1938b2aa9ca86ce7ce50c27ff9737c1008d2a03 +Subproject commit 3470b6895aa659b7559ed678e029a5338e535f14