Clean up RequestContext

Summary: Using a ThreadLocal cleans up the code quite a bit.  Also reuse the shared_ptr in create instead of creating a new one saves an allocation

Test Plan:
fbconfig thrift/lib/cpp/test:RequestContextTest; fbmake runtests

fbconfig common/services/cpp/test:trace_test; fbmake runtests

Reviewed By: hans@fb.com

Subscribers: trunkagent, doug, alandau, bmatheny, njormrod, mshneer, folly-diffs@, vloh

FB internal diff: D1663960

Signature: t1:1663960:1415390250:36d9b772016d2a12d804e98edbc1725af882e507

diff --git a/folly/io/async/Request.h b/folly/io/async/Request.h
index 9d1ff119581a452ea1321c4685298c86a596ee2a..c02ac5186d023226b97cb9afcfb1d691c58b2921 100644 (file)
--- a/folly/io/async/Request.h
+++ b/folly/io/async/Request.h
@@ -65,26 +65,22 @@ class RequestContext {
   // Create a unique requext context for this request.
   // It will be passed between queues / threads (where implemented),
   // so it should be valid for the lifetime of the request.
-  static bool create() {
+  static void create() {
     if(!FLAGS_enable_request_context) {
-      return false;
+      return;
     }
-    bool prev = getStaticContext().get() != nullptr;
-    getStaticContext().reset(new std::shared_ptr<RequestContext>(
-                     std::make_shared<RequestContext>()));
-    return prev;
+    getStaticContext() = std::make_shared<RequestContext>();
   }
 
   // Get the current context.
   static RequestContext* get() {
-    if (!FLAGS_enable_request_context ||
-        getStaticContext().get() == nullptr) {
+    if (getStaticContext() == nullptr) {
       if (defaultContext == nullptr) {
         defaultContext = new RequestContext;
       }
       return defaultContext;
     }
-    return getStaticContext().get()->get();
+    return getStaticContext().get();
   }
 
   // The following API may be used to set per-request data in a thread-safe way.
@@ -138,28 +134,17 @@ class RequestContext {
   setContext(std::shared_ptr<RequestContext> ctx) {
     if (FLAGS_enable_request_context) {
       std::shared_ptr<RequestContext> old_ctx;
-      if (getStaticContext().get()) {
-        old_ctx = *getStaticContext().get();
-      }
-      if (ctx == nullptr) {
-        getStaticContext().reset(nullptr);
-      } else {
-        getStaticContext().reset(new std::shared_ptr<RequestContext>(ctx));
+      if (getStaticContext()) {
+        old_ctx = getStaticContext();
       }
+      getStaticContext() = ctx;
       return old_ctx;
     }
-    return std::shared_ptr<RequestContext>();
+    return nullptr;
   }
 
   static std::shared_ptr<RequestContext> saveContext() {
-    if (!FLAGS_enable_request_context) {
-      return std::shared_ptr<RequestContext>();
-    }
-    if (getStaticContext().get() == nullptr) {
-      return std::shared_ptr<RequestContext>();
-    } else {
-      return *getStaticContext().get();
-    }
+    return getStaticContext();
   }
 
   // Used to solve static destruction ordering issue.  Any static object
@@ -168,10 +153,10 @@ class RequestContext {
   // See below link for more details.
   // http://stackoverflow.com/questions/335369/
   // finding-c-static-initialization-order-problems#335746
-  static folly::ThreadLocalPtr<std::shared_ptr<RequestContext>>&
+  static std::shared_ptr<RequestContext>&
   getStaticContext() {
-    static folly::ThreadLocalPtr<std::shared_ptr<RequestContext> > context;
-    return context;
+    static folly::ThreadLocal<std::shared_ptr<RequestContext> > context;
+    return *context;
   }
 
  private:
@@ -179,24 +164,4 @@ class RequestContext {
   std::map<std::string, std::unique_ptr<RequestData>> data_;
 };
 
-/**
- * Set the request context for a specific scope. For example,
- * if you ran a part of a request in another thread you could
- * use RequestContextGuard to copy apply the request context
- * inside the other therad.
- */
-class RequestContextGuard {
- public:
-  explicit RequestContextGuard(std::shared_ptr<RequestContext> ctx) {
-    oldctx_ = RequestContext::setContext(std::move(ctx));
-  }
-
-  ~RequestContextGuard() {
-    RequestContext::setContext(std::move(oldctx_));
-  }
-
- private:
-  std::shared_ptr<RequestContext> oldctx_;
-};
-
 }