<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 30, 2015 at 10:27 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, Jul 30, 2015 at 10:15 PM, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>> wrote:<br>
><br>
><br>
> On Thu, Jul 30, 2015 at 8:10 PM, Jake VanAdrighem <<a href="mailto:jvanadrighem@gmail.com">jvanadrighem@gmail.com</a>><br>
> wrote:<br>
>><br>
>><br>
>><br>
>> On Wed, Jul 29, 2015 at 7:27 PM, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>> wrote:<br>
>>><br>
>>><br>
>>><br>
>>> On Wed, Jul 29, 2015 at 6:55 PM, Bob Wilson <<a href="mailto:bob.wilson@apple.com">bob.wilson@apple.com</a>> wrote:<br>
>>>><br>
>>>><br>
>>>> On Jul 29, 2015, at 4:56 PM, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>> wrote:<br>
>>>><br>
>>>> Do relaxed atomics actually introduce that much of slowdown?<br>
>>>><br>
>>>><br>
>>>> I would definitely want to see some data showing that they do not slow<br>
>>>> things down before we decide to do this unconditionally. We’ve discussed<br>
>>>> this issue several times in the past. My recollection is those discussions<br>
>>>> ended with an acknowledgement there is a tradeoff between speed and accuracy<br>
>>>> and that we don’t all agree on where we want to be on that spectrum. Adding<br>
>>>> options to let people choose would be one solution. Good data, on a variety<br>
>>>> of platforms, showing that it doesn’t make much difference would be another<br>
>>>> way to resolve it.<br>
>>><br>
>>><br>
>>> +1<br>
>>><br>
>>> In my testing, the overhead of the existing instrumentation is about a 2x<br>
>>> slowdown, which is starting to get close to the range it would be very<br>
>>> difficult to play an instrumented game. I wouldn't want to make this much<br>
>>> slower. I'm glad to test this for you; I'll try to get around to this this<br>
>>> week.<br>
>>><br>
>><br>
>> Sean and I tested Alexey's patch on one of our large titles and got<br>
>> somewhere in the area of 2.5 to 3x worse performance than without AtomicRMW.<br>
>> For the game we tested, it was basically unplayable.<br>
><br>
><br>
> We also noticed that the top 100 functions (out of 10's of thousands; i.e.<br>
> <1%) accounted for over 97% of the total counts (and just the top 10 cover<br>
> more than 50% IIRC). The few we manually looked into seemed like they would<br>
> be trivially inlined anyway and/or their high count otherwise doesn't seem<br>
> to really contribute much useful information to the optimizer since we<br>
> should already be "getting those right" (stuff like simple constructors,<br>
> getters, or vector operators).<br>
<br>
</div></div>That matches exactly with our observation and the approach we take is<br>
to address this issue.<br>
<span class=""><br>
><br>
> Obviously this is a discussion for another thread, but there seems to be<br>
> enormous scope for reduction in our profiling overhead for those interested<br>
> in doing that; even just a simple file of 100 functions passed to the<br>
> compiler instructing those functions to not be instrumented would decrease<br>
> the profiling overhead by over 2 orders of magnitude for this title, based<br>
> on the above data.<br>
><br>
<br>
</span>yes, this will be another thread. Rong (cc'ed ) has collected lots of<br>
data (as well as the implementation to address this). He will share it<br>
early next week.<br></blockquote><div><br></div><div>Was Rong going to post something this week?</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
David<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
> -- Sean Silva<br>
><br>
><br>
>><br>
>><br>
>> Jake Van Adrighem<br>
>><br>
>>><br>
>>> Also, in the past David Li suggested that his findings were that not<br>
>>> using atomic operations "only contribute<br>
>>> to very small count variations"<br>
>>> <a href="https://groups.google.com/d/msg/llvm-dev/ScLa2xIdo9s/Ow1FPDVVRIoJ" rel="noreferrer" target="_blank">https://groups.google.com/d/msg/llvm-dev/ScLa2xIdo9s/Ow1FPDVVRIoJ</a><br>
>>> CC'ing David in case he has more input to the discussion.<br>
>>><br>
>>> -- Sean Silva<br>
>>><br>
>>><br>
>>>><br>
>>>><br>
>>>><br>
>>>> You're intentionally introducing a data race, this doesn't look good to<br>
>>>> me at all. However, I'm not confident about<br>
>>>> what's allowed in LLVM IR - it's not C++ where any source-level data<br>
>>>> race is UB, but not an x86 assembly either.<br>
>>>><br>
>>>> On Wed, Jul 29, 2015 at 4:43 PM, Justin Bogner <<a href="mailto:mail@justinbogner.com">mail@justinbogner.com</a>><br>
>>>> wrote:<br>
>>>>><br>
>>>>> Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>> writes:<br>
>>>>> > samsonov created this revision.<br>
>>>>> > samsonov added reviewers: dnovillo, bogner.<br>
>>>>> > samsonov added a subscriber: llvm-commits.<br>
>>>>> ><br>
>>>>> > Since we introduced counters for functions in COMDAT sections (e.g.<br>
>>>>> > inline functions from STL headers), these headers can easily be<br>
>>>>> > incremented concurrently by multiple threads. Replace load-add-store<br>
>>>>> > with a single "atomicrmw add" with monotonic memory ordering.<br>
>>>>><br>
>>>>> This significantly changes the performance characteristics of this<br>
>>>>> code,<br>
>>>>> pessimizing single-threaded users and potentially making the<br>
>>>>> multithreaded performance issues even worse.<br>
>>>>><br>
>>>>> It's fine to add an option to lower these to atomics, since this does<br>
>>>>> guarantee accuracy, but I think we need a switch to choose which kind<br>
>>>>> of<br>
>>>>> lowering we're interested in in that case.<br>
>>>>><br>
>>>>> > <a href="http://reviews.llvm.org/D11579" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11579</a><br>
>>>>> ><br>
>>>>> > Files:<br>
>>>>> >   lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
>>>>> ><br>
>>>>> > Index: lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
>>>>> > ===================================================================<br>
>>>>> > --- lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
>>>>> > +++ lib/Transforms/Instrumentation/InstrProfiling.cpp<br>
>>>>> > @@ -147,9 +147,9 @@<br>
>>>>> >    IRBuilder<> Builder(Inc->getParent(), *Inc);<br>
>>>>> >    uint64_t Index = Inc->getIndex()->getZExtValue();<br>
>>>>> >    Value *Addr = Builder.CreateConstInBoundsGEP2_64(Counters, 0,<br>
>>>>> > Index);<br>
>>>>> > -  Value *Count = Builder.CreateLoad(Addr, "pgocount");<br>
>>>>> > -  Count = Builder.CreateAdd(Count, Builder.getInt64(1));<br>
>>>>> > -  Inc->replaceAllUsesWith(Builder.CreateStore(Count, Addr));<br>
>>>>> > +  Builder.CreateAtomicRMW(AtomicRMWInst::Add, Addr,<br>
>>>>> > Builder.getInt64(1),<br>
>>>>> > +                          llvm::Monotonic);<br>
>>>>> > +  assert(Inc->use_empty() && "InstrProfIncrementInst has uses!");<br>
>>>>> >    Inc->eraseFromParent();<br>
>>>>> >  }<br>
>>>>><br>
>>>><br>
>>>><br>
>>>><br>
>>>> --<br>
>>>> Alexey Samsonov<br>
>>>> <a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a><br>
>>>> _______________________________________________<br>
>>>> llvm-commits mailing list<br>
>>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>>><br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> llvm-commits mailing list<br>
>>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>>><br>
>>><br>
>>><br>
>>> _______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>><br>
>><br>
><br>
</div></div></blockquote></div><br></div></div>