THREADED_DELTA_SEARCH

THREADED_DELTA_SEARCH

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 589 bytes --]


  I'm using it in production for quite a long time now, and I wonder if
there is any specific reason why it's not default. Debian activated it
by default too a couple of weeks ago, which means that it's in
production on quite a large scale, and there are no issues reported
either. Would a patch making it default be accepted ? It's quite handy
given that SMP machines are really pervasive nowadays…

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: THREADED_DELTA_SEARCH

[Johannes Sixt]

Pierre Habouzit schrieb:
>   I'm using it in production for quite a long time now, and I wonder if
> there is any specific reason why it's not default. Debian activated it
> by default too a couple of weeks ago, which means that it's in
> production on quite a large scale, and there are no issues reported
> either. Would a patch making it default be accepted ? It's quite handy
> given that SMP machines are really pervasive nowadays…

Don't forget to turn it off in the MINGW section in the Makefile since we
don't have pthreads on Windows.

-- Hannes

Re: THREADED_DELTA_SEARCH

[Andreas Ericsson]

Johannes Sixt wrote:
> Pierre Habouzit schrieb:
>>   I'm using it in production for quite a long time now, and I wonder if
>> there is any specific reason why it's not default. Debian activated it
>> by default too a couple of weeks ago, which means that it's in
>> production on quite a large scale, and there are no issues reported
>> either. Would a patch making it default be accepted ? It's quite handy
>> given that SMP machines are really pervasive nowadays…
> 
> Don't forget to turn it off in the MINGW section in the Makefile since we
> don't have pthreads on Windows.
> 

I started trying that, but ran into problems almost immediately. Enabling
pthreads by default and disabling it for certain systems means we screw
over those poor sods that have their own config.mak files on systems
where there's no <pthread.h> to include, or where that header file
requires a bunch of other stuff to be in first.

On the one hand, that's not nice.
On the other hand, those who have a config.mak are probably savvy enough
figure out how to amend it disable threads as well.

Opinions? I'm all for enabling THREADED_DELTA_SEARCH by default. I've
been using it ever since v1.5.4.3-193-g833e3df when using the same
.gitconfig on all my systems became a sane thing to do and I haven't
seen any ill effects what so ever from it, but how much can we screw
the build-system in place today, and what other systems would, by
default, require the NO_PTHREADS option in the Makefile?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: THREADED_DELTA_SEARCH

[Jakub Narebski]

Pierre Habouzit <madcoder@debian.org> writes:

>   I'm using it in production for quite a long time now, and I wonder if
> there is any specific reason why it's not default. Debian activated it
> by default too a couple of weeks ago, which means that it's in
> production on quite a large scale, and there are no issues reported
> either. Would a patch making it default be accepted ? It's quite handy
> given that SMP machines are really pervasive nowadays…

Doues our ./configure detects if it makes sense (i.e. if it is SMP
machine)?  What is the cost of THREADED_DELTA_SEARCH for single processor
single core machine?

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: THREADED_DELTA_SEARCH

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1172 bytes --]

On Thu, Jul 10, 2008 at 08:51:06AM +0000, Jakub Narebski wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> >   I'm using it in production for quite a long time now, and I wonder if
> > there is any specific reason why it's not default. Debian activated it
> > by default too a couple of weeks ago, which means that it's in
> > production on quite a large scale, and there are no issues reported
> > either. Would a patch making it default be accepted ? It's quite handy
> > given that SMP machines are really pervasive nowadays…
> 
> Doues our ./configure detects if it makes sense (i.e. if it is SMP
> machine)?

  I don't know if it does, but it wouldn't make sense for distros.

>  What is the cost of THREADED_DELTA_SEARCH for single processor
> single core machine?

  Threading has to be enabled in the configuration, and there is a
--thread=0 mode that enables autodetection of the number of CPUs online.
so there is no overhead for single CPU systems.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: THREADED_DELTA_SEARCH

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

>   I'm using it in production for quite a long time now, and I wonder if
> there is any specific reason why it's not default. Debian activated it
> by default too a couple of weeks ago, which means that it's in
> production on quite a large scale, and there are no issues reported
> either. Would a patch making it default be accepted ? It's quite handy
> given that SMP machines are really pervasive nowadays…

There were two issues, IIRC.

 (1) resulting pack could be suboptimal, due to delta-chain getting cut
     betweeen the worker thread boundary.

 (2) exploding memory use, suspected to be due to malloc pool
     fragmentation under multithreading.

The former was only an issue with early iterations of the code and the
current code has much improved behaviour on this point, but I do not
recall the latter issue has been addressed.

Re: THREADED_DELTA_SEARCH

[Nicolas Pitre]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1337 bytes --]

On Thu, 10 Jul 2008, Junio C Hamano wrote:

> Pierre Habouzit <madcoder@debian.org> writes:
> 
> >   I'm using it in production for quite a long time now, and I wonder if
> > there is any specific reason why it's not default. Debian activated it
> > by default too a couple of weeks ago, which means that it's in
> > production on quite a large scale, and there are no issues reported
> > either. Would a patch making it default be accepted ? It's quite handy
> > given that SMP machines are really pervasive nowadays…
> 
> There were two issues, IIRC.
> 
>  (1) resulting pack could be suboptimal, due to delta-chain getting cut
>      betweeen the worker thread boundary.
> 
>  (2) exploding memory use, suspected to be due to malloc pool
>      fragmentation under multithreading.
> 
> The former was only an issue with early iterations of the code and the
> current code has much improved behaviour on this point,

I would say it is even negligible.

> but I do not
> recall the latter issue has been addressed.

Well, for "standard" repositories such as the Linux kernel, things 
always worked just fine.  And commit eac12e2d is apparently helping a 
lot with the remaining odd cases.  And if someone has problems due to 
this then a simple 'git config --global pack.threads 1' would restore 
the non threaded behavior.


Nicolas

Re: THREADED_DELTA_SEARCH

[Alex Riesen]

Nicolas Pitre, Thu, Jul 10, 2008 18:21:09 +0200:
> On Thu, 10 Jul 2008, Junio C Hamano wrote:
> >  (2) exploding memory use, suspected to be due to malloc pool
> >      fragmentation under multithreading.
...
> > but I do not
> > recall the latter issue has been addressed.
> 
> Well, for "standard" repositories such as the Linux kernel, things 
> always worked just fine.  And commit eac12e2d is apparently helping a 
> lot with the remaining odd cases.  And if someone has problems due to 
> this then a simple 'git config --global pack.threads 1' would restore 
> the non threaded behavior.

Have you ever seen what happens to Windows XP on memory shortage?
It freezes *hard*. Not a good behaviour for _default_ settings.

And if someone has no problems with memory, then a simple "git config
pack.threads 100" would restore the threaded behaviour for a
repository which supposedly will never grow up.

Re: THREADED_DELTA_SEARCH

[Nicolas Pitre]

On Thu, 10 Jul 2008, Alex Riesen wrote:

> Nicolas Pitre, Thu, Jul 10, 2008 18:21:09 +0200:
> > On Thu, 10 Jul 2008, Junio C Hamano wrote:
> > >  (2) exploding memory use, suspected to be due to malloc pool
> > >      fragmentation under multithreading.
> ...
> > > but I do not
> > > recall the latter issue has been addressed.
> > 
> > Well, for "standard" repositories such as the Linux kernel, things 
> > always worked just fine.  And commit eac12e2d is apparently helping a 
> > lot with the remaining odd cases.  And if someone has problems due to 
> > this then a simple 'git config --global pack.threads 1' would restore 
> > the non threaded behavior.
> 
> Have you ever seen what happens to Windows XP on memory shortage?

Who said this would be enabled on Windows?


Nicolas

Re: THREADED_DELTA_SEARCH

[Alex Riesen]

Nicolas Pitre, Thu, Jul 10, 2008 22:26:03 +0200:
> On Thu, 10 Jul 2008, Alex Riesen wrote:
> > 
> > Have you ever seen what happens to Windows XP on memory shortage?
> 
> Who said this would be enabled on Windows?
> 

Windows is just an example (exaggerated one). There are just systems
were resources are a problem.

Re: THREADED_DELTA_SEARCH

[Nicolas Pitre]

On Thu, 10 Jul 2008, Alex Riesen wrote:

> Nicolas Pitre, Thu, Jul 10, 2008 22:26:03 +0200:
> > On Thu, 10 Jul 2008, Alex Riesen wrote:
> > > 
> > > Have you ever seen what happens to Windows XP on memory shortage?
> > 
> > Who said this would be enabled on Windows?
> > 
> 
> Windows is just an example (exaggerated one).

Don't use that example then.

> There are just systems were resources are a problem.

Then on those systems you simply have to adjust the appropriate knob.  
That's why those knobs are available after all.


Nicolas

Re: THREADED_DELTA_SEARCH

[Alex Riesen]

Nicolas Pitre, Thu, Jul 10, 2008 22:36:35 +0200:
> > There are just systems were resources are a problem.
> 
> Then on those systems you simply have to adjust the appropriate knob.  

_after_ they crashed?

Re: THREADED_DELTA_SEARCH

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On Fri, Jul 11, 2008 at 05:59:56AM +0000, Alex Riesen wrote:
> 2008/7/11 Nicolas Pitre <nico@cam.org>:
> > Furthermore, the number of threads is, by default, equivalent to the
> > number of detected CPUs.  So if you have a 8-CPU machine, I sure hope
> > you have the RAM configuration that goes along with such a system.
> 
> ....and while the threads limit is implemented, the global memory limit is not
> (well, frankly, I just don't know. Maybe it is there, and I can't find it).

  You mean --window-memory= ?

  I agree this feature should be opt-in. Linux, FreeBSD and Solaris
should probably do fine I assume ? I'm quite sure the two first will do,
I really know the latter less though.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]