<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">I'm sorry, this code doesn't even compile. But I hope you got what I tried to mean. :)</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Tue, Mar 17, 2015 at 1:46 PM, Rui Ueyama <span dir="ltr"><<a href="mailto:ruiu@google.com" target="_blank">ruiu@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 dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Tue, Mar 17, 2015 at 1:25 PM, Davide Italiano <span dir="ltr"><<a href="mailto:davide@freebsd.org" target="_blank">davide@freebsd.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span>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>ruiu wrote:<br>
> I think this change is not thread-safe because of this line. _deadAtoms.insert is not thread-safe.<br>
</span>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?</blockquote><div><br></div></span><div>Yeah, this is a hot code path. I imagine protecting this line with a mutex would be costly, but we probably should just do that and measure.</div><div><br></div><div>One other thing we could do is to use a concurrent queue or something as an accumulator and then copy its contents to _deadAtoms. I wrote the following concurrent container before. You might want to try. (Sorry, I haven't tested so this may be slow or has a bug.)<br></div><div><br></div><div><div>diff --git a/include/lld/Core/Parallel.h b/include/lld/Core/Parallel.h</div><div>index 65176ac..bd80770 100644</div><div>--- a/include/lld/Core/Parallel.h</div><div>+++ b/include/lld/Core/Parallel.h</div><div>@@ -104,6 +104,39 @@ private:</div><div>   std::condition_variable _cond;</div><div> };</div><div> </div><div>+template<typename T> class ConcurrentVector {</div><div>+public:</div><div>+  ConcurrentVector() : _capacity(16), _size(0) {</div><div>+    _container.resize(_capacity);</div><div>+  }</div><div>+</div><div>+  void insert(T &&val) {</div><div>+    size_t idx = _size.fetch_add(1);</div><div>+    if (idx < _capacity) {</div><div>+      _container[idx] = std::move(val);</div><div>+      return;</div><div>+    }</div><div>+    std::unique_lock<std::mutex> lock(_mutex);</div><div>+    if (idx < _capacity) {</div><div>+      _container[idx] = std::move(val);</div><div>+      return;</div><div>+    }</div><div>+    while (capacity <= _idx)</div><div>+      _capacity *= 2;</div><div>+    _container.resize(_capacity);</div><div>+    _container[idx] = std::move(val);</div><div>+  }</div><div>+</div><div>+  size_t size() { return _size; }</div><div>+  T *data() { return _container.data(); }</div><div>+</div><div>+private:</div><div>+  std::vector<T> _container;</div><div>+  std::atomic_size_t _capacity;</div><div>+  std::atomic_size_t _size;</div><div>+  std::mutex _mutex;</div><div>+};</div><div>+</div><div> /// \brief An abstract class that takes closures and runs them asynchronously.</div><div> class Executor {</div><div> public:</div></div></div></div></div>
</blockquote></div><br></div></div>