Re: [PATCH v5 00/11] nd/pack-objects-pack-struct updates

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 17, 2018, 19:45 UTC
Message-ID: <87po42cwql.fsf@evledraar.gmail.com>
In-Reply-To: <20180317141033.21545-1-pclouds@gmail.com>

On Sat, Mar 17 2018, Nguyễn Thái Ngọc Duy jotted:

> v5 changes are small enough that the interdiff is pretty self
> explanatory (there's also a couple commit msg updates).

I've been testing this and it's definitely an improvement. I think it would be good to get some mention in the commit messages themselves of the incremental improvement, to that end I wrote this:

    $ cat /tmp/howmuch-mem.sh
    #!/bin/sh
    cd /tmp &&
    (
        for i in {1..3}
        do
            /usr/bin/time -f MaxRSS:%M ~/g/git/git --git-dir=/tmp/linux.git --exec-path=/home/avar/g/git repack -A -d 2>&1
        done | grep MaxRSS: | sort -n | head -n 1 | tr '\n' '\t' &&
        git git-commit-summary &&
        echo
    ) | tee -a /tmp/git-memory.log

I.e. we repack linux.git (I'd already repacked it once) and do three runs, and take the lowest RSS size. This yields (I rebased your series on top of git@github.com:git/git.git master and pushed it to git@github.com:avar/git.git pack-objects-reduce-memory-footprint), via:

    git rebase --exec='make -j8 CFLAGS="-O3" && /tmp/howmuch-mem.sh' -i

That gave me, kb on the first column:

    MaxRSS:3746648  f23a196dd9 ("pack-objects: a bit of document about struct object_entry", 2018-03-01)
    MaxRSS:3700696  953b6473d7 ("pack-objects: turn type and in_pack_type to bitfields", 2018-03-01)
    MaxRSS:3700404  6cbe573539 ("pack-objects: use bitfield for object_entry::dfs_state", 2018-03-01)
    MaxRSS:3654044  0b93ebcae9 ("pack-objects: use bitfield for object_entry::depth", 2018-03-01)
    MaxRSS:3654040  67a4d48773 ("pack-objects: move in_pack_pos out of struct object_entry", 2018-03-01) [X]
    MaxRSS:3654104  e77319c65a ("pack-objects: move in_pack out of struct object_entry", 2018-03-01) [X]
    MaxRSS:3608096  a72cfcfea3 ("pack-objects: refer to delta objects by index instead of pointer", 2018-03-01)
    MaxRSS:3562212  76eaa779eb ("pack-objects: shrink z_delta_size field in struct object_entry", 2018-03-05)
    MaxRSS:3515164  42e28dd4b3 ("pack-objects: shrink size field in struct object_entry", 2018-03-05)
    MaxRSS:3469440  26eba3ded4 ("pack-objects: shrink delta_size field in struct object_entry", 2018-03-05)
    MaxRSS:3423704  c6493de964 ("pack-objects.h: reorder members to shrink struct object_entry", 2018-03-12)

I.e. on git.git we end up with just over a a 8.5% reduction, and interestingly have a slight increase over a past commit in one change, and one that just makes 4kb of difference (marked via [X] above).

Also, your v0 says it overall saves 260MB of memory. According to this it's 320MB. You did note some reductions in subsequent patches, but it's worth calling that out explicitly.

I have a bigger in-house repo that looks like this with this change:

    MaxRSS:4753120  f23a196dd9 ("pack-objects: a bit of document about struct object_entry", 2018-03-01)
    MaxRSS:4699084  953b6473d7 ("pack-objects: turn type and in_pack_type to bitfields", 2018-03-01)
    MaxRSS:4699028  6cbe573539 ("pack-objects: use bitfield for object_entry::dfs_state", 2018-03-01)
    MaxRSS:4645452  0b93ebcae9 ("pack-objects: use bitfield for object_entry::depth", 2018-03-01)
    MaxRSS:4645288  67a4d48773 ("pack-objects: move in_pack_pos out of struct object_entry", 2018-03-01)
    MaxRSS:4645548  e77319c65a ("pack-objects: move in_pack out of struct object_entry", 2018-03-01)
    MaxRSS:4591484  a72cfcfea3 ("pack-objects: refer to delta objects by index instead of pointer", 2018-03-01)
    MaxRSS:4537980  76eaa779eb ("pack-objects: shrink z_delta_size field in struct object_entry", 2018-03-05)
    MaxRSS:4484148  42e28dd4b3 ("pack-objects: shrink size field in struct object_entry", 2018-03-05)
    MaxRSS:4430404  26eba3ded4 ("pack-objects: shrink delta_size field in struct object_entry", 2018-03-05)
    MaxRSS:4376148  c6493de964 ("pack-objects.h: reorder members to shrink struct object_entry", 2018-03-12)

I.e. a tad more than a 7.9% reduction in memory use.

This series also doesn't make a difference to the total runtime (which is good, just wanted to make sure). On linux.git on my box best out of three is 1:15.74 before and 1:14.93 after, which is within the margin of random error.