Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Wed, Mar 21, 2018 at 9:03 AM, Jeff King wrote: > On Tue, Mar 20, 2018 at 07:08:07PM +0100, Duy Nguyen wrote: > >> BTW can you apply this patch? This broken && chain made me think the >> problem was in the next test. It would have saved me lots of time if I >> saw this "BUG" line

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Tue, Mar 20, 2018 at 07:08:07PM +0100, Duy Nguyen wrote: > BTW can you apply this patch? This broken && chain made me think the > problem was in the next test. It would have saved me lots of time if I > saw this "BUG" line coming from the previous test. > > -- 8< -- > Subject: [PATCH] t9300:

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

Duy Nguyen writes: > This "size" field contains the delta size if the in-pack object is a > delta. So blindly falling back to object_sha1_info() which returns the > canonical object size is definitely wrong. Yup. Also we need to be careful when going back to the packfile to

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Mon, Mar 19, 2018 at 5:43 PM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> +static inline void oe_set_size(struct object_entry *e, >> +unsigned long size) >> +{ >> + e->size_ = size; >> + e->size_valid =

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Mon, Mar 19, 2018 at 01:10:49PM -0700, Junio C Hamano wrote: > > ... I was trying to exercise this > > code the other day by reducing size_ field down to 4 bits, and a > > couple tests broke but I still don't understand how. > > Off by one? Two or more copies of the same objects available

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

Duy Nguyen writes: > There is a difference. For sizes smaller than 2^32, whatever you > pass to oe_set_size() will be returned by oe_size(), > consistently. It does not matter if this size is "good" If > it's different, oe_size() will return something else other than >

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

Duy Nguyen writes: > This is why I do "size_valid = size_ == size". In my private build, I > reduced size_ to less than 32 bits and change the "fits_in_32bits" > function to do something like > > int fits_in_32bits(unsigned long size) > { > struct object_entry e; > e.size_ =

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Mon, Mar 19, 2018 at 7:29 PM, Junio C Hamano wrote: + if (!e->size_valid) { + unsigned long real_size; + + if (sha1_object_info(e->idx.oid.hash, _size) < 0 || + size != real_size) +

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Mon, Mar 19, 2018 at 5:43 PM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> +static inline void oe_set_size(struct object_entry *e, >> +unsigned long size) >> +{ >> + e->size_ = size; >> + e->size_valid =

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

Nguyễn Thái Ngọc Duy writes: > +static inline void oe_set_size(struct object_entry *e, > +unsigned long size) > +{ > + e->size_ = size; > + e->size_valid = e->size_ == size; A quite similar comment as my earlier one applies here. I wonder

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Mon, Mar 19, 2018 at 5:19 PM, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duy writes: > >> +static inline int oe_fits_in_32bits(unsigned long limit) >> +{ >> + uint32_t truncated_limit = (uint32_t)limit; >> + >> + return limit == truncated_limit; >>

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

Nguyễn Thái Ngọc Duy writes: > +static inline int oe_fits_in_32bits(unsigned long limit) > +{ > + uint32_t truncated_limit = (uint32_t)limit; > + > + return limit == truncated_limit; > +} I do not think it is worth a reroll (there only are a few callsites), but the

Re: [PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

On Sun, Mar 18 2018, Nguyễn Thái Ngọc Duy jotted: > It's very very rare that an uncompressedd object is larger than 4GB So this went from a typo of "uncompressd" in v5 to "uncompressedd", needs one less "d": "uncompressed".

[PATCH v6 09/11] pack-objects: shrink size field in struct object_entry

It's very very rare that an uncompressedd object is larger than 4GB (partly because Git does not handle those large files very well to begin with). Let's optimize it for the common case where object size is smaller than this limit. Shrink size field down to 32 bits [1] and one overflow bit. If