Re: [PATCH][BZ #16145] Reduce lock contention in __tz_convert()

[Mike Frysinger <vapier at gentoo dot org>]

On 26 Feb 2015 02:29, Kevin Easton wrote:
> On Tue, Feb 24, 2015 at 12:01:58AM -0500, Mike Frysinger wrote:
> > On 12 Feb 2015 00:37, Kevin Easton wrote:
> > > This patch is an "easy win" partial fix for BZ #16145, which notes
> > > the heavy contention on tzset_lock when multiple threads are converting
> > > times with localtime_r().
> > > 
> > > In __tz_convert(), the lock does not need to be held after
> > > __tzfile_compute() / __tz_compute() have been called, so we can move the
> > > unlock up.  At this point there is still significant work to be done in
> > > __offtime(), so we see some improvement (in my testing with 8 cores
> > > banging on localtime_r(), ~20% improvement in throughput).
> > 
> > my reading of __offtime is that it only operates on its arguments (or const 
> > global data like __mon_yday).  it also looks expensive, so maybe we should 
> > hoist the remaining call outside of holding the lock ?  can you see if that'd 
> > have any measurable improvement ?
> > 
> >   ...
> >   int offtime = 0;
> >   if (!__use_tzfile)
> >     offtime = __offtime (...);
> > 
> >   __libc_lock_lock (tzset_lock);
> 
> I don't think it's that easy, because __use_tzfile is protected by
> tzset_lock.

blah, i was thinking it was a local.  man i hate global state :).

> We could call __offtime() outside the lock unconditionally, but that
> would slow down the common case where __use_tzfile is set.
> 
> I'm intending to look at deeper changes to allow the entire 
> __tzfile_compute() / tz_compute() to be done outside the lock, by taking
> a reference on the parsed tz data.  (I think I will probably need to sort
> the copyright assignment stuff for that though).

yes, most likely that level of work would require assignment
-mike

Attachment: signature.asc
Description: Digital signature