Re: [PATCH v7 12/13] pack-objects: shrink delta_size field in struct object_entry
On Fri, Mar 30, 2018 at 11:24 PM, Jeff King wrote: > How come this doesn't get a pdata->oe_delta_size_limit like we have > pdata->oe_size_limit? Would we want a matching > $GIT_TEST_OE_DELTA_SIZE_BITS to test it, too? Nope. This changes how the delta chain is formed (e.g. produces shorter chains) and apparently some tests rely on that, like t5303. -- Duy
Re: [PATCH v7 12/13] pack-objects: shrink delta_size field in struct object_entry
On Fri, Mar 30, 2018 at 11:24 PM, Jeff King wrote: > On Sat, Mar 24, 2018 at 07:33:52AM +0100, Nguyễn Thái Ngọc Duy wrote: >> @@ -2004,10 +2006,12 @@ static int try_delta(struct unpacked *trg, struct >> unpacked *src, >> delta_buf = create_delta(src->index, trg->data, trg_size, &delta_size, >> max_size); >> if (!delta_buf) >> return 0; >> + if (delta_size >= (1 << OE_DELTA_SIZE_BITS)) >> + return 0; > > This is the other spot that needs to be "1U". > > How come this doesn't get a pdata->oe_delta_size_limit like we have > pdata->oe_size_limit? Would we want a matching > $GIT_TEST_OE_DELTA_SIZE_BITS to test it, too? Probably. This change does not look as risky as the others (no complicated fallback). But without $GIT_TEST_OE_DELTA_SIZE_BITS it's hard to know how the new code reacts when we get over the limit. I will add it. -- Duy
Re: [PATCH v7 12/13] pack-objects: shrink delta_size field in struct object_entry
On Sat, Mar 24, 2018 at 07:33:52AM +0100, Nguyễn Thái Ngọc Duy wrote: > Allowing a delta size of 64 bits is crazy. Shrink this field down to > 31 bits with one overflow bit. > > If we find an existing delta larger than 2GB, we do not cache > delta_size at all and will get the value from oe_size(), potentially > from disk if it's larger than 4GB. Since we have a fallback, we can put this slider wherever we want. Probably something like 20 bits would be plenty, if we ever needed to squeeze in a few more small-bit items. > @@ -2004,10 +2006,12 @@ static int try_delta(struct unpacked *trg, struct > unpacked *src, > delta_buf = create_delta(src->index, trg->data, trg_size, &delta_size, > max_size); > if (!delta_buf) > return 0; > + if (delta_size >= (1 << OE_DELTA_SIZE_BITS)) > + return 0; This is the other spot that needs to be "1U". How come this doesn't get a pdata->oe_delta_size_limit like we have pdata->oe_size_limit? Would we want a matching $GIT_TEST_OE_DELTA_SIZE_BITS to test it, too? -Peff
