[PATCH] [Core] Update references in parallel
Rui Ueyama
ruiu at google.com
Tue Mar 17 13:55:13 PDT 2015
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>
More information about the llvm-commits
mailing list