[PATCH] [Core] Update references in parallel

[Sean Silva]

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

> On Tue, Mar 17, 2015 at 1:58 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>> REPOSITORY
>>   rL LLVM
>>
>> ================
>> Comment at: lib/Core/Resolver.cpp:355
>> @@ -353,3 +354,3 @@
>>            if (_symbolTable.isCoalescedAway(atom))
>>              _deadAtoms.insert(ref->target());
>>            continue;
>> ----------------
>> davide wrote:
>> > 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?
>> Another solution would be to accumulate a separate vector in each thread,
>> then merge them at the end. The `const_cast<Reference
>> *>(ref)->setTarget(newTarget);` also needs some scrutiny. I would suggest
>> testing these parallelism changes with threadsanitizer.
>>
>
> You suggested creating map<thread::id, vector<T>> to accumulate results?
> An issue would be that before executing parallel_for_each, we don't know
> which and how many threads are going to execute its body. That means we
> need to extend the map in an atomic way.
>

This sounds like a deficiency in the API of parallel_for_each. Maybe it
could take a 4th argument lambda that is called with the number of threads
or an array of thread::id's?

-- Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150317/ddcb7762/attachment.html>