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

Re: [PATCH 1/1] commit-graph.c: avoid unnecessary tag dereference when merging

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 21, 2020, 05:01 UTC
Message-ID
<xmqqk13e9u7x.fsf@gitster.c.googlers.com>
In-Reply-To
<4c79a9ea909ebff8c0987bcf95692da92e79bda4.1584762087.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 14 quoted lines
> When performing a 'git commit-graph write' with '--split', the
> commit-graph machinery calls 'merge_commit_graph()' after deciding on a
> split strategy to optionally clean up any existing commit-graph
> layers that were made obsolete by the split strategy [1].
>
> At this time, 'merge_commit_graph()' checks each commit that it writes
> into the merged graph to make sure that it still exists in the object
> store.
>
> To do this, it uses 'lookup_commit_reference_gently()', which accepts
> either a commit object, or a tag that refers to a commit. However, since
> all 'oid' arguments passed to this function are from within the
> commit-graphs being merged, we never pass a commit reference, and so any
> time we spend in 'deref_tag()' is wasted.

Ahh, so my question on the cover letter was utterly off the mark. It is that feeding a commit to deref_tag() is unneeded. It is quite surprising to hear that deref_tag() call is _so_ expensive that it wastes 7% of the total cycles.

The patch itself looks good.
Thanks.
Show 19 quoted lines
>  	for (i = 0; i < g->num_commits; i++) {
>  		struct object_id oid;
> -		struct commit *result;
> +		struct commit *result = NULL;
>  
>  		display_progress(ctx->progress, i + 1);
>  
>  		load_oid_from_graph(g, i + offset, &oid);
>  
>  		/* only add commits if they still exist in the repo */
> -		result = lookup_commit_reference_gently(ctx->r, &oid, 1);
> +		if (repo_has_object_file(ctx->r, &oid)) {
> +			result = lookup_commit(ctx->r, &oid);
> +			if (repo_parse_commit(ctx->r, result))
> +				result = NULL;
> +		}
>  
>  		if (result) {
>  			ctx->commits.list[ctx->commits.nr] = result;
Previous: Jeff KingNext: Junio C Hamano
Message 28 of 31 in “commit-graph: avoid unnecessary tag deference when merging”
  1. 0/1 commit-graph: avoid unnecessary tag deference when mergingTaylor Blau, Mar 21, 2020
  2. 1/1 commit-graph.c: avoid unnecessary tag dereference when mergingTaylor Blau, Mar 21, 2020
  3. Jeff KingMar 21, 2020
  4. Taylor BlauMar 21, 2020
  5. Taylor BlauMar 21, 2020
  6. Jeff KingMar 21, 2020
  7. Taylor BlauMar 21, 2020
  8. Jeff KingMar 22, 2020
  9. SZEDER GáborMar 22, 2020
  10. looking up object types quickly, was Re: [PATCH 1/1] commit-graph.c: avoid unnecessary tag dereference when mergingJeff King, Mar 22, 2020
  11. Jeff KingMar 22, 2020
  12. Taylor BlauMar 23, 2020
  13. Taylor BlauMar 22, 2020
  14. Jeff KingMar 24, 2020
  15. Junio C HamanoMar 21, 2020
  16. Derrick StoleeMar 22, 2020
  17. Taylor BlauMar 22, 2020
  18. Derrick StoleeMar 22, 2020
  19. Jeff KingMar 22, 2020
  20. Jeff KingMar 22, 2020
  21. Taylor BlauMar 22, 2020
  22. Jeff KingMar 24, 2020
  23. Taylor BlauMar 24, 2020
  24. Jeff KingMar 27, 2020
  25. Taylor BlauMar 27, 2020
  26. Taylor BlauMar 22, 2020
  27. Jeff KingMar 24, 2020
  28. Junio C HamanoMar 21, 2020
  29. Junio C HamanoMar 21, 2020
  30. Jeff KingMar 21, 2020
  31. Taylor BlauMar 21, 2020

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.