Subject: [PATCH v5 00/11] nd/pack-objects-pack-struct updates

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Sat, 17 Mar 2018 15:10:22 +0100

v5 changes are small enough that the interdiff is pretty self
explanatory (there's also a couple commit msg updates).

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c388d87c3e..fb2aba80bf 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1611,7 +1611,7 @@ static void drop_reused_delta(struct object_entry *entry)
 		/*
 		 * We failed to get the info from this pack for some reason;
 		 * fall back to sha1_object_info, which may find another copy.
-		 * And if that fails, the error will be recorded in entry->type
+		 * And if that fails, the error will be recorded in oe_type(entry)
 		 * and dealt with in prepare_pack().
 		 */
 		oe_set_type(entry, sha1_object_info(entry->idx.oid.hash,
@@ -1968,7 +1968,7 @@ 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 >= maximum_unsigned_value_of_type(uint32_t))
+	if (delta_size >= (1 << OE_DELTA_SIZE_BITS))
 		return 0;
 
 	if (DELTA(trg_entry)) {
@@ -2125,8 +2125,8 @@ static void find_deltas(struct object_entry **list, unsigned *list_size,
 			unsigned long size;
 
 			size = do_compress(&entry->delta_data, DELTA_SIZE(entry));
-			entry->z_delta_size = size;
-			if (entry->z_delta_size == size) {
+			if (size < (1 << OE_Z_DELTA_BITS)) {
+				entry->z_delta_size = size;
 				cache_lock();
 				delta_cache_size -= DELTA_SIZE(entry);
 				delta_cache_size += entry->z_delta_size;
diff --git a/pack-objects.h b/pack-objects.h
index 0fa0c83294..8979289f5f 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -27,14 +27,15 @@ enum dfs_state {
  *
  * basic object info
  * -----------------
- * idx.oid is filled up before delta searching starts. idx.crc32 and
- * is only valid after the object is written out and will be used for
+ * idx.oid is filled up before delta searching starts. idx.crc32 is
+ * only valid after the object is written out and will be used for
  * generating the index. idx.offset will be both gradually set and
  * used in writing phase (base objects get offset first, then deltas
  * refer to them)
  *
- * "size" is the uncompressed object size. Compressed size is not
- * cached (ie. raw data in a pack) but available via revindex.
+ * "size" is the uncompressed object size. Compressed size of the raw
+ * data for an object in a pack is not stored anywhere but is computed
+ * and made available when reverse .idx is made.
  *
  * "hash" contains a path name hash which is used for sorting the
  * delta list and also during delta searching. Once prepare_pack()
@@ -42,16 +43,16 @@ enum dfs_state {
  *
  * source pack info
  * ----------------
- * The (in_pack, in_pack_offset, in_pack_header_size) tuple contains
- * the location of the object in the source pack, with or without
- * header.
+ * The (in_pack, in_pack_offset) tuple contains the location of the
+ * object in the source pack. in_pack_header_size allows quickly
+ * skipping the header and going straight to the zlib stream.
  *
  * "type" and "in_pack_type" both describe object type. in_pack_type
  * may contain a delta type, while type is always the canonical type.
  *
  * deltas
  * ------
- * Delta links (delta, delta_child and delta_sibling) are created
+ * Delta links (delta, delta_child and delta_sibling) are created to
  * reflect that delta graph from the source pack then updated or added
  * during delta searching phase when we find better deltas.
  *
@@ -59,7 +60,7 @@ enum dfs_state {
  * compute_write_order(). "delta" and "delta_size" must remain valid
  * at object writing phase in case the delta is not cached.
  *
- * If a delta is cached in memory and is compressed delta_data points
+ * If a delta is cached in memory and is compressed, delta_data points
  * to the data and z_delta_size contains the compressed size. If it's
  * uncompressed [1], z_delta_size must be zero. delta_size is always
  * the uncompressed size and must be valid even if the delta is not
@@ -274,12 +275,19 @@ static inline unsigned long oe_size(const struct object_entry *e)
 	}
 }
 
+static inline int contains_in_32bits(unsigned long limit)
+{
+	uint32_t truncated_limit = (uint32_t)limit;
+
+	return limit == truncated_limit;
+}
+
 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))
+	if (contains_in_32bits(limit))
 		return 1;
 	return oe_size(e) < limit;
 }
@@ -289,8 +297,8 @@ static inline int oe_size_greater_than(const struct object_entry *e,
 {
 	if (e->size_valid)
 		return e->size_ > limit;
-	if (limit <= maximum_unsigned_value_of_type(uint32_t))
-		return 1;
+	if (contains_in_32bits(limit))
+		return 0;
 	return oe_size(e) > limit;
 }
 
@@ -314,7 +322,7 @@ static inline void oe_set_delta_size(struct packing_data *pack,
 				     unsigned long size)
 {
 	e->delta_size_ = size;
-	e->delta_size_valid =e->delta_size_ == size;
+	e->delta_size_valid = e->delta_size_ == size;
 	if (!e->delta_size_valid && size != oe_size(e))
 		die("BUG: this can only happen in check_object() "
 		    "where delta size is the same as entry size");

-- 
2.17.0.rc0.347.gf9cf61673a