Re: [PATCH v4 11/11] pack-objects.h: reorder members to shrink struct object_entry
On Fri, Mar 16, 2018 at 10:02 PM, Junio C Hamano wrote:
> Nguyễn Thái Ngọc Duy writes:
>
>> Previous patches leave lots of holes and padding in this struct. This
>> patch reorders the members and shrinks the struct down to 80 bytes
>> (from 136 bytes, before any field shrinking is done) with 16 bits to
>> spare (and a couple more in in_pack_header_size when we really run out
>> of bits).
>
> Nice.
>
> I am wondering if we need some conditional code for 32-bit platform.
> For example, you have uint32_t field and do things like this:
>
> static inline int oe_size_less_than(const struct object_entry *e,
> unsigned long limit)
> {
> if (e->size_valid)
> return e->size_ < limit;
> if (limit > maximum_unsigned_value_of_type(uint32_t))
> return 1;
> return oe_size(e) < limit;
> }
>
> Do we and compilers do the right thing when your ulong is uint32_t?
Another good point. My 32-bit build does complain
In file included from builtin/pack-objects.c:20:0:
./pack-objects.h: In function ?oe_size_less_than?:
./pack-objects.h:282:12: error: comparison is always false due to
limited range of data type [-Werror=type-limits]
if (limit > maximum_unsigned_value_of_type(uint32_t))
^
--
Duy
Re: [PATCH v4 11/11] pack-objects.h: reorder members to shrink struct object_entry
Nguyễn Thái Ngọc Duy writes:
> Previous patches leave lots of holes and padding in this struct. This
> patch reorders the members and shrinks the struct down to 80 bytes
> (from 136 bytes, before any field shrinking is done) with 16 bits to
> spare (and a couple more in in_pack_header_size when we really run out
> of bits).
Nice.
I am wondering if we need some conditional code for 32-bit platform.
For example, you have uint32_t field and do things like this:
static inline int oe_size_less_than(const struct object_entry *e,
unsigned long limit)
{
if (e->size_valid)
return e->size_ < limit;
if (limit > maximum_unsigned_value_of_type(uint32_t))
return 1;
return oe_size(e) < limit;
}
Do we and compilers do the right thing when your ulong is uint32_t?
[PATCH v4 11/11] pack-objects.h: reorder members to shrink struct object_entry
Previous patches leave lots of holes and padding in this struct. This
patch reorders the members and shrinks the struct down to 80 bytes
(from 136 bytes, before any field shrinking is done) with 16 bits to
spare (and a couple more in in_pack_header_size when we really run out
of bits).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
pack-objects.h | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/pack-objects.h b/pack-objects.h
index f430d938c6..0fa0c83294 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -70,35 +70,36 @@ enum dfs_state {
*/
struct object_entry {
struct pack_idx_entry idx;
- /* object uncompressed size _if_ size_valid is true */
- uint32_t size_;
- unsigned size_valid:1;
- unsigned in_pack_idx:OE_IN_PACK_BITS; /* already in pack */
+ void *delta_data; /* cached delta (uncompressed) */
off_t in_pack_offset;
+ uint32_t hash; /* name hint hash */
+ uint32_t size_; /* object uncompressed size _if_ size_valid is true */
uint32_t delta_idx; /* delta base object */
uint32_t delta_child_idx; /* deltified objects who bases me */
uint32_t delta_sibling_idx; /* other deltified objects who
* uses the same base as me
*/
- void *delta_data; /* cached delta (uncompressed) */
uint32_t delta_size_:OE_DELTA_SIZE_BITS; /* delta data size
(uncompressed) */
uint32_t delta_size_valid:1;
+ unsigned in_pack_idx:OE_IN_PACK_BITS; /* already in pack */
+ unsigned size_valid:1;
unsigned z_delta_size:OE_Z_DELTA_BITS;
+ unsigned type_valid:1;
unsigned type_:TYPE_BITS;
unsigned in_pack_type:TYPE_BITS; /* could be delta */
- unsigned type_valid:1;
- uint32_t hash; /* name hint hash */
- unsigned char in_pack_header_size;
unsigned preferred_base:1; /*
* we do not pack this, but is available
* to be used as the base object to delta
* objects against.
*/
unsigned no_try_delta:1;
+ unsigned char in_pack_header_size;
unsigned tagged:1; /* near the very tip of refs */
unsigned filled:1; /* assigned write-order */
unsigned dfs_state:OE_DFS_STATE_BITS;
unsigned depth:OE_DEPTH_BITS;
+
+ /* size: 80, bit_padding: 16 bits */
};
struct packing_data {
--
2.16.2.903.gd04caf5039
