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

[David Blaikie via llvm-commits]

On Mon, Jul 31, 2017 at 7:17 PM Dean Michael Berris via Phabricator <
reviews at reviews.llvm.org> wrote:

> dberris added inline comments.
>
>
> ================
> Comment at: lib/xray/xray_inmemory_log.cc:46
>
> -std::mutex LogMutex;
> +__sanitizer::SpinMutex LogMutex;
>
> ----------------
> dblaikie wrote:
> > Might be worth separating these two changes (the mutex from the
> shared-ptr) - they seem pretty independent & this one looks like it's good
> forward progress to move away from standard library dependencies as is
> required by compiler-rt, the other's more of a stop-gap (or at least
> neutral on the progress in that direction).
> Thought about it, but for efficiency's sake, I suspect I'd like to make
> both changes as a group. They're both trying to achieve the same thing (no
> complex/atomic initialisation at init-time) and are working towards the
> same (partial) goal.
>

Not sure what efficiency there is to be gained here - pretty easy to commit
them separately. Though no great need to send them out for separate review,
you can make two commits once this is signed off. (though as a rule,
smaller changes are exponentially easier/faster to review - but I realize
we're already started here)


>
>
> ================
> Comment at: lib/xray/xray_inmemory_log.cc:85-86
>    static bool TSCSupported = probeRequiredCPUFeatures();
> -  static uint64_t CycleFrequency = TSCSupported ? getTSCFrequency()
> -                                   : __xray::NanosecondsPerSecond;
> +  static uint64_t CycleFrequency =
> +      TSCSupported ? getTSCFrequency() : __xray::NanosecondsPerSecond;
>
> ----------------
> dblaikie wrote:
> > unnecessary reformatting? or did something change in this line that I
> can't quite spot.
> Looks like this wasn't formatted correctly before, and clang-format did
> the needful for me. :)
>

But formatting unrelated parts of the file? Generally to be avoided.


>
>
> https://reviews.llvm.org/D36078
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170801/49acfa39/attachment.html>