[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