From 0975e12db1af7fdf17144140f0d267fc666b586a Mon Sep 17 00:00:00 2001
From: Arjun Roy <arjunroy@google.com>
Date: Thu, 18 Jul 2019 12:45:47 -0700
Subject: [PATCH] Reduced atomic strictness inside grpc_core::Fork.

The enable state for fork operations, used by ExecCtx initialization, uses
default std::atomic semantics on read/write - this is sequential consistency by
default. However, this variable is only set:
1) On init,
2) Using testing interfaces

And thus we can just use relaxed semantics instead. This PR also moves from
std::atomic to grpc_core::Atomic.
---
 src/core/lib/gprpp/fork.cc | 35 ++++++++++++++---------------------
 src/core/lib/gprpp/fork.h  | 19 +++++++++++++++----
 2 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/src/core/lib/gprpp/fork.cc b/src/core/lib/gprpp/fork.cc
index 37552692373..51836d52cf4 100644
--- a/src/core/lib/gprpp/fork.cc
+++ b/src/core/lib/gprpp/fork.cc
@@ -168,40 +168,33 @@ class ThreadState {
 
 void Fork::GlobalInit() {
   if (!override_enabled_) {
-    support_enabled_ = GPR_GLOBAL_CONFIG_GET(grpc_enable_fork_support);
+    support_enabled_.Store(GPR_GLOBAL_CONFIG_GET(grpc_enable_fork_support),
+                           MemoryOrder::RELAXED);
   }
-  if (support_enabled_) {
+  if (support_enabled_.Load(MemoryOrder::RELAXED)) {
     exec_ctx_state_ = grpc_core::New<internal::ExecCtxState>();
     thread_state_ = grpc_core::New<internal::ThreadState>();
   }
 }
 
 void Fork::GlobalShutdown() {
-  if (support_enabled_) {
+  if (support_enabled_.Load(MemoryOrder::RELAXED)) {
     grpc_core::Delete(exec_ctx_state_);
     grpc_core::Delete(thread_state_);
   }
 }
 
-bool Fork::Enabled() { return support_enabled_; }
+bool Fork::Enabled() { return support_enabled_.Load(MemoryOrder::RELAXED); }
 
 // Testing Only
 void Fork::Enable(bool enable) {
   override_enabled_ = true;
-  support_enabled_ = enable;
+  support_enabled_.Store(enable, MemoryOrder::RELAXED);
 }
 
-void Fork::IncExecCtxCount() {
-  if (support_enabled_) {
-    exec_ctx_state_->IncExecCtxCount();
-  }
-}
+void Fork::DoIncExecCtxCount() { exec_ctx_state_->IncExecCtxCount(); }
 
-void Fork::DecExecCtxCount() {
-  if (support_enabled_) {
-    exec_ctx_state_->DecExecCtxCount();
-  }
-}
+void Fork::DoDecExecCtxCount() { exec_ctx_state_->DecExecCtxCount(); }
 
 void Fork::SetResetChildPollingEngineFunc(
     Fork::child_postfork_func reset_child_polling_engine) {
@@ -212,38 +205,38 @@ Fork::child_postfork_func Fork::GetResetChildPollingEngineFunc() {
 }
 
 bool Fork::BlockExecCtx() {
-  if (support_enabled_) {
+  if (support_enabled_.Load(MemoryOrder::RELAXED)) {
     return exec_ctx_state_->BlockExecCtx();
   }
   return false;
 }
 
 void Fork::AllowExecCtx() {
-  if (support_enabled_) {
+  if (support_enabled_.Load(MemoryOrder::RELAXED)) {
     exec_ctx_state_->AllowExecCtx();
   }
 }
 
 void Fork::IncThreadCount() {
-  if (support_enabled_) {
+  if (support_enabled_.Load(MemoryOrder::RELAXED)) {
     thread_state_->IncThreadCount();
   }
 }
 
 void Fork::DecThreadCount() {
-  if (support_enabled_) {
+  if (support_enabled_.Load(MemoryOrder::RELAXED)) {
     thread_state_->DecThreadCount();
   }
 }
 void Fork::AwaitThreads() {
-  if (support_enabled_) {
+  if (support_enabled_.Load(MemoryOrder::RELAXED)) {
     thread_state_->AwaitThreads();
   }
 }
 
 internal::ExecCtxState* Fork::exec_ctx_state_ = nullptr;
 internal::ThreadState* Fork::thread_state_ = nullptr;
-std::atomic<bool> Fork::support_enabled_;
+Atomic<bool> Fork::support_enabled_(false);
 bool Fork::override_enabled_ = false;
 Fork::child_postfork_func Fork::reset_child_polling_engine_ = nullptr;
 }  // namespace grpc_core
diff --git a/src/core/lib/gprpp/fork.h b/src/core/lib/gprpp/fork.h
index 73f2fa56aa7..3601d7ca925 100644
--- a/src/core/lib/gprpp/fork.h
+++ b/src/core/lib/gprpp/fork.h
@@ -21,7 +21,7 @@
 
 #include <grpc/support/port_platform.h>
 
-#include <atomic>
+#include "src/core/lib/gprpp/atomic.h"
 
 /*
  * NOTE: FORKING IS NOT GENERALLY SUPPORTED, THIS IS ONLY INTENDED TO WORK
@@ -47,10 +47,18 @@ class Fork {
 
   // Increment the count of active ExecCtxs.
   // Will block until a pending fork is complete if one is in progress.
-  static void IncExecCtxCount();
+  static void IncExecCtxCount() {
+    if (GPR_UNLIKELY(support_enabled_.Load(MemoryOrder::RELAXED))) {
+      DoIncExecCtxCount();
+    }
+  }
 
   // Decrement the count of active ExecCtxs
-  static void DecExecCtxCount();
+  static void DecExecCtxCount() {
+    if (GPR_UNLIKELY(support_enabled_.Load(MemoryOrder::RELAXED))) {
+      DoDecExecCtxCount();
+    }
+  }
 
   // Provide a function that will be invoked in the child's postfork handler to
   // reset the polling engine's internal state.
@@ -80,9 +88,12 @@ class Fork {
   static void Enable(bool enable);
 
  private:
+  static void DoIncExecCtxCount();
+  static void DoDecExecCtxCount();
+
   static internal::ExecCtxState* exec_ctx_state_;
   static internal::ThreadState* thread_state_;
-  static std::atomic<bool> support_enabled_;
+  static grpc_core::Atomic<bool> support_enabled_;
   static bool override_enabled_;
   static child_postfork_func reset_child_polling_engine_;
 };