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

[Dean Michael Berris via llvm-commits]

On Tue, Aug 1, 2017 at 12:29 PM David Blaikie <dblaikie at gmail.com> wrote:

> 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)
>

ACK -- yes, I meant in terms of review back-forth and timezone overlap. :)


>
>
>>
>>
>> ================
>> 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.
>
>

Yes, reverted. :)

Thanks Dave!
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170801/218f25c2/attachment.html>