Re: [PATCH] [RFC] profiler updates

On 06/08/2012 07:02 PM, Amos Jeffries wrote:
> On 8/06/2012 2:46 a.m., Alex Rousskov wrote:
>> On 06/06/2012 07:25 PM, Amos Jeffries wrote:
>>>>> + if (delta< head->best)
>>>>> + head->best = delta;
>>>>> + if (delta> head->worst)
>>>>> + head->worst = delta;
>>>>> + head->summ += delta;
>>>>> + head->count++;
>>>>> + head->overheads += get_tick() - stopped_;
>>>> The above XProfilerNode::stop() code is not thread-safe because
>>>> multiple
>>>> threads may update the same "head" and you are not using atomics for
>>>> *head members.
>>> Ok, for now this is the second major TODO.
>>> I'm torn between GCC __sync_* and C++11 atomic_* types.
>>>
>>> There is maybe a wrapper compat layer needed temporarily I think.
>> Since SMP Squid already uses GCC __sync_* primitives and has the
>> corresponding wrappers, consider reusing them for your project. See
>> ipc/AtomicWord.h for a starting point.
>>
>> The existing wrappers can be adjusted to use C++11 atomic_* types when
>> those are provided by the build environment, but that sounds like a very
>> different project to me.
>
> The __sync seem to needs a lot of individual data entry locks and
> un-locks for this.

Based on the code you have posted and my recollection of the old
profiling code, no explicit locks are needed. Perhaps my recollection is
wrong or your code is different now.

Alex.

> For a simpler patch I've gone with one spinlocks on the global
> variables, and one on each linked-list entry of the stats accuumulation
> tree. Making it atomic change to each bunch of details as a small group
> rather than multiple sync and unlock operations. They are always
> accessed for write as a group. Just means GCC-4.4 or later to build the
> profiler.
>
> Amos
Received on Sun Jun 10 2012 - 16:47:43 MDT