<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 29, 2017 at 4:32 PM, Vedant Kumar via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">vsk added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/ProfileData/<wbr>InstrProfReader.cpp:736<br>
 Error IndexedInstrProfReader::<wbr>readNextRecord(<wbr>NamedInstrProfRecord &Record) {<br>
-  static unsigned RecordIndex = 0;<br>
+  thread_local static unsigned RecordIndex = 0;<br>
<br>
----------------<br>
Why isn't this just a member variable?<br>
<br>
If we used "thread_local", wouldn't there still be a bug if you instantiated and used two IndexedInstrProfReaders?<br></blockquote><div><br></div><div>I think making this member variable would also work -- and this might be a better way.</div><div><br></div><div>But why you think "thread_local" won't work? IndexedInstrProfReader::readNextRecord can not be multithreaded in either way. If we instantiate two IndexedInstrProfReaders, each should be in it own thread and accesses its own RecordIndex. There is not racing.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
================<br>
Comment at: tools/llvm-profdata/llvm-<wbr>profdata.cpp:230<br>
     // Load the inputs in parallel (N/NumThreads serial steps).<br>
-    unsigned Ctx = 0;<br>
-    for (const auto &Input : Inputs) {<br>
-      Pool.async(loadInput, Input, Contexts[Ctx].get());<br>
-      Ctx = (Ctx + 1) % NumThreads;<br>
+    unsigned ChunkSize = Inputs.size() / NumThreads;<br>
+    unsigned ChunkRemainder = Inputs.size() % NumThreads;<br>
----------------<br>
I don't understand the problem here. Each time a WriterContext is accessed, the accessing thread immediately acquires a unique lock on the context. Where does the racing access occur?<br></blockquote><div><br></div><div>Where the lock to WriterContext comes from? I only see the lock for the queue in ThreadPool. I thought it was the user's responsibility to make sure the share data is race free.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D38431" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D38431</a><br>
<br>
<br>
<br>
</blockquote></div><br></div></div>