From 264879fec50a699c8c93fead66bcb4a6fe4a2bd5 Mon Sep 17 00:00:00 2001
From: Noah Eisen <ncteisen@google.com>
Date: Tue, 20 Jun 2017 17:14:47 -0700
Subject: [PATCH] Overhaul the new pollers

---
 src/core/lib/iomgr/ev_epoll1_linux.c          |  6 +-
 .../iomgr/ev_epoll_limited_pollers_linux.c    | 65 +++++++++++--------
 .../lib/iomgr/ev_epoll_thread_pool_linux.c    | 28 ++++----
 src/core/lib/iomgr/ev_epollex_linux.c         | 32 +++++----
 src/core/lib/iomgr/ev_epollsig_linux.c        |  8 +--
 5 files changed, 80 insertions(+), 59 deletions(-)

diff --git a/src/core/lib/iomgr/ev_epoll1_linux.c b/src/core/lib/iomgr/ev_epoll1_linux.c
index e0c05457e3..66ba601adb 100644
--- a/src/core/lib/iomgr/ev_epoll1_linux.c
+++ b/src/core/lib/iomgr/ev_epoll1_linux.c
@@ -194,8 +194,10 @@ static grpc_fd *fd_create(int fd, const char *name) {
   char *fd_name;
   gpr_asprintf(&fd_name, "%s fd=%d", name, fd);
   grpc_iomgr_register_object(&new_fd->iomgr_object, fd_name);
-#ifdef GRPC_FD_REF_COUNT_DEBUG
-  gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, (void *)new_fd, fd_name);
+#ifndef NDEBUG
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, new_fd, fd_name);
+  }
 #endif
   gpr_free(fd_name);
 
diff --git a/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c b/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c
index 761e2ed5fe..2c91ad357c 100644
--- a/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c
+++ b/src/core/lib/iomgr/ev_epoll_limited_pollers_linux.c
@@ -147,8 +147,7 @@ struct grpc_fd {
 };
 
 /* Reference counting for fds */
-// #define GRPC_FD_REF_COUNT_DEBUG
-#ifdef GRPC_FD_REF_COUNT_DEBUG
+#ifndef NDEBUG
 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);
@@ -168,20 +167,18 @@ static void fd_global_shutdown(void);
  * Polling island Declarations
  */
 
-//#define PI_REFCOUNT_DEBUG
-
-#ifdef PI_REFCOUNT_DEBUG
+#ifndef NDEBUG
 
 #define PI_ADD_REF(p, r) pi_add_ref_dbg((p), (r), __FILE__, __LINE__)
 #define PI_UNREF(exec_ctx, p, r) \
   pi_unref_dbg((exec_ctx), (p), (r), __FILE__, __LINE__)
 
-#else /* defined(PI_REFCOUNT_DEBUG) */
+#else
 
 #define PI_ADD_REF(p, r) pi_add_ref((p))
 #define PI_UNREF(exec_ctx, p, r) pi_unref((exec_ctx), (p))
 
-#endif /* !defined(GRPC_PI_REF_COUNT_DEBUG) */
+#endif
 
 typedef struct worker_node {
   struct worker_node *next;
@@ -313,21 +310,27 @@ gpr_atm g_epoll_sync;
 static void pi_add_ref(polling_island *pi);
 static void pi_unref(grpc_exec_ctx *exec_ctx, polling_island *pi);
 
-#ifdef PI_REFCOUNT_DEBUG
+#ifndef NDEBUG
 static void pi_add_ref_dbg(polling_island *pi, const char *reason,
                            const char *file, int line) {
-  long old_cnt = gpr_atm_acq_load(&pi->ref_count);
+  if (GRPC_TRACER_ON(grpc_polling_trace)) {
+    gpr_atm old_cnt = gpr_atm_acq_load(&pi->ref_count);
+    gpr_log(GPR_DEBUG, "Add ref pi: %p, old:%" PRIdPTR " -> new:%" PRIdPTR
+                       " (%s) - (%s, %d)",
+            pi, old_cnt, old_cnt + 1, reason, file, line);
+  }
   pi_add_ref(pi);
-  gpr_log(GPR_DEBUG, "Add ref pi: %p, old: %ld -> new:%ld (%s) - (%s, %d)",
-          (void *)pi, old_cnt, old_cnt + 1, reason, file, line);
 }
 
 static void pi_unref_dbg(grpc_exec_ctx *exec_ctx, polling_island *pi,
                          const char *reason, const char *file, int line) {
-  long old_cnt = gpr_atm_acq_load(&pi->ref_count);
+  if (GRPC_TRACER_ON(grpc_polling_trace)) {
+    gpr_atm old_cnt = gpr_atm_acq_load(&pi->ref_count);
+    gpr_log(GPR_DEBUG, "Unref pi: %p, old:%" PRIdPTR " -> new:%" PRIdPTR
+                       " (%s) - (%s, %d)",
+            pi, old_cnt, (old_cnt - 1), reason, file, line);
+  }
   pi_unref(exec_ctx, pi);
-  gpr_log(GPR_DEBUG, "Unref pi: %p, old:%ld -> new:%ld (%s) - (%s, %d)",
-          (void *)pi, old_cnt, (old_cnt - 1), reason, file, line);
 }
 #endif
 
@@ -792,14 +795,17 @@ static void polling_island_global_shutdown() {
 static grpc_fd *fd_freelist = NULL;
 static gpr_mu fd_freelist_mu;
 
-#ifdef GRPC_FD_REF_COUNT_DEBUG
+#ifndef NDEBUG
 #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 %ld -> %ld [%s; %s:%d]", fd->fd,
-          (void *)fd, n, gpr_atm_no_barrier_load(&fd->refst),
-          gpr_atm_no_barrier_load(&fd->refst) + n, reason, file, line);
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG,
+            "FD %d %p   ref %d %" PRIdPTR " -> %" PRIdPTR " [%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)
@@ -808,18 +814,19 @@ static void ref_by(grpc_fd *fd, int n) {
   GPR_ASSERT(gpr_atm_no_barrier_fetch_add(&fd->refst, n) > 0);
 }
 
-#ifdef GRPC_FD_REF_COUNT_DEBUG
+#ifndef NDEBUG
 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 %ld -> %ld [%s; %s:%d]", fd->fd,
-          (void *)fd, n, gpr_atm_no_barrier_load(&fd->refst),
-          gpr_atm_no_barrier_load(&fd->refst) - n, reason, file, line);
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG,
+            "FD %d %p unref %d %" PRIdPTR " -> %" PRIdPTR " [%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);
+  gpr_atm old = gpr_atm_full_fetch_add(&fd->refst, -n);
   if (old == n) {
     /* Add the fd to the freelist */
     gpr_mu_lock(&fd_freelist_mu);
@@ -837,7 +844,7 @@ static void unref_by(grpc_fd *fd, int n) {
 }
 
 /* Increment refcount by two to avoid changing the orphan bit */
-#ifdef GRPC_FD_REF_COUNT_DEBUG
+#ifndef NDEBUG
 static void fd_ref(grpc_fd *fd, const char *reason, const char *file,
                    int line) {
   ref_by(fd, 2, reason, file, line);
@@ -905,8 +912,10 @@ static grpc_fd *fd_create(int fd, const char *name) {
   char *fd_name;
   gpr_asprintf(&fd_name, "%s fd=%d", name, fd);
   grpc_iomgr_register_object(&new_fd->iomgr_object, fd_name);
-#ifdef GRPC_FD_REF_COUNT_DEBUG
-  gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, (void *)new_fd, fd_name);
+#ifndef NDEBUG
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, new_fd, fd_name);
+  }
 #endif
   gpr_free(fd_name);
   return new_fd;
diff --git a/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c b/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c
index 0ab3594818..49be72c03e 100644
--- a/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c
+++ b/src/core/lib/iomgr/ev_epoll_thread_pool_linux.c
@@ -93,20 +93,18 @@ static void fd_global_shutdown(void);
  * epoll set Declarations
  */
 
-//#define EPS_REFCOUNT_DEBUG
-
-#ifdef EPS_REFCOUNT_DEBUG
+#ifndef NDEBUG
 
 #define EPS_ADD_REF(p, r) eps_add_ref_dbg((p), (r), __FILE__, __LINE__)
 #define EPS_UNREF(exec_ctx, p, r) \
   eps_unref_dbg((exec_ctx), (p), (r), __FILE__, __LINE__)
 
-#else /* defined(EPS_REFCOUNT_DEBUG) */
+#else
 
 #define EPS_ADD_REF(p, r) eps_add_ref((p))
 #define EPS_UNREF(exec_ctx, p, r) eps_unref((exec_ctx), (p))
 
-#endif /* !defined(GRPC_EPS_REF_COUNT_DEBUG) */
+#endif
 
 typedef struct epoll_set {
   /* Mutex poller should acquire to poll this. This enforces that only one
@@ -226,21 +224,27 @@ gpr_atm g_epoll_sync;
 static void eps_add_ref(epoll_set *eps);
 static void eps_unref(grpc_exec_ctx *exec_ctx, epoll_set *eps);
 
-#ifdef EPS_REFCOUNT_DEBUG
+#ifndef NDEBUG
 static void eps_add_ref_dbg(epoll_set *eps, const char *reason,
                             const char *file, int line) {
-  long old_cnt = gpr_atm_acq_load(&eps->ref_count);
+  if (GRPC_TRACER_ON(grpc_polling_trace)) {
+    gpr_atm old_cnt = gpr_atm_acq_load(&eps->ref_count);
+    gpr_log(GPR_DEBUG, "Add ref eps: %p, old:%" PRIdPTR " -> new:%" PRIdPTR
+                       " (%s) - (%s, %d)",
+            eps, old_cnt, old_cnt + 1, reason, file, line);
+  }
   eps_add_ref(eps);
-  gpr_log(GPR_DEBUG, "Add ref eps: %p, old: %ld -> new:%ld (%s) - (%s, %d)",
-          (void *)eps, old_cnt, old_cnt + 1, reason, file, line);
 }
 
 static void eps_unref_dbg(grpc_exec_ctx *exec_ctx, epoll_set *eps,
                           const char *reason, const char *file, int line) {
-  long old_cnt = gpr_atm_acq_load(&eps->ref_count);
+  if (GRPC_TRACER_ON(grpc_polling_trace)) {
+    gpr_atm old_cnt = gpr_atm_acq_load(&eps->ref_count);
+    gpr_log(GPR_DEBUG, "Unref eps: %p, old:%" PRIdPTR " -> new:%" PRIdPTR
+                       " (%s) - (%s, %d)",
+            eps, old_cnt, (old_cnt - 1), reason, file, line);
+  }
   eps_unref(exec_ctx, eps);
-  gpr_log(GPR_DEBUG, "Unref eps: %p, old:%ld -> new:%ld (%s) - (%s, %d)",
-          (void *)eps, old_cnt, (old_cnt - 1), reason, file, line);
 }
 #endif
 
diff --git a/src/core/lib/iomgr/ev_epollex_linux.c b/src/core/lib/iomgr/ev_epollex_linux.c
index 949f8a845d..5574838187 100644
--- a/src/core/lib/iomgr/ev_epollex_linux.c
+++ b/src/core/lib/iomgr/ev_epollex_linux.c
@@ -231,15 +231,18 @@ static bool append_error(grpc_error **composite, grpc_error *error,
 static grpc_fd *fd_freelist = NULL;
 static gpr_mu fd_freelist_mu;
 
-#ifdef GRPC_FD_REF_COUNT_DEBUG
+#ifndef NDEBUG
 #define REF_BY(fd, n, reason) ref_by(fd, n, reason, __FILE__, __LINE__)
 #define UNREF_BY(ec, fd, n, reason) \
   unref_by(ec, 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 %ld -> %ld [%s; %s:%d]", fd->fd,
-          (void *)fd, n, gpr_atm_no_barrier_load(&fd->refst),
-          gpr_atm_no_barrier_load(&fd->refst) + n, reason, file, line);
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG,
+            "FD %d %p   ref %d %" PRIdPTR " -> %" PRIdPTR " [%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(ec, fd, n, reason) unref_by(ec, fd, n)
@@ -264,18 +267,19 @@ static void fd_destroy(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
   gpr_mu_unlock(&fd_freelist_mu);
 }
 
-#ifdef GRPC_FD_REF_COUNT_DEBUG
+#ifndef NDEBUG
 static void unref_by(grpc_exec_ctx *exec_ctx, 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 %ld -> %ld [%s; %s:%d]", fd->fd,
-          (void *)fd, n, gpr_atm_no_barrier_load(&fd->refst),
-          gpr_atm_no_barrier_load(&fd->refst) - n, reason, file, line);
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG,
+            "FD %d %p unref %d %" PRIdPTR " -> %" PRIdPTR " [%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_exec_ctx *exec_ctx, grpc_fd *fd, int n) {
-  gpr_atm old;
 #endif
-  old = gpr_atm_full_fetch_add(&fd->refst, -n);
+  gpr_atm old = gpr_atm_full_fetch_add(&fd->refst, -n);
   if (old == n) {
     GRPC_CLOSURE_SCHED(exec_ctx, GRPC_CLOSURE_CREATE(fd_destroy, fd,
                                                      grpc_schedule_on_exec_ctx),
@@ -328,8 +332,10 @@ static grpc_fd *fd_create(int fd, const char *name) {
   char *fd_name;
   gpr_asprintf(&fd_name, "%s fd=%d", name, fd);
   grpc_iomgr_register_object(&new_fd->iomgr_object, fd_name);
-#ifdef GRPC_FD_REF_COUNT_DEBUG
-  gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, (void *)new_fd, fd_name);
+#ifndef NDEBUG
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, new_fd, fd_name);
+  }
 #endif
   gpr_free(fd_name);
   return new_fd;
diff --git a/src/core/lib/iomgr/ev_epollsig_linux.c b/src/core/lib/iomgr/ev_epollsig_linux.c
index 7406036ca6..17fef01e6e 100644
--- a/src/core/lib/iomgr/ev_epollsig_linux.c
+++ b/src/core/lib/iomgr/ev_epollsig_linux.c
@@ -140,7 +140,6 @@ struct grpc_fd {
 };
 
 /* Reference counting for fds */
-// #define GRPC_FD_REF_COUNT_DEBUG
 #ifndef NDEBUG
 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,
@@ -755,8 +754,7 @@ static void unref_by(grpc_fd *fd, int n, const char *reason, const char *file,
 #else
 static void unref_by(grpc_fd *fd, int n) {
 #endif
-  gpr_atm old;
-  old = gpr_atm_full_fetch_add(&fd->refst, -n);
+  gpr_atm old = gpr_atm_full_fetch_add(&fd->refst, -n);
   if (old == n) {
     /* Add the fd to the freelist */
     gpr_mu_lock(&fd_freelist_mu);
@@ -843,7 +841,9 @@ static grpc_fd *fd_create(int fd, const char *name) {
   gpr_asprintf(&fd_name, "%s fd=%d", name, fd);
   grpc_iomgr_register_object(&new_fd->iomgr_object, fd_name);
 #ifndef NDEBUG
-  gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, new_fd, fd_name);
+  if (GRPC_TRACER_ON(grpc_trace_fd_refcount)) {
+    gpr_log(GPR_DEBUG, "FD %d %p create %s", fd, new_fd, fd_name);
+  }
 #endif
   gpr_free(fd_name);
   return new_fd;
-- 
GitLab