Switch to using a CAS loop to update the token value.

BUILD

@@ -309,6 +309,7 @@ grpc_cc_library(
         "src/core/lib/profiling/basic_timers.c",
         "src/core/lib/profiling/stap_timers.c",
         "src/core/lib/support/alloc.c",
+        "src/core/lib/support/atm.c",
         "src/core/lib/support/avl.c",
         "src/core/lib/support/backoff.c",
         "src/core/lib/support/cmdline.c",

CMakeLists.txt

@@ -693,6 +693,7 @@ add_library(gpr
   src/core/lib/profiling/basic_timers.c
   src/core/lib/profiling/stap_timers.c
   src/core/lib/support/alloc.c
+  src/core/lib/support/atm.c
   src/core/lib/support/avl.c
   src/core/lib/support/backoff.c
   src/core/lib/support/cmdline.c

Makefile

@@ -2599,6 +2599,7 @@ LIBGPR_SRC = \
     src/core/lib/profiling/basic_timers.c \
     src/core/lib/profiling/stap_timers.c \
     src/core/lib/support/alloc.c \
+    src/core/lib/support/atm.c \
     src/core/lib/support/avl.c \
     src/core/lib/support/backoff.c \
     src/core/lib/support/cmdline.c \

binding.gyp

@@ -544,6 +544,7 @@
         'src/core/lib/profiling/basic_timers.c',
         'src/core/lib/profiling/stap_timers.c',
         'src/core/lib/support/alloc.c',
+        'src/core/lib/support/atm.c',
         'src/core/lib/support/avl.c',
         'src/core/lib/support/backoff.c',
         'src/core/lib/support/cmdline.c',

build.yaml

@@ -101,6 +101,7 @@ filegroups:
   - src/core/lib/profiling/basic_timers.c
   - src/core/lib/profiling/stap_timers.c
   - src/core/lib/support/alloc.c
+  - src/core/lib/support/atm.c
   - src/core/lib/support/avl.c
   - src/core/lib/support/backoff.c
   - src/core/lib/support/cmdline.c

config.m4

@@ -39,6 +39,7 @@ if test "$PHP_GRPC" != "no"; then
     src/core/lib/profiling/basic_timers.c \
     src/core/lib/profiling/stap_timers.c \
     src/core/lib/support/alloc.c \
+    src/core/lib/support/atm.c \
     src/core/lib/support/avl.c \
     src/core/lib/support/backoff.c \
     src/core/lib/support/cmdline.c \

gRPC-Core.podspec

@@ -211,6 +211,7 @@ Pod::Spec.new do |s|
                       'src/core/lib/profiling/basic_timers.c',
                       'src/core/lib/profiling/stap_timers.c',
                       'src/core/lib/support/alloc.c',
+                      'src/core/lib/support/atm.c',
                       'src/core/lib/support/avl.c',
                       'src/core/lib/support/backoff.c',
                       'src/core/lib/support/cmdline.c',

grpc.gemspec

@@ -97,6 +97,7 @@ Gem::Specification.new do |s|
   s.files += %w( src/core/lib/profiling/basic_timers.c )
   s.files += %w( src/core/lib/profiling/stap_timers.c )
   s.files += %w( src/core/lib/support/alloc.c )
+  s.files += %w( src/core/lib/support/atm.c )
   s.files += %w( src/core/lib/support/avl.c )
   s.files += %w( src/core/lib/support/backoff.c )
   s.files += %w( src/core/lib/support/cmdline.c )

include/grpc/impl/codegen/atm.h

@@ -92,4 +92,9 @@
 #error could not determine platform for atm
 #endif
 
+/** Adds \a delta to \a *value, clamping the result to the range specified
+    by \a min and \a max.  Returns the new value. */
+gpr_atm gpr_atm_no_barrier_clamped_add(gpr_atm *value, gpr_atm delta,
+                                       gpr_atm min, gpr_atm max);
+
 #endif /* GRPC_IMPL_CODEGEN_ATM_H */

package.xml

@@ -106,6 +106,7 @@
     <file baseinstalldir="/" name="src/core/lib/profiling/basic_timers.c" role="src" />
     <file baseinstalldir="/" name="src/core/lib/profiling/stap_timers.c" role="src" />
     <file baseinstalldir="/" name="src/core/lib/support/alloc.c" role="src" />
+    <file baseinstalldir="/" name="src/core/lib/support/atm.c" role="src" />
     <file baseinstalldir="/" name="src/core/lib/support/avl.c" role="src" />
     <file baseinstalldir="/" name="src/core/lib/support/backoff.c" role="src" />
     <file baseinstalldir="/" name="src/core/lib/support/cmdline.c" role="src" />

src/core/ext/client_channel/retry_throttle.c

@@ -73,20 +73,9 @@ bool grpc_server_retry_throttle_data_record_failure(
   // First, check if we are stale and need to be replaced.
   get_replacement_throttle_data_if_needed(&throttle_data);
   // We decrement milli_tokens by 1000 (1 token) for each failure.
-  const int delta = -1000;
-  const int old_value = (int)gpr_atm_full_fetch_add(
-      &throttle_data->milli_tokens, (gpr_atm)delta);
-  // If the above change takes us below 0, then re-add the excess.  Note
-  // that between these two atomic operations, the value will be
-  // artificially low by as much as 1000, but this window should be
-  // brief.
-  int new_value = old_value - 1000;
-  if (new_value < 0) {
-    const int excess_value = new_value - (old_value < 0 ? old_value : 0);
-    gpr_atm_full_fetch_add(&throttle_data->milli_tokens,
-                           (gpr_atm)-excess_value);
-    new_value = 0;
-  }
+  const int new_value = (int)gpr_atm_no_barrier_clamped_add(
+      &throttle_data->milli_tokens, (gpr_atm)-1000, (gpr_atm)0,
+      (gpr_atm)throttle_data->max_milli_tokens);
   // Retries are allowed as long as the new value is above the threshold
   // (max_milli_tokens / 2).
   return new_value > throttle_data->max_milli_tokens / 2;
@@ -97,22 +86,9 @@ void grpc_server_retry_throttle_data_record_success(
   // First, check if we are stale and need to be replaced.
   get_replacement_throttle_data_if_needed(&throttle_data);
   // We increment milli_tokens by milli_token_ratio for each success.
-  const int delta = throttle_data->milli_token_ratio;
-  const int old_value = (int)gpr_atm_full_fetch_add(
-      &throttle_data->milli_tokens, (gpr_atm)delta);
-  // If the above change takes us over max_milli_tokens, then subtract
-  // the excess.  Note that between these two atomic operations, the
-  // value will be artificially high by as much as milli_token_ratio,
-  // but this window should be brief.
-  const int new_value = old_value + throttle_data->milli_token_ratio;
-  if (new_value > throttle_data->max_milli_tokens) {
-    const int excess_value =
-        new_value - (old_value > throttle_data->max_milli_tokens
-                         ? old_value
-                         : throttle_data->max_milli_tokens);
-    gpr_atm_full_fetch_add(&throttle_data->milli_tokens,
-                           (gpr_atm)-excess_value);
-  }
+  gpr_atm_no_barrier_clamped_add(
+      &throttle_data->milli_tokens, (gpr_atm)throttle_data->milli_token_ratio,
+      (gpr_atm)0, (gpr_atm)throttle_data->max_milli_tokens);
 }
 
 grpc_server_retry_throttle_data* grpc_server_retry_throttle_data_ref(

src/core/lib/support/atm.c

@@ -0,0 +1,47 @@
+/*
+ *
+ * Copyright 2017, 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.
+ *
+ */
+
+#include <grpc/support/atm.h>
+#include <grpc/support/useful.h>
+
+gpr_atm gpr_atm_no_barrier_clamped_add(gpr_atm *value, gpr_atm delta,
+                                       gpr_atm min, gpr_atm max) {
+  gpr_atm current;
+  gpr_atm new;
+  do {
+    current = gpr_atm_no_barrier_load(value);
+    new = GPR_CLAMP(current + delta, min, max);
+    if (new == current) break;
+  } while (!gpr_atm_no_barrier_cas(value, current, new));
+  return new;
+}

src/python/grpcio/grpc_core_dependencies.py

@@ -33,6 +33,7 @@ CORE_SOURCE_FILES = [
   'src/core/lib/profiling/basic_timers.c',
   'src/core/lib/profiling/stap_timers.c',
   'src/core/lib/support/alloc.c',
+  'src/core/lib/support/atm.c',
   'src/core/lib/support/avl.c',
   'src/core/lib/support/backoff.c',
   'src/core/lib/support/cmdline.c',

tools/doxygen/Doxyfile.core.internal

@@ -1230,6 +1230,7 @@ src/core/lib/slice/slice_internal.h \
 src/core/lib/slice/slice_string_helpers.c \
 src/core/lib/slice/slice_string_helpers.h \
 src/core/lib/support/alloc.c \
+src/core/lib/support/atm.c \
 src/core/lib/support/avl.c \
 src/core/lib/support/backoff.c \
 src/core/lib/support/backoff.h \

tools/run_tests/generated/sources_and_headers.json

@@ -7311,6 +7311,7 @@
       "src/core/lib/profiling/stap_timers.c", 
       "src/core/lib/profiling/timers.h", 
       "src/core/lib/support/alloc.c", 
+      "src/core/lib/support/atm.c", 
       "src/core/lib/support/avl.c", 
       "src/core/lib/support/backoff.c", 
       "src/core/lib/support/backoff.h", 

vsprojects/vcxproj/gpr/gpr.vcxproj

@@ -208,6 +208,8 @@
     </ClCompile>
     <ClCompile Include="$(SolutionDir)\..\src\core\lib\support\alloc.c">
     </ClCompile>
+    <ClCompile Include="$(SolutionDir)\..\src\core\lib\support\atm.c">
+    </ClCompile>
     <ClCompile Include="$(SolutionDir)\..\src\core\lib\support\avl.c">
     </ClCompile>
     <ClCompile Include="$(SolutionDir)\..\src\core\lib\support\backoff.c">

vsprojects/vcxproj/gpr/gpr.vcxproj.filters

@@ -10,6 +10,9 @@
     <ClCompile Include="$(SolutionDir)\..\src\core\lib\support\alloc.c">
       <Filter>src\core\lib\support</Filter>
     </ClCompile>
+    <ClCompile Include="$(SolutionDir)\..\src\core\lib\support\atm.c">
+      <Filter>src\core\lib\support</Filter>
+    </ClCompile>
     <ClCompile Include="$(SolutionDir)\..\src\core\lib\support\avl.c">
       <Filter>src\core\lib\support</Filter>
     </ClCompile>