[PATCH] [Core] Update references in parallel

Davide Italiano davide at freebsd.org
Tue Mar 17 14:01:06 PDT 2015


On Tue, Mar 17, 2015 at 1:55 PM, Rui Ueyama <ruiu at google.com> wrote:
> I'm sorry, this code doesn't even compile. But I hope you got what I tried
> to mean. :)
>

Yeah, I'll play around a little bit with there (I also thought about
per-thread sets) and hopefully report results.
We're lucky we need to support only insert() and count() for this data
structure so we might get away with something not too
complicated.Thanks for the input.

> 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:
>
>



-- 
Davide

"There are no solved problems; there are only problems that are more
or less solved" -- Henri Poincare



More information about the llvm-commits mailing list