Re: [PATCH] process_{tree,blob}: Remove useless xstrdup calls

["Björn Steinbrink" <B.Steinbrink@gmx.de>]

On 2009.04.11 11:19:01 -0700, Linus Torvalds wrote:
> On Sat, 11 Apr 2009, Björn Steinbrink wrote:
> > On 2009.04.11 15:41:12 +0200, Björn Steinbrink wrote:
> > > On 2009.04.10 18:15:26 -0700, Linus Torvalds wrote:
> > > > It obviously goes on top of my previous patch.
> > > 
> > > Gives some nice results for the "rev-list --all --objects" test on the
> > > gentoo repo says (with the old pack):
> > >      | With my patch | With your patch on top
> > > -----|---------------|-----------------------
> > > VSZ  |       1667952 | 1319324
> > > RSS  |       1388408 | 1126080
> > 
> > linux-2.6.git:
> > 
> >      | With my patch | With your patch on top
> > -----|---------------|-----------------------
> > VSZ  |        460376 | 407900
> > RSS  |        292996 | 239760
> 
> Interesting. That's a 18+% reduction in RSS in both cases. Much bigger 
> than I expected, or what I saw in my limited testing. Is this in 32-bit 
> mode, where the pointers are cheaper, and thus the non-pointer data 
> relatively more expensive and a bigger percentage of the total? We really 
> wasted a _lot_ of memory on those names.

No, this is x86-64, 8 byte pointers. But the savings are trivially
explained I think. The struct object_array things are 20 bytes here (per
object overhead!), so that's about 5M * 20 = 100M. And the average name
length for the objects was 19 bytes, which means about another 100M.
Both, the object_array stuff as well as the path names, were allocated
and never freed. Your patch removed the object_array stuff, and it made
the memory allocations for the names temporary. Right?

Had you moved just the path_name() calls, that would have meant that we
had needed to keep the name_path stuff around, which is also 20 bytes
here (two pointers, one int). And that would have meant that anything
that has a leading-up path shorter than 20 bytes (64 bit pointers) would
have seen increased memory usage (64bit pointers), but with 32 pointers,
the limit would have been 12 bytes.

So for the "just move path_name() call" solution, 32bit vs. 64bit would
have made a difference, but with your actual patches, you just turned
everything into temporary allocations. So the 4byte overhead on 64bit
platforms is just once linear with the directory-depth of the current
object, instead of with the number of objects in total.

Right?

Björn