[PATCH] [Core] Update references in parallel

[Rui Ueyama]

On Tue, Mar 17, 2015 at 1:25 PM, Davide Italiano <davide at freebsd.org> wrote:

> REPOSITORY
>   rL LLVM
>
> ================
> Comment at: lib/Core/Resolver.cpp:355
> @@ -353,3 +354,3 @@
>            if (_symbolTable.isCoalescedAway(atom))
>              _deadAtoms.insert(ref->target());
>            continue;
> ----------------
> ruiu wrote:
> > I think this change is not thread-safe because of this line.
> _deadAtoms.insert is not thread-safe.
> Oh this is really a bummer, considering it's such an hot codepath :(
> 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?


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.

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

diff --git a/include/lld/Core/Parallel.h b/include/lld/Core/Parallel.h
index 65176ac..bd80770 100644
--- a/include/lld/Core/Parallel.h
+++ b/include/lld/Core/Parallel.h
@@ -104,6 +104,39 @@ private:
   std::condition_variable _cond;
 };

+template<typename T> class ConcurrentVector {
+public:
+  ConcurrentVector() : _capacity(16), _size(0) {
+    _container.resize(_capacity);
+  }
+
+  void insert(T &&val) {
+    size_t idx = _size.fetch_add(1);
+    if (idx < _capacity) {
+      _container[idx] = std::move(val);
+      return;
+    }
+    std::unique_lock<std::mutex> lock(_mutex);
+    if (idx < _capacity) {
+      _container[idx] = std::move(val);
+      return;
+    }
+    while (capacity <= _idx)
+      _capacity *= 2;
+    _container.resize(_capacity);
+    _container[idx] = std::move(val);
+  }
+
+  size_t size() { return _size; }
+  T *data() { return _container.data(); }
+
+private:
+  std::vector<T> _container;
+  std::atomic_size_t _capacity;
+  std::atomic_size_t _size;
+  std::mutex _mutex;
+};
+
 /// \brief An abstract class that takes closures and runs them
asynchronously.
 class Executor {
 public:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150317/0b451c8a/attachment.html>