[PATCH] D36078: [XRay][compiler-rt] Remove use of std::mutex and std::shared_ptr from global scope.

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 1 21:34:42 PDT 2017


dberris added a comment.

Thanks @kpw!



================
Comment at: lib/xray/xray_fdr_logging_impl.h:172
 // FDRLogging, and that we're going to clean it up when the thread exits.
-thread_local std::shared_ptr<BufferQueue> LocalBQ = nullptr;
-thread_local ThreadExitBufferCleanup Cleanup(LocalBQ, Buffer);
+thread_local std::shared_ptr<BufferQueue> *LocalBQ =
+    new std::shared_ptr<BufferQueue>();
----------------
kpw wrote:
> Not a big deal, but earlier you declare "shared_ptr<T>* name" and here you have "shared_ptr<T> *name".
> 
> Not sure which is preferred in LLVM.
The former is LLVM preferred. Somehow my local formatter keeps ignoring the LLVM style. :(


================
Comment at: lib/xray/xray_fdr_logging_impl.h:497
       writeEOBMetadata();
-      if (!releaseThreadLocalBuffer(LocalBQ.get()))
+      if (!releaseThreadLocalBuffer(*LocalBQ))
         return false;
----------------
kpw wrote:
> Here LocalBQ is a shared_ptr& that shadows the thread local shared_ptr* of the same name. The shadowing is not new, but the types used to match.
> 
> I find this confusing. Can we rename the param LocalBQ to ParamBQ? Although I don't think dblakie's suggestion to pass \*\*LocalBQ from the call sites applies here too because \*\*LocalBQ is allowed to be nullptr if RecordPtr == nullptr or xray status is XRAY_LOG_INITIALIZING.
Good point. Renamed to LBQ instead in this function.


https://reviews.llvm.org/D36078





More information about the llvm-commits mailing list