{"thread":{"id":"1042","subject":"[PATCH] denser delta header encoding","startedAt":"2005-06-29T04:27:45Z","lastAt":"2005-06-29T06:07:34Z","messageCount":9,"participants":["Nicolas Pitre","Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"5394","messageId":"Pine.LNX.4.63.0506290021050.1667@localhost.localdomain","threadId":"1042","inReplyTo":null,"subject":"[PATCH] denser delta header encoding","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2005-06-29T04:27:45Z","receivedAt":"2005-06-29T04:27:45Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"\nSince the delta data format is not tied to any actual git object \nanymore, now is the time to add a small improvement to the delta data \nheader as it is been done for packed object header. This patch allows \nfor reducing the delta header of about 2 bytes and makes for simpler \ncode.\n\nSigned-off-by: Nicolas Pitre \n\ndiff --git a/count-delta.c b/count-delta.c\n--- a/count-delta.c\n+++ b/count-delta.c\n@@ -11,16 +11,13 @@\n static unsigned long get_hdr_size(const unsigned char **datap)\n {\n \tconst unsigned char *data = *datap;\n-\tunsigned long size;\n-\tunsigned char cmd;\n-\tint i;\n-\tsize = i = 0;\n-\tcmd = *data++;\n-\twhile (cmd) {\n-\t\tif (cmd & 1)\n-\t\t\tsize |= *data++ << i;\n-\t\ti += 8;\n-\t\tcmd >>= 1;\n+\tunsigned char cmd = *data++;\n+\tunsigned long size = cmd & ~0x80;\n+\tint i = 7;\n+\twhile (cmd & 0x80) {\n+\t\tcmd = *data++;\n+\t\tsize |= (cmd & ~0x80) << i;\n+\t\ti += 7;\n \t}\n \t*datap = data;\n \treturn size;\n@@ -47,8 +44,8 @@ int count_delta(void *delta_buf, unsigne\n \tunsigned char cmd;\n \tunsigned long src_size, dst_size, out;\n \n-\t/* the smallest delta size possible is 6 bytes */\n-\tif (delta_size < 6)\n+\t/* the smallest delta size possible is 4 bytes */\n+\tif (delta_size < 4)\n \t\treturn -1;\n \n \tdata = delta_buf;\ndiff --git a/diff-delta.c b/diff-delta.c\n--- a/diff-delta.c\n+++ b/diff-delta.c\n@@ -228,28 +228,22 @@ void *diff_delta(void *from_buf, unsigne\n \ttop = to_buf + to_size;\n \n \t/* store reference buffer size */\n-\torig = out + outpos++;\n-\t*orig = i = 0;\n-\tdo {\n-\t\tif (from_size & 0xff) {\n-\t\t\t*orig |= (1 << i);\n-\t\t\tout[outpos++] = from_size;\n-\t\t}\n-\t\ti++;\n-\t\tfrom_size >>= 8;\n-\t} while (from_size);\n+\tout[outpos++] = from_size;\n+\tfrom_size >>= 7;\n+\twhile (from_size) {\n+\t\tout[outpos - 1] |= 0x80;\n+\t\tout[outpos++] = from_size;\n+\t\tfrom_size >>= 7;\n+\t}\n \n \t/* store target buffer size */\n-\torig = out + outpos++;\n-\t*orig = i = 0;\n-\tdo {\n-\t\tif (to_size & 0xff) {\n-\t\t\t*orig |= (1 << i);\n-\t\t\tout[outpos++] = to_size;\n-\t\t}\n-\t\ti++;\n-\t\tto_size >>= 8;\n-\t} while (to_size);\n+\tout[outpos++] = to_size;\n+\tto_size >>= 7;\n+\twhile (to_size) {\n+\t\tout[outpos - 1] |= 0x80;\n+\t\tout[outpos++] = to_size;\n+\t\tto_size >>= 7;\n+\t}\n \n \tinscnt = 0;\n \tmoff = 0;\ndiff --git a/patch-delta.c b/patch-delta.c\n--- a/patch-delta.c\n+++ b/patch-delta.c\n@@ -22,33 +22,33 @@ void *patch_delta(void *src_buf, unsigne\n \tunsigned long size;\n \tint i;\n \n-\t/* the smallest delta size possible is 6 bytes */\n-\tif (delta_size < 6)\n+\t/* the smallest delta size possible is 4 bytes */\n+\tif (delta_size < 4)\n \t\treturn NULL;\n \n \tdata = delta_buf;\n \ttop = delta_buf + delta_size;\n \n \t/* make sure the orig file size matches what we expect */\n-\tsize = i = 0;\n \tcmd = *data++;\n-\twhile (cmd) {\n-\t\tif (cmd & 1)\n-\t\t\tsize |= *data++ << i;\n-\t\ti += 8;\n-\t\tcmd >>= 1;\n+\tsize = cmd & ~0x80;\n+\ti = 7;\n+\twhile (cmd & 0x80) {\n+\t\tcmd = *data++;\n+\t\tsize |= (cmd & ~0x80) << i;\n+\t\ti += 7;\n \t}\n \tif (size != src_size)\n \t\treturn NULL;\n \n \t/* now the result size */\n-\tsize = i = 0;\n \tcmd = *data++;\n-\twhile (cmd) {\n-\t\tif (cmd & 1)\n-\t\t\tsize |= *data++ << i;\n-\t\ti += 8;\n-\t\tcmd >>= 1;\n+\tsize = cmd & ~0x80;\n+\ti = 7;\n+\twhile (cmd & 0x80) {\n+\t\tcmd = *data++;\n+\t\tsize |= (cmd & ~0x80) << i;\n+\t\ti += 7;\n \t}\n \tdst_buf = malloc(size);\n \tif (!dst_buf)\n"},{"id":"5395","messageId":"7vmzp9kbcf.fsf@assigned-by-dhcp.cox.net","threadId":"1042","inReplyTo":"Pine.LNX.4.63.0506290021050.1667@localhost.localdomain","subject":"Re: [PATCH] denser delta header encoding","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-29T04:58:56Z","receivedAt":"2005-06-29T04:58:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus, please do not apply this as is.\n\nThere are code other than what Nico updated with this patch in\nsha1_file.c that also need updating, that count the number of\nbytes in the delta-patch result by reading from the delta\nheader.\n\nI wonder if we can have a helper function in delta suite\nsomewhere (maybe in diff-delta.c):\n\n int look_at_delta_header(void **delta_data, ulong delta_size,\n \t ulong *src_size, ulong *dst_size)\n\nthat:\n\n - checks delta size and barf if it is small;\n - reads the header and fills src_size and dst_size;\n - advances *delta_data pointer;\n\nand have count-delta, patch-delta and sha1_file.c users use it\nconsistently. Nico, what do you think?\n"},{"id":"5397","messageId":"Pine.LNX.4.58.0506282217010.19755@ppc970.osdl.org","threadId":"1042","inReplyTo":"7vmzp9kbcf.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] denser delta header encoding","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-29T05:21:37Z","receivedAt":"2005-06-29T05:21:37Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 28 Jun 2005, Junio C Hamano wrote:\n>\n> Linus, please do not apply this as is.\n\nArgh. Too late.\n\n> There are code other than what Nico updated with this patch in\n> sha1_file.c that also need updating, that count the number of\n> bytes in the delta-patch result by reading from the delta\n> header.\n\nHmm.. I tested this (along with my change to make the pack object also \nuse the little-endian size encoding) with \"gitk\" on a packed git archive, \nand with git-unpack-objects. So it can't break _too_ seriously. Is it the \n\"git-cat-file -s\" thing that gets the wrong answer?\n\nAhh. I see it. \"packed_delta_info()\". And it looks like \"diff\" uses it \ntoo. Oh, for the copy and rename detection. I don't think I tested that \npart, nope.\n\n> I wonder if we can have a helper function in delta suite\n> somewhere (maybe in diff-delta.c):\n> \n> int look_at_delta_header(void **delta_data, ulong delta_size,\n> \t ulong *src_size, ulong *dst_size)\n> \n> that:\n> \n> - checks delta size and barf if it is small;\n> - reads the header and fills src_size and dst_size;\n> - advances *delta_data pointer;\n> \n> and have count-delta, patch-delta and sha1_file.c users use it\n> consistently. Nico, what do you think?\n\nSounds like a good idea.\n\n\t\tLinus\n"},{"id":"5398","messageId":"7virzxk9nd.fsf@assigned-by-dhcp.cox.net","threadId":"1042","inReplyTo":"Pine.LNX.4.58.0506282217010.19755@ppc970.osdl.org","subject":"Re: [PATCH] denser delta header encoding","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-29T05:35:34Z","receivedAt":"2005-06-29T05:35:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"LT\" == Linus Torvalds writes:\n\nLT> Ahh. I see it. \"packed_delta_info()\". And it looks like \"diff\" uses it \nLT> too. Oh, for the copy and rename detection. I don't think I tested that \nLT> part, nope.\n\nOK, not too much damage done. I'll fix the rest up.\n"},{"id":"5400","messageId":"Pine.LNX.4.58.0506282244110.19755@ppc970.osdl.org","threadId":"1042","inReplyTo":"7virzxk9nd.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] denser delta header encoding","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-29T05:44:49Z","receivedAt":"2005-06-29T05:44:49Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 28 Jun 2005, Junio C Hamano wrote:\n> \n> OK, not too much damage done. I'll fix the rest up.\n\nActually, I already did, and pushed out. And this time I verified it by \ndoing a \"git-cat-file -s\" on every object on a packed repo.\n\n\t\tLinus\n"},{"id":"5401","messageId":"Pine.LNX.4.63.0506290146270.1667@localhost.localdomain","threadId":"1042","inReplyTo":"Pine.LNX.4.58.0506282244110.19755@ppc970.osdl.org","subject":"Re: [PATCH] denser delta header encoding","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2005-06-29T05:49:04Z","receivedAt":"2005-06-29T05:49:04Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 28 Jun 2005, Linus Torvalds wrote:\n\n> \n> \n> On Tue, 28 Jun 2005, Junio C Hamano wrote:\n> > \n> > OK, not too much damage done. I'll fix the rest up.\n> \n> Actually, I already did, and pushed out. And this time I verified it by \n> doing a \"git-cat-file -s\" on every object on a packed repo.\n\nDamn!\n\nAnd just when I was about to send a new patch with the thing nicely \nabstracted to fix the dammage.\n\nOK... let me pull again to see what's left then.\n\n\nNicolas\n"},{"id":"5403","messageId":"7vacl9k8iz.fsf@assigned-by-dhcp.cox.net","threadId":"1042","inReplyTo":"Pine.LNX.4.63.0506290146270.1667@localhost.localdomain","subject":"Re: [PATCH] denser delta header encoding","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-29T05:59:48Z","receivedAt":"2005-06-29T05:59:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":">>>>> \"NP\" == Nicolas Pitre writes:\n\nNP> On Tue, 28 Jun 2005, Linus Torvalds wrote:\n>> \n>> \n>> On Tue, 28 Jun 2005, Junio C Hamano wrote:\n>> > \n>> > OK, not too much damage done. I'll fix the rest up.\n>> \n>> Actually, I already did, and pushed out. And this time I verified it by \n>> doing a \"git-cat-file -s\" on every object on a packed repo.\n\nNP> Damn!\n\nNP> And just when I was about to send a new patch with the thing nicely \nNP> abstracted to fix the dammage.\n\nDouble Damn!! Three people working on the same piece of code.\n\nAnd Linus head still breaks with t5300. Triple damn X-<.\n"},{"id":"5404","messageId":"Pine.LNX.4.58.0506282306420.14331@ppc970.osdl.org","threadId":"1042","inReplyTo":"7vacl9k8iz.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] denser delta header encoding","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-06-29T06:07:22Z","receivedAt":"2005-06-29T06:07:22Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 28 Jun 2005, Junio C Hamano wrote:\n> \n> Double Damn!! Three people working on the same piece of code.\n\nThe good news is that I'm pushing out a linux-2.6.13-rc1 release, and will \ngo to bed after that, so I'm done for the day. No more races with me ;)\n\n\t\tLinus\n"},{"id":"5405","messageId":"7v4qbhk861.fsf_-_@assigned-by-dhcp.cox.net","threadId":"1042","inReplyTo":"7vacl9k8iz.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Adjust t5300 test for unpack-objects change","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-06-29T06:07:34Z","receivedAt":"2005-06-29T06:07:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"It now always read from standard input and rejects non-flag\narguments.\n\nSigned-off-by: Junio C Hamano \n---\ncd /opt/packrat/playpen/public/in-place/git/git.junio/\njit-diff\n# - linus: Fix packed_delta_info() that was broken by the delta header packing change\n# + (working tree)\ndiff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh\n--- a/t/t5300-pack-object.sh\n+++ b/t/t5300-pack-object.sh\n@@ -45,7 +45,8 @@ test_expect_success \\\n 'GIT_OBJECT_DIRECTORY=.git2/objects &&\n export GIT_OBJECT_DIRECTORY &&\n git-init-db &&\n- git-unpack-objects test-1'\n+ git-unpack-objects -n