Merge pull request #3476 from vjpai/block_annotate

Annotate blocking points

BUILD

@@ -44,6 +44,7 @@ package(default_visibility = ["//visibility:public"])
 cc_library(
   name = "gpr",
   srcs = [
+    "src/core/support/block_annotate.h",
     "src/core/support/env.h",
     "src/core/support/file.h",
     "src/core/support/murmur_hash.h",
@@ -1023,6 +1024,7 @@ objc_library(
     "include/grpc/support/tls_msvc.h",
     "include/grpc/support/tls_pthread.h",
     "include/grpc/support/useful.h",
+    "src/core/support/block_annotate.h",
     "src/core/support/env.h",
     "src/core/support/file.h",
     "src/core/support/murmur_hash.h",

build.yaml

@@ -152,9 +152,9 @@ libs:
     include/grpc/support/sync_posix.h, include/grpc/support/sync_win32.h, include/grpc/support/thd.h,
     include/grpc/support/time.h, include/grpc/support/tls.h, include/grpc/support/tls_gcc.h,
     include/grpc/support/tls_msvc.h, include/grpc/support/tls_pthread.h, include/grpc/support/useful.h]
-  headers: [src/core/support/env.h, src/core/support/file.h, src/core/support/murmur_hash.h,
-    src/core/support/stack_lockfree.h, src/core/support/string.h, src/core/support/string_win32.h,
-    src/core/support/thd_internal.h, src/core/support/time_precise.h]
+  headers: [src/core/support/block_annotate.h, src/core/support/env.h, src/core/support/file.h,
+    src/core/support/murmur_hash.h, src/core/support/stack_lockfree.h, src/core/support/string.h,
+    src/core/support/string_win32.h, src/core/support/thd_internal.h, src/core/support/time_precise.h]
   src: [src/core/support/alloc.c, src/core/support/cmdline.c, src/core/support/cpu_iphone.c,
     src/core/support/cpu_linux.c, src/core/support/cpu_posix.c, src/core/support/cpu_windows.c,
     src/core/support/env_linux.c, src/core/support/env_posix.c, src/core/support/env_win32.c,

gRPC.podspec

@@ -61,7 +61,8 @@ Pod::Spec.new do |s|
 
   # Core cross-platform gRPC library, written in C.
   s.subspec 'C-Core' do |ss|
-    ss.source_files = 'src/core/support/env.h',
+    ss.source_files = 'src/core/support/block_annotate.h',
+                      'src/core/support/env.h',
                       'src/core/support/file.h',
                       'src/core/support/murmur_hash.h',
                       'src/core/support/stack_lockfree.h',
@@ -414,7 +415,8 @@ Pod::Spec.new do |s|
                       'src/core/census/operation.c',
                       'src/core/census/tracing.c'
 
-    ss.private_header_files = 'src/core/support/env.h',
+    ss.private_header_files = 'src/core/support/block_annotate.h',
+                              'src/core/support/env.h',
                               'src/core/support/file.h',
                               'src/core/support/murmur_hash.h',
                               'src/core/support/stack_lockfree.h',

src/core/iomgr/pollset_multipoller_with_epoll.c

@@ -42,6 +42,7 @@
 #include <unistd.h>
 
 #include "src/core/iomgr/fd_posix.h"
+#include "src/core/support/block_annotate.h"
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 
@@ -179,7 +180,9 @@ static void multipoll_with_epoll_pollset_maybe_work_and_unlock(
   pfds[1].events = POLLIN;
   pfds[1].revents = 0;
 
+  GRPC_SCHEDULING_START_BLOCKING_REGION;
   poll_rv = grpc_poll_function(pfds, 2, timeout_ms);
+  GRPC_SCHEDULING_END_BLOCKING_REGION;
 
   if (poll_rv < 0) {
     if (errno != EINTR) {
@@ -193,6 +196,7 @@ static void multipoll_with_epoll_pollset_maybe_work_and_unlock(
     }
     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) {

src/core/iomgr/pollset_multipoller_with_poll_posix.c

@@ -44,6 +44,7 @@
 
 #include "src/core/iomgr/fd_posix.h"
 #include "src/core/iomgr/iomgr_internal.h"
+#include "src/core/support/block_annotate.h"
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/useful.h>
@@ -147,7 +148,9 @@ static void multipoll_with_poll_pollset_maybe_work_and_unlock(
                                                POLLOUT, &watchers[i]);
   }
 
+  GRPC_SCHEDULING_START_BLOCKING_REGION;
   r = grpc_poll_function(pfds, pfd_count, timeout);
+  GRPC_SCHEDULING_END_BLOCKING_REGION;
 
   for (i = 1; i < pfd_count; i++) {
     grpc_fd_end_poll(exec_ctx, &watchers[i], pfds[i].revents & POLLIN,

src/core/iomgr/pollset_posix.c

@@ -47,6 +47,7 @@
 #include "src/core/iomgr/iomgr_internal.h"
 #include "src/core/iomgr/socket_utils_posix.h"
 #include "src/core/profiling/timers.h"
+#include "src/core/support/block_annotate.h"
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 #include <grpc/support/thd.h>
@@ -468,7 +469,9 @@ static void basic_pollset_maybe_work_and_unlock(grpc_exec_ctx *exec_ctx,
 
   /* poll fd count (argument 2) is shortened by one if we have no events
      to poll on - such that it only includes the kicker */
+  GRPC_SCHEDULING_START_BLOCKING_REGION;
   r = grpc_poll_function(pfd, nfds, timeout);
+  GRPC_SCHEDULING_END_BLOCKING_REGION;
   GRPC_TIMER_MARK(GRPC_PTAG_POLL_FINISHED, r);
 
   if (fd) {

src/core/iomgr/resolve_address_posix.c

@@ -43,6 +43,7 @@
 
 #include "src/core/iomgr/iomgr_internal.h"
 #include "src/core/iomgr/sockaddr_utils.h"
+#include "src/core/support/block_annotate.h"
 #include "src/core/support/string.h"
 #include <grpc/support/alloc.h>
 #include <grpc/support/host_port.h>
@@ -103,13 +104,18 @@ grpc_resolved_addresses *grpc_blocking_resolve_address(
   hints.ai_socktype = SOCK_STREAM; /* stream socket */
   hints.ai_flags = AI_PASSIVE;     /* for wildcard IP address */
 
+  GRPC_SCHEDULING_START_BLOCKING_REGION;
   s = getaddrinfo(host, port, &hints, &result);
+  GRPC_SCHEDULING_END_BLOCKING_REGION;
+
   if (s != 0) {
     /* Retry if well-known service name is recognized */
     char *svc[][2] = {{"http", "80"}, {"https", "443"}};
     for (i = 0; i < GPR_ARRAY_SIZE(svc); i++) {
       if (strcmp(port, svc[i][0]) == 0) {
+        GRPC_SCHEDULING_START_BLOCKING_REGION;
         s = getaddrinfo(host, svc[i][1], &hints, &result);
+        GRPC_SCHEDULING_END_BLOCKING_REGION;
         break;
       }
     }

src/core/iomgr/resolve_address_windows.c

@@ -42,6 +42,7 @@
 
 #include "src/core/iomgr/iomgr_internal.h"
 #include "src/core/iomgr/sockaddr_utils.h"
+#include "src/core/support/block_annotate.h"
 #include "src/core/support/string.h"
 #include <grpc/support/alloc.h>
 #include <grpc/support/host_port.h>
@@ -88,7 +89,9 @@ grpc_resolved_addresses *grpc_blocking_resolve_address(
   hints.ai_socktype = SOCK_STREAM; /* stream socket */
   hints.ai_flags = AI_PASSIVE;     /* for wildcard IP address */
 
+  GRPC_SCHEDULING_START_BLOCKING_REGION;
   s = getaddrinfo(host, port, &hints, &result);
+  GRPC_SCHEDULING_END_BLOCKING_REGION;
   if (s != 0) {
     gpr_log(GPR_ERROR, "getaddrinfo: %s", gai_strerror(s));
     goto done;

src/core/support/block_annotate.h

@@ -0,0 +1,44 @@
+/*
+ *
+ * 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_INTERNAL_CORE_SUPPORT_BLOCK_ANNOTATE_H
+#define GRPC_INTERNAL_CORE_SUPPORT_BLOCK_ANNOTATE_H
+
+/* These annotations identify the beginning and end of regions where
+   the code may block for reasons other than synchronization functions.
+   These include poll, epoll, and getaddrinfo. */
+
+#define GRPC_SCHEDULING_START_BLOCKING_REGION do {} while (0)
+#define GRPC_SCHEDULING_END_BLOCKING_REGION do {} while (0)
+
+#endif /* GRPC_INTERNAL_CORE_SUPPORT_BLOCK_ANNOTATE_H */

src/core/support/file.c

@@ -40,6 +40,7 @@
 #include <grpc/support/log.h>
 #include <grpc/support/string_util.h>
 
+#include "src/core/support/block_annotate.h"
 #include "src/core/support/string.h"
 
 gpr_slice gpr_load_file(const char *filename, int add_null_terminator,
@@ -48,9 +49,11 @@ gpr_slice gpr_load_file(const char *filename, int add_null_terminator,
   size_t contents_size = 0;
   char *error_msg = NULL;
   gpr_slice result = gpr_empty_slice();
-  FILE *file = fopen(filename, "rb");
+  FILE *file;
   size_t bytes_read = 0;
 
+  GRPC_SCHEDULING_START_BLOCKING_REGION;
+  file = fopen(filename, "rb");
   if (file == NULL) {
     gpr_asprintf(&error_msg, "Could not open file %s (error = %s).", filename,
                  strerror(errno));
@@ -83,5 +86,6 @@ end:
     if (success != NULL) *success = 0;
   }
   if (file != NULL) fclose(file);
+  GRPC_SCHEDULING_END_BLOCKING_REGION;
   return result;
 }

src/core/support/time_posix.c

@@ -41,6 +41,7 @@
 #include <unistd.h>
 #include <grpc/support/log.h>
 #include <grpc/support/time.h>
+#include "src/core/support/block_annotate.h"
 
 static struct timespec timespec_from_gpr(gpr_timespec gts) {
   struct timespec rv;
@@ -126,6 +127,7 @@ void gpr_sleep_until(gpr_timespec until) {
   gpr_timespec now;
   gpr_timespec delta;
   struct timespec delta_ts;
+  int ns_result;
 
   for (;;) {
     /* We could simplify by using clock_nanosleep instead, but it might be
@@ -137,7 +139,10 @@ void gpr_sleep_until(gpr_timespec until) {
 
     delta = gpr_time_sub(until, now);
     delta_ts = timespec_from_gpr(delta);
-    if (nanosleep(&delta_ts, NULL) == 0) {
+    GRPC_SCHEDULING_START_BLOCKING_REGION;
+    ns_result = nanosleep(&delta_ts, NULL);
+    GRPC_SCHEDULING_END_BLOCKING_REGION;
+    if (ns_result == 0) {
       break;
     }
   }

src/core/support/time_win32.c

@@ -41,6 +41,8 @@
 #include <src/core/support/time_precise.h>
 #include <sys/timeb.h>
 
+#include "src/core/support/block_annotate.h"
+
 static LARGE_INTEGER g_start_time;
 static double g_time_scale;
 
@@ -92,7 +94,9 @@ void gpr_sleep_until(gpr_timespec until) {
     delta = gpr_time_sub(until, now);
     sleep_millis =
         (DWORD)delta.tv_sec * GPR_MS_PER_SEC + delta.tv_nsec / GPR_NS_PER_MS;
+    GRPC_SCHEDULING_START_BLOCKING_REGION;
     Sleep(sleep_millis);
+    GRPC_SCHEDULING_END_BLOCKING_REGION;
   }
 }
 

test/cpp/end2end/streaming_throughput_test.cc

@@ -103,6 +103,7 @@ class TestServiceImpl : public ::grpc::cpp::test::util::TestService::Service {
     EchoResponse response;
     response.set_message(kLargeString);
     while (!should_exit->load()) {
+      // TODO(vpai): Decide if the below requires blocking annotation
       std::this_thread::sleep_for(std::chrono::milliseconds(1));
       stream->Write(response);
     }
@@ -117,6 +118,7 @@ class TestServiceImpl : public ::grpc::cpp::test::util::TestService::Service {
     std::thread sender(std::bind(&TestServiceImpl::BidiStream_Sender, stream, &should_exit));
 
     while (stream->Read(&request)) {
+      // TODO(vpai): Decide if the below requires blocking annotation
       std::this_thread::sleep_for(std::chrono::milliseconds(3));
     }
     should_exit.store(true);

tools/doxygen/Doxyfile.core.internal

@@ -1067,6 +1067,7 @@ include/grpc/support/tls_gcc.h \
 include/grpc/support/tls_msvc.h \
 include/grpc/support/tls_pthread.h \
 include/grpc/support/useful.h \
+src/core/support/block_annotate.h \
 src/core/support/env.h \
 src/core/support/file.h \
 src/core/support/murmur_hash.h \

tools/run_tests/sources_and_headers.json

@@ -12161,6 +12161,7 @@
       "include/grpc/support/tls_msvc.h", 
       "include/grpc/support/tls_pthread.h", 
       "include/grpc/support/useful.h", 
+      "src/core/support/block_annotate.h", 
       "src/core/support/env.h", 
       "src/core/support/file.h", 
       "src/core/support/murmur_hash.h", 
@@ -12201,6 +12202,7 @@
       "include/grpc/support/tls_pthread.h", 
       "include/grpc/support/useful.h", 
       "src/core/support/alloc.c", 
+      "src/core/support/block_annotate.h", 
       "src/core/support/cmdline.c", 
       "src/core/support/cpu_iphone.c", 
       "src/core/support/cpu_linux.c", 

vsprojects/vcxproj/gpr/gpr.vcxproj

@@ -163,6 +163,7 @@
     <ClInclude Include="..\..\..\include\grpc\support\useful.h" />
   </ItemGroup>
   <ItemGroup>
+    <ClInclude Include="..\..\..\src\core\support\block_annotate.h" />
     <ClInclude Include="..\..\..\src\core\support\env.h" />
     <ClInclude Include="..\..\..\src\core\support\file.h" />
     <ClInclude Include="..\..\..\src\core\support\murmur_hash.h" />

vsprojects/vcxproj/gpr/gpr.vcxproj.filters

@@ -197,6 +197,9 @@
     </ClInclude>
   </ItemGroup>
   <ItemGroup>
+    <ClInclude Include="..\..\..\src\core\support\block_annotate.h">
+      <Filter>src\core\support</Filter>
+    </ClInclude>
     <ClInclude Include="..\..\..\src\core\support\env.h">
       <Filter>src\core\support</Filter>
     </ClInclude>