git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 00/11] writing out a huge blob to working tree

From
Jeff King <peff@peff.net>
Date
May 18, 2011, 07:08 UTC
Message-ID
<20110518070837.GC27482@sigill.intra.peff.net>
In-Reply-To
<20110518064158.GB27482@sigill.intra.peff.net>
On Wed, May 18, 2011 at 02:41:58AM -0400, Jeff King wrote:
Show 10 quoted lines
> According to perf, though, it's not the increased writes; the slowdown
> is actually from create_pack_revindex, in this call chain:
> 
>  create_pack_revindex
>  find_pack_revindex
>  packed_object_info_detail
>  sha1_object_info_extended
>  istream_source
>  open_istream
>  streaming_write_entry

Part of the problem is that with the current code, all you care about is "Is it loose, packed non-delta, or packed delta?". And packed_object_info_detail will tell you not just whether it's deltafied, but will go to a lot more work to make the revindex. One solution is to let the cheap packed_object_info() report back on delta status, since it's free there, and then we don't have to deal with the revindex at all.

Of course, it may turn out that the extra information is useful if and when open_istream_* actually gets implemented for delta-fied objects.

The patch below implements the cheap "is_delta" check. But it only shaves off a half second (dropping us from 7s to 6.5s). Prior to your patches, we were at 4.5 seconds. So there's still quite a bit of slowdown to figure out.

diff --git a/cache.h b/cache.h
index 39e53c8..829c3a4 100644
--- a/cache.h
+++ b/cache.h
@@ -1026,6 +1026,7 @@ extern int unpack_object_header(struct packed_git *, struct pack_window **, off_
 struct object_info {
 	/* Request */
 	unsigned long *sizep;
+	int *is_deltap;
 	int want_deltainfo;
 
 	/* Response */
diff --git a/sha1_file.c b/sha1_file.c
index 15f1c05..35be909 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1481,7 +1481,7 @@ static off_t get_delta_base(struct packed_git *p,
 
 /* forward declaration for a mutually recursive function */
 static int packed_object_info(struct packed_git *p, off_t offset,
-			      unsigned long *sizep);
+			      unsigned long *sizep, int *is_delta);
 
 static int packed_delta_info(struct packed_git *p,
 			     struct pack_window **w_curs,
@@ -1495,7 +1495,7 @@ static int packed_delta_info(struct packed_git *p,
 	base_offset = get_delta_base(p, w_curs, &curpos, type, obj_offset);
 	if (!base_offset)
 		return OBJ_BAD;
-	type = packed_object_info(p, base_offset, NULL);
+	type = packed_object_info(p, base_offset, NULL, NULL);
 	if (type <= OBJ_NONE) {
 		struct revindex_entry *revidx;
 		const unsigned char *base_sha1;
@@ -1605,7 +1605,7 @@ int packed_object_info_detail(struct packed_git *p,
 }
 
 static int packed_object_info(struct packed_git *p, off_t obj_offset,
-			      unsigned long *sizep)
+			      unsigned long *sizep, int *is_delta)
 {
 	struct pack_window *w_curs = NULL;
 	unsigned long size;
@@ -1619,6 +1619,8 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset,
 	case OBJ_REF_DELTA:
 		type = packed_delta_info(p, &w_curs, curpos,
 					 type, obj_offset, sizep);
+		if (is_delta)
+			*is_delta = 1;
 		break;
 	case OBJ_COMMIT:
 	case OBJ_TREE:
@@ -1626,11 +1628,15 @@ static int packed_object_info(struct packed_git *p, off_t obj_offset,
 	case OBJ_TAG:
 		if (sizep)
 			*sizep = size;
+		if (is_delta)
+			*is_delta = 0;
 		break;
 	default:
 		error("unknown object type %i at offset %"PRIuMAX" in %s",
 		      type, (uintmax_t)obj_offset, p->pack_name);
 		type = OBJ_BAD;
+		if (is_delta)
+			*is_delta = 0;
 	}
 	unuse_pack(&w_curs);
 	return type;
@@ -2130,7 +2136,8 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi)
 	}
 
 	if (!oi->want_deltainfo) {
-		status = packed_object_info(e.p, e.offset, oi->sizep);
+		status = packed_object_info(e.p, e.offset, oi->sizep,
+					    oi->is_deltap);
 	} else {
 		unsigned long size, store_size;
 		unsigned int delta_chain_length;
diff --git a/streaming.c b/streaming.c
index 03c58b2..a2c0e84 100644
--- a/streaming.c
+++ b/streaming.c
@@ -84,10 +84,12 @@ static enum input_source istream_source(const unsigned char *sha1,
 					struct object_info *oi)
 {
 	unsigned long size;
+	int is_delta;
 	int status;
 
 	oi->sizep = &size;
-	oi->want_deltainfo = 1;
+	oi->is_deltap = &is_delta;
+	oi->want_deltainfo = 0;
 
 	status = sha1_object_info_extended(sha1, oi);
 	if (status < 0)
@@ -98,7 +100,7 @@ static enum input_source istream_source(const unsigned char *sha1,
 	case OI_LOOSE:
 		return loose;
 	case OI_PACKED:
-		if (!oi->u.packed.delta && big_file_threshold <= size)
+		if (!is_delta && big_file_threshold <= size)
 			return pack_non_delta;
 		/* fallthru */
 	default:
Previous: Jeff KingNext: Jeff King
Message 27 of 49 in “writing out a huge blob to working tree”
  1. 00/11 writing out a huge blob to working treeJunio C Hamano, May 16, 2011
  2. 01/11 packed_object_info_detail(): do not return a stringJunio C Hamano, May 16, 2011
  3. Thiago FarinaMay 17, 2011
  4. Junio C HamanoMay 17, 2011
  5. 02/11 sha1_object_info_extended(): expose a bit more infoJunio C Hamano, May 16, 2011
  6. 03/11 sha1_object_info_extended(): hint about objects in delta-base cacheJunio C Hamano, May 16, 2011
  7. Shawn PearceMay 16, 2011
  8. 04/11 unpack_object_header(): make it publicJunio C Hamano, May 16, 2011
  9. 05/11 write_entry(): separate two helper functions outJunio C Hamano, May 16, 2011
  10. 06/11 streaming: a new API to read from the object storeJunio C Hamano, May 16, 2011
  11. Jeff KingMay 18, 2011
  12. Junio C HamanoMay 19, 2011
  13. 07/11 streaming_write_entry(): use streaming API in write_entry()Junio C Hamano, May 16, 2011
  14. 08/11 streaming_write_entry(): support files with holesJunio C Hamano, May 16, 2011
  15. Nguyen Thai Ngoc DuyMay 16, 2011
  16. Junio C HamanoMay 16, 2011
  17. Nguyen Thai Ngoc DuyMay 17, 2011
  18. Junio C HamanoMay 17, 2011
  19. Thiago FarinaMay 16, 2011
  20. 09/11 streaming: read non-delta incrementally from a packJunio C Hamano, May 16, 2011
  21. Shawn PearceMay 16, 2011
  22. Junio C HamanoMay 16, 2011
  23. 10/11 sha1_file.c: expose helpers to read loose objectsJunio C Hamano, May 16, 2011
  24. 11/11 streaming: read loose objects incrementallyJunio C Hamano, May 16, 2011
  25. Shawn PearceMay 16, 2011
  26. Jeff KingMay 18, 2011
  27. Jeff KingMay 18, 2011
  28. Jeff KingMay 18, 2011
  29. Junio C HamanoMay 18, 2011
  30. Jeff KingMay 18, 2011
  31. 00/11 writing out a huge blob to working treeJunio C Hamano, May 19, 2011
  32. 01/11 packed_object_info_detail(): do not return a stringJunio C Hamano, May 19, 2011
  33. 02/11 sha1_object_info_extended(): expose a bit more infoJunio C Hamano, May 19, 2011
  34. 03/11 sha1_object_info_extended(): hint about objects in delta-base cacheJunio C Hamano, May 19, 2011
  35. René ScharfeMay 20, 2011
  36. Junio C HamanoMay 21, 2011
  37. 04/11 unpack_object_header(): make it publicJunio C Hamano, May 19, 2011
  38. 05/11 write_entry(): separate two helper functions outJunio C Hamano, May 19, 2011
  39. 06/11 streaming: a new API to read from the object storeJunio C Hamano, May 19, 2011
  40. René ScharfeMay 20, 2011
  41. Junio C HamanoMay 21, 2011
  42. 07/11 streaming_write_entry(): use streaming API in write_entry()Junio C Hamano, May 19, 2011
  43. Junio C HamanoMay 20, 2011
  44. 08/11 streaming_write_entry(): support files with holesJunio C Hamano, May 19, 2011
  45. 09/11 streaming: read non-delta incrementally from a packJunio C Hamano, May 19, 2011
  46. 10/11 sha1_file.c: expose helpers to read loose objectsJunio C Hamano, May 19, 2011
  47. 11/11 streaming: read loose objects incrementallyJunio C Hamano, May 19, 2011
  48. 02/11 interdiffJunio C Hamano, May 19, 2011
  49. Jeff KingMay 19, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.