{"thread":{"id":"7537","subject":"[PATCH 08/13] prevent try_delta from using objects not in pack","startedAt":"2007-04-05T22:35:12Z","lastAt":"2007-04-06T18:17:21Z","messageCount":8,"participants":["Dana How","Junio C Hamano","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"38749","messageId":"56b7f5510704051535ya98e86eu7f786f118c4fe229@mail.gmail.com","threadId":"7537","inReplyTo":null,"subject":"[PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-04-05T22:35:12Z","receivedAt":"2007-04-05T22:35:12Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"---\n builtin-pack-objects.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n\n\nFrom 7d510f82b1e6acccc25596052abf3c5f1961ebcc Mon Sep 17 00:00:00 2001\nFrom: Dana How \nDate: Thu, 5 Apr 2007 14:04:59 -0700\nSubject: [PATCH 08/13] prevent try_delta from using objects not in pack\n\n---\n builtin-pack-objects.c | 4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex 9eb5fd6..37b0150 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -1251,6 +1251,10 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n \tif (trg_entry->type != src_entry->type)\n \t\treturn -1;\n \n+\t/* Don't try deltas involving already/non written objects */\n+\tif (trg_entry->no_write || src_entry->no_write)\n+\t\treturn -1;\n+\n \t/* We do not compute delta to *create* objects we are not\n \t * going to pack.\n \t */\n-- \n1.5.1.rc2.18.g9c88-dirty\n\n"},{"id":"38759","messageId":"7v8xd6mni1.fsf@assigned-by-dhcp.cox.net","threadId":"7537","inReplyTo":"56b7f5510704051535ya98e86eu7f786f118c4fe229@mail.gmail.com","subject":"Re: [PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-05T23:04:38Z","receivedAt":"2007-04-05T23:04:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Dana How\" writes:\n\n> Subject: [PATCH 08/13] prevent try_delta from using objects not in pack\n>\n> ---\n> builtin-pack-objects.c | 4 ++++\n> 1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\n> index 9eb5fd6..37b0150 100644\n> --- a/builtin-pack-objects.c\n> +++ b/builtin-pack-objects.c\n> @@ -1251,6 +1251,10 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,\n> \tif (trg_entry->type != src_entry->type)\n> \t\treturn -1;\n> \n> +\t/* Don't try deltas involving already/non written objects */\n> +\tif (trg_entry->no_write || src_entry->no_write)\n> +\t\treturn -1;\n> +\n> \t/* We do not compute delta to *create* objects we are not\n> \t * going to pack.\n> \t */\n\nI think trg_entry->no_write case should return -1 (meaning:\ndoesn't delta with this source, nor any other sources you would\nthrow at try_delta()), but shouldn't src_entry->no_write case\nreturn 0 (meaning: does not delta with this source, but do not\ngive up -- other source candidate you have might be usable)?\n"},{"id":"38769","messageId":"alpine.LFD.0.98.0704052100420.28181@xanadu.home","threadId":"7537","inReplyTo":"56b7f5510704051535ya98e86eu7f786f118c4fe229@mail.gmail.com","subject":"Re: [PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-06T01:02:46Z","receivedAt":"2007-04-06T01:02:46Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"\nWhat is the purpose of this patch?\n\nThe try_delta() function is called with all objects before any object is \nwritten to a pack to find out how to deltify objects upfront.\n\n\nNicolas\n"},{"id":"38773","messageId":"56b7f5510704051928l5d1f5a02yccc8e57c222e5f64@mail.gmail.com","threadId":"7537","inReplyTo":"alpine.LFD.0.98.0704052100420.28181@xanadu.home","subject":"Re: [PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-04-06T02:28:20Z","receivedAt":"2007-04-06T02:28:20Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 4/5/07, Nicolas Pitre wrote:\n> What is the purpose of this patch?\n>\n> The try_delta() function is called with all objects before any object is\n> written to a pack to find out how to deltify objects upfront.\n\nI set no_write for 2 different reasons in the patchset.\n(1) When the blob is too big (--blob-limit) and will never be written.\n(2) When the blob has been written to a previous, finished pack.\n\nYou're correct that this patch will never see condition (2).\n\nI think my repository statistics are a little unusual.\nPerhaps I'm getting ahead of myself here,\nbut I also wanted to experiment with writing all blobs to one set\nof packs, and all trees, commits, and tags to another set\n(but probably just one small pack).\nI would use no_write for that and it would matter here.\nBut I haven't run any experiments yet so I don't know\nif it makes any difference. I have seen related discussions\non the mailing list so I know this isn't a new idea.\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"38776","messageId":"alpine.LFD.0.98.0704052320560.28181@xanadu.home","threadId":"7537","inReplyTo":"56b7f5510704051928l5d1f5a02yccc8e57c222e5f64@mail.gmail.com","subject":"Re: [PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-06T03:27:14Z","receivedAt":"2007-04-06T03:27:14Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Apr 2007, Dana How wrote:\n\n> On 4/5/07, Nicolas Pitre wrote:\n> > What is the purpose of this patch?\n> > \n> > The try_delta() function is called with all objects before any object is\n> > written to a pack to find out how to deltify objects upfront.\n> \n> I set no_write for 2 different reasons in the patchset.\n> (1) When the blob is too big (--blob-limit) and will never be written.\n> (2) When the blob has been written to a previous, finished pack.\n> \n> You're correct that this patch will never see condition (2).\n\nGiven that I proposed another way for big blobs in my previous email, \nthen (1) should not be needed either.\n\n> I think my repository statistics are a little unusual.\n> Perhaps I'm getting ahead of myself here,\n> but I also wanted to experiment with writing all blobs to one set\n> of packs, and all trees, commits, and tags to another set\n> (but probably just one small pack).\n> I would use no_write for that and it would matter here.\n\nAgain you should simply _not_ add objects you don't want to the list \ninstead of adding them and marking them as unwanted.\n\n\nNicolas\n"},{"id":"38777","messageId":"56b7f5510704052047r30166e53kffbdd8bf71885264@mail.gmail.com","threadId":"7537","inReplyTo":"alpine.LFD.0.98.0704052320560.28181@xanadu.home","subject":"Re: [PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-04-06T03:47:50Z","receivedAt":"2007-04-06T03:47:50Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 4/5/07, Nicolas Pitre wrote:\n> On Thu, 5 Apr 2007, Dana How wrote:\n> > On 4/5/07, Nicolas Pitre wrote:\n> > > What is the purpose of this patch?\n> > >\n> > > The try_delta() function is called with all objects before any object is\n> > > written to a pack to find out how to deltify objects upfront.\n> >\n> > I set no_write for 2 different reasons in the patchset.\n> > (1) When the blob is too big (--blob-limit) and will never be written.\n> > (2) When the blob has been written to a previous, finished pack.\n> >\n> > You're correct that this patch will never see condition (2).\n>\n> Given that I proposed another way for big blobs in my previous email,\n> then (1) should not be needed either.\n>\n> > I think my repository statistics are a little unusual.\n> > Perhaps I'm getting ahead of myself here,\n> > but I also wanted to experiment with writing all blobs to one set\n> > of packs, and all trees, commits, and tags to another set\n> > (but probably just one small pack).\n> > I would use no_write for that and it would matter here.\n>\n> Again you should simply _not_ add objects you don't want to the list\n> instead of adding them and marking them as unwanted.\n\nAgreed, the marking just required less code change at the time.\nI would like not to have to add nr_skipped and nr_actual.\n\nCurrently we have get_object_list -> traverse_commit_list ->\nshow_{commit,object} -> add_object_entry , which is all\ncalled way before get_object_details -> check_object -> sha1_object_info .\nCan I safely move the sha1_object_info calll earlier into\nadd_object_entry so I will know the size for pruning?\n\nIn your other email you mention memory consumption due to object_entry.\nThis structure could benefit somewhat from some attention beyond removing\nmy no_write.(e.g. int->short for .depth and .delta_limit; int->enum\nfor .preferred_base). Perhaps for later.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"},{"id":"38791","messageId":"alpine.LFD.0.98.0704060859230.28181@xanadu.home","threadId":"7537","inReplyTo":"56b7f5510704052047r30166e53kffbdd8bf71885264@mail.gmail.com","subject":"Re: [PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-04-06T14:59:39Z","receivedAt":"2007-04-06T14:59:39Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 5 Apr 2007, Dana How wrote:\n\n> Currently we have get_object_list -> traverse_commit_list ->\n> show_{commit,object} -> add_object_entry , which is all\n> called way before get_object_details -> check_object -> sha1_object_info .\n> Can I safely move the sha1_object_info calll earlier into\n> add_object_entry so I will know the size for pruning?\n\nI think so, yes. And actually this call to sha1_object_info() has been \nbothering me for a while.\n\nThere are currently two ways to get the list of objects to pack: one is \nfrom stdin where objects are listed with their SHA1's, the other one \nuses the internal revision walking code. In the later case we _already_ \nhave the information that sha1_object_info() later provides making it \nrather wasteful by forcing a second object header parsing.\n\nHowever... aren't you more interested in the _compressed_ blob size than \nits raw size?\n\n> In your other email you mention memory consumption due to object_entry.\n> This structure could benefit somewhat from some attention beyond removing\n> my no_write.(e.g. int->short for .depth and .delta_limit; int->enum\n> for .preferred_base). Perhaps for later.\n\nSure.\n\n\nNicolas\n"},{"id":"38793","messageId":"56b7f5510704061117q2a833904g130246c164dd8e3f@mail.gmail.com","threadId":"7537","inReplyTo":"alpine.LFD.0.98.0704060859230.28181@xanadu.home","subject":"Re: [PATCH 08/13] prevent try_delta from using objects not in pack","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-04-06T18:17:21Z","receivedAt":"2007-04-06T18:17:21Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 4/6/07, Nicolas Pitre wrote:\n> On Thu, 5 Apr 2007, Dana How wrote:\n> > Currently we have get_object_list -> traverse_commit_list ->\n> > show_{commit,object} -> add_object_entry , which is all\n> > called way before get_object_details -> check_object -> sha1_object_info .\n> > Can I safely move the sha1_object_info calll earlier into\n> > add_object_entry so I will know the size for pruning?\n>\n> I think so, yes. And actually this call to sha1_object_info() has been\n> bothering me for a while.\n>\n> There are currently two ways to get the list of objects to pack: one is\n> from stdin where objects are listed with their SHA1's, the other one\n> uses the internal revision walking code. In the later case we _already_\n> have the information that sha1_object_info() later provides making it\n> rather wasteful by forcing a second object header parsing.\nPerhaps I'll add the early call (or actually, re-use the info if I can find it),\nand change the second call to only happen if the size is zero,\nsince that's how it starts out in the current add_object_entry.\n\n> However... aren't you more interested in the _compressed_ blob size than\n> its raw size?\nYes! So I am leaning towards disposing of --blob-limit anyway,\nwhich was in part a poor response to a bug in --pack-limit.\nPlease see the other email I sent out so far today.\n\nThanks,\n-- \nDana L. How danahow@gmail.com +1 650 804 5991 cell\n"}]}