On Thu, Feb 13, 2014 at 9:02 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 13 February 2014 23:40, Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
> wrote:
> > On Thu, Feb 13, 2014 at 7:55 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> >>
> >> On 13 February 2014 22:24, <stefan2_at_apache.org> wrote:
> >> > Author: stefan2
> >> > Date: Thu Feb 13 18:24:17 2014
> >> > New Revision: 1567996
> >> >
> >> > URL: http://svn.apache.org/r1567996
> >
> >
> >>
> >> > +/* Count a hit in ENTRY within CACHE.
> >>
> >> > + */
> >> > +static void
> >> > +increment_hit_counters(svn_membuffer_t *cache, entry_t *entry)
> >> > +{
> >> > + /* To minimize the memory footprint of the cache index, we limit
> >> > local
> >> > + * hit counters to 32 bits. These may overflow and we must make
> sure
> >> > that
> >> > + * the global sums are still (roughly due to races) the sum of all
> >> > local
> >> > + * counters. */
> >> > + if (++entry->hit_count == 0)
> >> > + cache->hit_count -= APR_UINT32_MAX;
> >> > + else
> >> > + cache->hit_count++;
> >> > +
> >> As far I understand this counters are updated from several threads
> >> without mutex?
> >
> >
> > That is correct.
> >
> >>
> >> In that case we have to use atomic functions to
> >> increment them, otherwise we may skip hit_count == 0 in some cases.
> >
> >
> > No.
>
> > Assuming that the system does not split 32 bit read nor write operation
> I really, really don't like such assumptions in Subversion code. APR
> and OS already provides functions for proper inter process
> manipulations and we must use them, instead of saving several cycles.
>
Well, you were wrong on your initial point ("missing zero crossing").
And you have been wrong on the "use the atomic functions" bit because
they don't cover 64 bit integers. The only available option is a critical
section (svn_mutex__t).
So, I spent a few hours running actual load tests to see how lock
contention in a critical section would play out on a "real" server
machine with high multi-threaded load. And we got lucky (as opposed
to e.g. the APR thread pool code that we currently use in svnserve).
The new code is in r1568062.
-- Stefan^2.
Received on 2014-02-13 23:57:20 CET