[PATCH] [Core] Update references in parallel

[Rui Ueyama]

I'm sorry, this code doesn't even compile. But I hope you got what I tried
to mean. :)

On Tue, Mar 17, 2015 at 1:46 PM, Rui Ueyama <ruiu at google.com> wrote:

> 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/0aad8d38/attachment.html>