Re: [PATCH 2/2] pack-objects: report actual number of threads to be used

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 5 Apr 2009, Dan McGee wrote:

> On Sat, Apr 4, 2009 at 7:11 PM, Nicolas Pitre <nico@xxxxxxx> wrote:
> > On Sat, 4 Apr 2009, Jeff King wrote:
> >
> >> On Sat, Apr 04, 2009 at 01:20:18PM -0500, Dan McGee wrote:
> >>
> >> > > That makes sense to me, though I wonder if it may confuse and frustrate
> >> > > users who are expecting their awesome quad-core machine to be using 4
> >> > > threads when it only uses 2. Is it worth printing both values, or some
> >> > > indicator that we could have been using more?
> >> >
> >> > I thought of this, but decided it wasn't really worth it. The default
> >> > window size of 10 makes it a very rare case that you will use fewer
> >> > than 4 threads. With the default, each thread needs a minimum of 20
> >> > objects, so even a 100-object repository would spawn the 4 threads.
> >>
> >> Good point. Though by that logic, isn't your patch also not worth it
> >> (i.e., it is unlikely not to fill the threads, so the output will be the
> >> same with or without it)?
> >>
> >> I still think yours is an improvement, though, however slight.
> >
> > I don't think this is worth it at all.
> >
> > This display is there mainly to confirm expected number of available
> > threads.  The number of actually active threads is an implementation
> > detail.  Sure if the number of objects is too low, or if the window size
> > is too large, then the number of active threads will be lower.  But in
> > practice it is also possible that with some patological object set you
> > end up with 2 threads out of 4 completing very quickly and the other 2
> > threads still busy with big objects and total remaining work set too
> > small to split it further amongst idle threads, meaning that you'll end
> > up with only 2 busy CPUs even though the display said 4 threads
> > initially even with this patch.
> >
> > In other words I don't think this patch is a good idea as we don't
> > update the display with remaining active threads along the way.
> 
> Why do we show this misleading at best piece of information at all
> then? I'd rather completely remove it than show lies to the user.

As you might imagine, I don't share your above appreciation.

> It
> sounds like it is only there for debugging purposes.

... which is still worthwhile nevertheless.

> If we choose to keep it, I propose either accepting my patch so we are
> not mislead, or dropping the thread count completely from the output
> and saying only something like "Using multi-threaded delta
> compression."

Your patch is not better.  Instead, it will confuse people who 
explicitly told git to use x threads but the display might say x-y 
threads, with 0 <= y < x.

The number currently displayed has real meaning: this is the number of 
threads git is allowed to use.  The number of threads it will actually 
use is variable and it changes with time.


Nicolas

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]