<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Mar 17, 2015 at 1:58 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">REPOSITORY<br>
  rL LLVM<br>
<br>
================<br>
Comment at: lib/Core/Resolver.cpp:355<br>
@@ -353,3 +354,3 @@<br>
           if (_symbolTable.isCoalescedAway(atom))<br>
             _deadAtoms.insert(ref->target());<br>
           continue;<br>
----------------<br>
</span><span class="">davide wrote:<br>
> ruiu wrote:<br>
> > I think this change is not thread-safe because of this line. _deadAtoms.insert is not thread-safe.<br>
> Oh this is really a bummer, considering it's such an hot codepath :(<br>
> Do you think it makes sense to surround insert with a lock and measure again and/or do you have a proposal for an alternative?<br>
</span>Another solution would be to accumulate a separate vector in each thread, then merge them at the end. The `const_cast<Reference *>(ref)->setTarget(newTarget);` also needs some scrutiny. I would suggest testing these parallelism changes with threadsanitizer.<br></blockquote><div><br></div><div>You suggested creating map<thread::id, vector<T>> to accumulate results? An issue would be that before executing parallel_for_each, we don't know which and how many threads are going to execute its body. That means we need to extend the map in an atomic way.</div></div></div></div>