Implement pack.compression and pack-objects --compression=N
Git's object store and packing are optimized for
* Expensive repo to repo bandwidth; and
* Small-ish files
This justifies extensive use of compression.
In a multi-developer *office* with inter-repository transfers occurring over a 100Mb+ LAN, there is less reason to compress files and slow down response times. Response times suffer even more when large files are involved. However, *off-line* pack compression may still be desirable to reduce storage space.
Consequently, for such a usage pattern it is useful to specify different compression levels for loose objects and packs. This patch implements a config variable pack.compression in addition to the existing core.compression, meant to be used for repacking. It also adds --compression=N to pack-objects, meant for push/pull/fetch, if different, or if different on a per-repository basis.
** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. ** git-repack.sh might also need to be modified, and how to pass --compression=N during push/pull/fetch has not been investigated.
This applies on top of the git-repack --max-pack-size patchset.
--- builtin-pack-objects.c | 56 ++++++++++++++++++++++++++++++++++++------------ 1 files changed, 42 insertions(+), 14 deletions(-)
builtin-pack-objects.c +42 -14
@@ -70,6 +70,7 @@ static uint32_t pack_size_limit;static int pack_to_stdout;static int num_preferred_base;static struct progress progress_state; +static int pack_compression_level, pack_compression_seen;/* * The object names in objects array are hashed with this hashtable,
@@ -414,6 +415,16 @@ static unsigned long write_object(struct sha1file *f,/* write limit if limited packsize and not first object */ unsigned long limit = pack_size_limit && nr_written ? pack_size_limit - write_offset :0 ; +/* no if no delta */ +int usable_delta = !entry->delta ?0 : +/* yes if unlimited packfile */ + !pack_size_limit ?1 : +/* no if base written to previous pack */ + entry->delta->offset == (off_t)-1 ?0 : +/* otherwise double-check written to this +* pack, like we do below +*/ + entry->delta->offset ?1 :0 ;if (!pack_to_stdout)crc32_begin (f);
@@ -423,8 +434,7 @@ static unsigned long write_object(struct sha1file *f, to_reuse =0 ;/* can't reuse what we don't have */ else if (obj_type == OBJ_REF_DELTA || obj_type == OBJ_OFS_DELTA)/* check_object() decided it for us ... */ - to_reuse = !pack_size_limit || - (entry->delta->offset && entry->delta->offset != (off_t)-1 ); + to_reuse = usable_delta;/* ... but pack split may override that */ else if (obj_type != entry->in_pack_type) to_reuse =0 ;/* pack has delta which is unusable */
@@ -435,6 +445,10 @@ static unsigned long write_object(struct sha1file *f,* and we do not need to deltify it. */ +/* differing core & pack compression when loose object -> must recompress */ +if (!entry->in_pack && pack_compression_level != zlib_compression_level) + to_reuse =0 ; +else if (!entry->in_pack && !entry->delta) {unsigned char *map;unsigned long mapsize;
@@ -462,16 +476,6 @@ static unsigned long write_object(struct sha1file *f,z_stream stream;unsigned long maxsize;void *out; -/* no if no delta */ -int usable_delta = !entry->delta ?0 : -/* yes if unlimited packfile */ - !pack_size_limit ?1 : -/* no if base written to previous pack */ - entry->delta->offset == (off_t)-1 ?0 : -/* otherwise double-check written to this -* pack, like we do below -*/ - entry->delta->offset ?1 :0 ; buf =read_sha1_file (entry->sha1, &type, &size);if (!buf)die ("unable to read %s" ,sha1_to_hex (entry->sha1));
@@ -493,7 +497,7 @@ static unsigned long write_object(struct sha1file *f, }/* compress the data to store and put compressed length in datalen */ memset (&stream,0 ,sizeof (stream)); -deflateInit (&stream, zlib_compression_level); +deflateInit (&stream, pack_compression_level); maxsize =deflateBound (&stream, size); out =xmalloc (maxsize);/* Compress it */
@@ -606,7 +610,7 @@ static unsigned long write_object(struct sha1file *f,unuse_pack (&w_curs); reused++; } -if (entry->delta) +if (usable_delta) written_delta++; written++;if (!pack_to_stdout)
@@ -1622,6 +1626,16 @@ static int git_pack_config(const char *k, const char *v) window =git_config_int (k, v);return 0 ; } +if (!strcmp (k,"pack.compression" )) { +int level =git_config_int (k, v); +if (level ==-1 ) + level =Z_DEFAULT_COMPRESSION ; +else if (level <0 || level >Z_BEST_COMPRESSION ) +die ("bad pack compression level %d" , level); + pack_compression_level = level; + pack_compression_seen =1 ; +return 0 ; + }return git_default_config (k, v); }
@@ -1732,6 +1746,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix) rp_ac =2 ;git_config (git_pack_config); +if (!pack_compression_seen) + pack_compression_level = zlib_compression_level; progress =isatty (2 );for (i =1 ; i < argc; i++) {
@@ -1759,6 +1775,18 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)usage (pack_usage);continue ; } +if (!prefixcmp (arg,"--compression=" )) { +char *end; +int level =strtoul (arg+14 , &end,0 ); +if (!arg[14 ] || *end) +usage (pack_usage); +if (level ==-1 ) + level =Z_DEFAULT_COMPRESSION ; +else if (level <0 || level >Z_BEST_COMPRESSION ) +die ("bad pack compression level %d" , level); + pack_compression_level = level; +continue ; + }if (!prefixcmp (arg,"--window=" )) {char *end; window =strtoul (arg+9 , &end,0 );
Signature · 1 line
1.5.2.rc0.787.g0014
Quoted 10 lines · Dana How
Consequently, for such a usage pattern it is useful to specify different compression levels for loose objects and packs. This patch implements a config variable pack.compression in addition to the existing core.compression, meant to be used for repacking. It also adds --compression=N to pack-objects, meant for push/pull/fetch, if different, or if different on a per-repository basis.
** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. **
I think we tweaked this area in the past, but I do not think the current setting was determined to be the best tradeoff for all workloads. To be able to discuss the patch, I think it needs to come with benchmark numbers using publicly available repositories as guinea pigs and set of typical git operations, so people can reproduce and compare notes.
Quoted 10 lines · Junio C Hamano
Quoted 9 lines · Dana How
Consequently, for such a usage pattern it is useful to specify different compression levels for loose objects and packs. This patch implements a config variable pack.compression in addition to the existing core.compression, meant to be used for repacking. It also adds --compression=N to pack-objects, meant for push/pull/fetch, if different, or if different on a per-repository basis.
Quoted 8 lines
Quoted 1 line
** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. **
I think we tweaked this area in the past, but I do not think the current setting was determined to be the best tradeoff for all workloads. To be able to discuss the patch, I think it needs to come with benchmark numbers using publicly available repositories as guinea pigs and set of typical git operations, so people can reproduce and compare notes.
OK, but this patch doesn't mandate any particular setting.
Its motivation in my work environment is for pack.compression
to be what core.compression currently is, and to set core.compression to 0 to speed up large commits
(the resulting space-inefficient loose objects will be scrubbed away by a later off-line repack).
Thus, my config settings (almost) change the gzip's behind a git-add to cp's. Do you want me to submit timings for a git-add/git-commit -a
on a typical 50-file commit I would be interested in,
with the (new) settings that I would use?
Thanks,
Signature · 1 line
Dana L. How [email protected] +1 650 804 5991 cell
Quoted 22 lines · Dana How
Quoted 10 lines · Junio C Hamano
Quoted 9 lines · Dana How
Consequently, for such a usage pattern it is useful to specify different compression levels for loose objects and packs. This patch implements a config variable pack.compression in addition to the existing core.compression, meant to be used for repacking. It also adds --compression=N to pack-objects, meant for push/pull/fetch, if different, or if different on a per-repository basis.
Quoted 8 lines
Quoted 1 line
** THIS PATCH IS UNTESTED AND MEANT FOR DISCUSSION. **
I think we tweaked this area in the past, but I do not think the current setting was determined to be the best tradeoff for all workloads. To be able to discuss the patch, I think it needs to come with benchmark numbers using publicly available repositories as guinea pigs and set of typical git operations, so people can reproduce and compare notes.
OK, but this patch doesn't mandate any particular setting.
Quoted 9 lines
Its motivation in my work environment is for pack.compression
to be what core.compression currently is, and to set core.compression to 0 to speed up large commits
(the resulting space-inefficient loose objects will be scrubbed away
by a later off-line repack).
Thus, my config settings (almost) change the gzip's behind a git-add to cp's. Do you want me to submit timings for a git-add/git-commit -a
on a typical 50-file commit I would be interested in,
with the (new) settings that I would use?
Note the linux-2.6 git tree from a week ago has 22K checked-out files of average size 11KB; the largest is fs/nls/nls-cp949.c at 874KB. (The largest file in git is gitk at 176K.)
The tree I'm interested in maintaining with git is almost 70GB checked-out in 13K files of average size >5.2MB. This is over 2 orders of magnitude larger average file size than current git users. (Some of these numbers may decrease after a little retraining ;-).) I would like git to perform as responsively as possible on files up to ~500MB. Within this tree, the largest file is 1234MB [I think checking this in was a mistake!]
and I did the following experiments on it:
$ rm -rf .git $ git-init Initialized empty Git repository in .git/ $ git-config core.compression -1 $ wc large.spef 12762072 37832482 1234082774 large.spef $ /usr/bin/time git-add large.spef 41.54user 0.70system 0:49.76elapsed 84%CPU (0avgtext+0avgdata 0maxresident)k $ ls -lR .git/objects/?? .git/objects/d5: total 83836 -r--r--r-- 1 how group 85670068 May 2 15:11 d6cde2af063cdfa835038385f29a897bf9533b
$ rm -rf .git $ git-init Initialized empty Git repository in .git/ $ git-config core.compression 1 $ wc large.spef 12762072 37832482 1234082774 large.spef $ /usr/bin/time git-add large.spef 23.66user 0.74system 0:34.07elapsed 71%CPU (0avgtext+0avgdata 0maxresident)k $ ls -lR .git/objects/?? .git/objects/d5: total 105116 -r--r--r-- 1 how group 107419557 May 2 15:13 d6cde2af063cdfa835038385f29a897bf9533b
So for a 25% increase in blob size I get 33% less elapsed time in git-add, all by changing core.compression from -1 to 1. I'll definitely take that improvement. [For the compressible files we typically have, using 0 is a bad idea: the CPU "advantage" is swamped out by the time to write a much larger file.]
Since I don't care [to the same degree] about the responsiveness of packing, I'd rather pack with -1 or better to keep packs small. (And inflation time seems independent of compression setting.) Since someone might be working while the packing is happening, I'd rather not change the config setting to achieve this. Hence the patch.
Concerning various public repositories, clearly the patch has no impact if you don't specify different core.compression and pack.compression values. If you do specify different values, I doubt there would be much noticeable speed-up for e.g. the linux-2.6 repo stats I included above. There would be some, but that wasn't the motivation for the patch.
Thanks,
Signature · 1 line
Dana L. How [email protected] +1 650 804 5991 cell
Quoted 5 lines · Dana How
So for a 25% increase in blob size I get 33% less elapsed time in git-add, all by changing core.compression from -1 to 1. I'll definitely take that improvement. [For the compressible files we typically have, using 0 is a bad idea: the CPU "advantage" is swamped out by the time to write a much larger file.]
The above number is about loose objects, right?
Quoted 2 lines
Since I don't care [to the same degree] about the responsiveness of packing, I'd rather pack with -1 or better to keep packs small.
I see. You are saying that the fact that core.compression is used also for packing makes the variable less useful.
I agree that it would make sense to have at least the pack and core compression independent. I am not sure if we would also want to make the pack compression tweakable depending on the purpose of the packing (network transfer vs .git/objects/pack/).
Quoted 1 line
(And inflation time seems independent of compression setting.)
True.
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Dana How
So for a 25% increase in blob size I get 33% less elapsed time in git-add, all by changing core.compression from -1 to 1. I'll definitely take that improvement. [For the compressible files we typically have, using 0 is a bad idea: the CPU "advantage" is swamped out by the time to write a much larger file.]
The above number is about loose objects, right?
Correct.
Quoted 4 lines
Quoted 2 lines
Since I don't care [to the same degree] about the responsiveness of packing, I'd rather pack with -1 or better to keep packs small.
I see. You are saying that the fact that core.compression is used also for packing makes the variable less useful.
Exactly.
Quoted 4 lines
I agree that it would make sense to have at least the pack and core compression independent. I am not sure if we would also want to make the pack compression tweakable depending on the purpose of the packing (network transfer vs .git/objects/pack/).
The final 12-line hunk in the patch implements --pack-compression=N. To be blunt, I'm not sure *I* need it. Perhaps a high-use web-accessible public repository would be interested in repacking off-line with 9, and creating packs for each puller using -1 to reduce CPU load. In the latter case, due to builtin-pack-objects.c:write_object():to_reuse==1, much of the transferred data would still be at the better compression but without again paying the CPU cost. Shall I remove it?
BTW, I'm now in the habit of browsing Git's Gitweb at git.or.cz and notice "pu" has --max-pack-size. You may want to upgrade to the last 5-patch version in which --max-pack-size no longer forces --no-reuse-delta at Shawn's request, among other clean-ups.
Thanks,
Signature · 1 line
Dana L. How [email protected] +1 650 804 5991 cell
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · Dana How
So for a 25% increase in blob size I get 33% less elapsed time in git-add, all by changing core.compression from -1 to 1. I'll definitely take that improvement. [For the compressible files we typically have, using 0 is a bad idea: the CPU "advantage" is swamped out by the time to write a much larger file.]
Quoted 7 lines
The above number is about loose objects, right?
Quoted 2 lines
Since I don't care [to the same degree] about the responsiveness of packing, I'd rather pack with -1 or better to keep packs small.
I see. You are saying that the fact that core.compression is used also for packing makes the variable less useful.
I think that would make sense to have separate configs for pack and loose object compression. When not specified they should simply default to core.compression if it exists. Otherwise I'd suggest that pack compression default level be Z_DEFAULT_COMPRESSION and loose object compression default level be Z_BEST_SPEED. This would make interactive operations like git-add and git-commit even faster by default.
Nicolas
Quoted 18 lines · Nicolas Pitre
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · Dana How
So for a 25% increase in blob size I get 33% less elapsed time in git-add, all by changing core.compression from -1 to 1. I'll definitely take that improvement. [For the compressible files we typically have, using 0 is a bad idea: the CPU "advantage" is swamped out by the time to write a much larger file.]
Quoted 7 lines
The above number is about loose objects, right?
Quoted 2 lines
Since I don't care [to the same degree] about the responsiveness of packing, I'd rather pack with -1 or better to keep packs small.
I see. You are saying that the fact that core.compression is used also for packing makes the variable less useful.
Quoted 6 lines
I think that would make sense to have separate configs for pack and loose object compression. When not specified they should simply default to core.compression if it exists. Otherwise I'd suggest that pack compression default level be
Z_DEFAULT_COMPRESSIONand loose object compression default level beZ_BEST_SPEED. This would make interactive operations like git-add and git-commit even faster by default.
I agree with your Z_BEST_SPEED idea. I did not include it in the patch b/c I didn't want to change any behavior in the absence of new config settings.
Are you actually arguing for *3* different compression-related config variables? How about:
(a) core.compression controls loose objects. defaults to Z_BEST_SPEED.
(b) pack.compression controls packing. defaults to Z_DEFAULT_COMPRESSION if neither variable exists. defaults to core.compression if only that exists
The last sentence mimics current behavior and to me seems least surprising. Or pack.compression could be simpler: default to Z_DEFAULT_COMPRESSION if pack.compression doesn't exist (no interaction with core.compression).
Thanks,
Signature · 1 line
Dana L. How [email protected] +1 650 804 5991 cell
Quoted 26 lines · Dana How
Quoted 18 lines · Nicolas Pitre
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · Dana How
So for a 25% increase in blob size I get 33% less elapsed time in git-add, all by changing core.compression from -1 to 1. I'll definitely take that improvement. [For the compressible files we typically have, using 0 is a bad idea: the CPU "advantage" is swamped out by the time to write a much larger file.]
Quoted 7 lines
The above number is about loose objects, right?
Quoted 2 lines
Since I don't care [to the same degree] about the responsiveness of packing, I'd rather pack with -1 or better to keep packs small.
I see. You are saying that the fact that core.compression is used also for packing makes the variable less useful.
Quoted 6 lines
I think that would make sense to have separate configs for pack and loose object compression. When not specified they should simply default to core.compression if it exists. Otherwise I'd suggest that pack compression default level be
Z_DEFAULT_COMPRESSIONand loose object compression default level beZ_BEST_SPEED. This would make interactive operations like git-add and git-commit even faster by default.
Quoted 6 lines
I agree with your
Z_BEST_SPEEDidea. I did not include it in the patch b/c I didn't want to change any behavior in the absence of new config settings.Are you actually arguing for *3* different compression-related config variables?
Yes.
Quoted 4 lines
How about:
(a) core.compression controls loose objects. defaults toZ_BEST_SPEED.
(b) pack.compression controls packing. defaults toZ_DEFAULT_COMPRESSIONif neither variable exists. defaults to core.compression if only that exists
Yes, although I wouldn't default pack.compression to core.compression if pack.compression doesn't exist. The documentation about core.compression currently talks
(wrongly) only about loose objects anyway, so making pack.compression stand on its own won't be that bad.
Nicolas
Quoted 19 lines · Nicolas Pitre
Quoted 7 lines · Dana How
Quoted 6 lines · Nicolas Pitre
I think that would make sense to have separate configs for pack and loose object compression. When not specified they should simply default to core.compression if it exists. Otherwise I'd suggest that pack compression default level be
Z_DEFAULT_COMPRESSIONand loose object compression default level beZ_BEST_SPEED. This would make interactive operations like git-add and git-commit even faster by default.Quoted 5 lines
I agree with your
Z_BEST_SPEEDidea. I did not include it in the patch b/c I didn't want to change any behavior in the absence of new config settings.
Are you actually arguing for *3* different compression-related config variables?Yes.
Quoted 4 lines
How about:
(a) core.compression controls loose objects. defaults toZ_BEST_SPEED.
(b) pack.compression controls packing. defaults toZ_DEFAULT_COMPRESSIONif neither variable exists. defaults to core.compression if only that exists
Quoted 5 lines
Yes, although I wouldn't default pack.compression to core.compression if pack.compression doesn't exist. The documentation about core.compression currently talks
(wrongly) only about loose objects anyway, so making pack.compression stand on its own won't be that bad.
Now that I'm awake your original quote at the top suggests:
(a) zlib_compression_level =
isset(core.loosecompression) ? core.loosecompression :
isset(core.compression) ? core.compression : Z_BEST_SPEED;
(b) pack_compression_level =
isset(pack.compression) ? pack.compression :
isset(core.compression) ? core.compression : Z_DEFAULT_COMPRESSION;
Your later reaction to my quoted (a)/(b) table suggests:
(a) zlib_compression_level =
isset(core.compression) ? core.compression : Z_BEST_SPEED;
(b) pack_compresion_level =
isset(pack.compression) ? pack.compression : Z_DEFAULT_COMPRESSION;In either case, the C variablezlib_compression_level controls compression level for loose objects, and pack_compression_level controls compression in a pack. "isset()" means an active setting does appear in a config file (it could be the default value).
The 2nd behavior table is sufficient for me and simpler than my patch. Do you want the 1st or 2nd behavior?
Thanks,
Signature · 1 line
Dana L. How [email protected] +1 650 804 5991 cell
Quoted 8 lines · Dana How
Now that I'm awake your original quote at the top suggests:
(a)zlib_compression_level=
isset(core.loosecompression) ? core.loosecompression :
isset(core.compression) ? core.compression :Z_BEST_SPEED;
(b)pack_compression_level=
isset(pack.compression) ? pack.compression :
isset(core.compression) ? core.compression :Z_DEFAULT_COMPRESSION;
Quoted 5 lines
Your later reaction to my quoted (a)/(b) table suggests:
(a)zlib_compression_level=isset(core.compression) ? core.compression : Z_BEST_SPEED; (b) pack_compresion_level = isset(pack.compression) ? pack.compression : Z_DEFAULT_COMPRESSION;
My final comment is that I think the former looks more coherent, while the later is simpler to implement.
Nicolas