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

Re: [PATCH 2/5] t6421: add tests checking for excessive object downloads during merge

From
Derrick Stolee <stolee@gmail.com>
Date
Jun 9, 2021, 21:16 UTC
Message-ID
<7fb76c82-bc54-01c1-451f-7dc05177d58f@gmail.com>
In-Reply-To
<6462bb63310dbc889a3442951ef261bc7fd7e0dc.1622856485.git.gitgitgadget@gmail.com>
On 6/4/2021 9:28 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
...
Show 12 quoted lines
> +test_expect_merge_algorithm failure failure 'Objects downloaded for single relevant rename' '
> +	test_setup_repo &&
> +	git clone --sparse --filter=blob:none "file://$(pwd)/server" objects-single &&
> +	(
> +		cd objects-single &&
> +
> +		git rev-list --objects --all --missing=print |
> +			grep '\?' >missing-objects-before &&
> +
> +		git checkout -q origin/A &&
> +
> +		GIT_TRACE2_PERF="$(pwd)/trace.output" git -c merge.directoryRenames=true merge --no-stat --no-progress origin/B-single &&
nit: this long line could be split up with \ marks.
Show 5 quoted lines
> +
> +		# Check the number of objects we reported we would fetch
> +		cat >expect <<-EOF &&
> +		 ..........fetch_count:2
> +		 ......fetch_count:1

I'm concerned that the number of dots here could change for various reasons (e.g. adding regions). Could the 'cut' below cut everything after the dots, so we only care about "fetch_count:X"?

Show 18 quoted lines
> +		EOF
> +		grep fetch_count trace.output | cut -d "|" -f 9 >actual &&
> +		test_cmp expect actual &&
> +
> +		# Check the number of fetch commands exec-ed
> +		grep d0.*fetch.negotiationAlgorithm trace.output >fetches &&
> +		test_line_count = 2 fetches &&
> +
> +		git rev-list --objects --all --missing=print |
> +			grep ^? >missing-objects-after &&
> +		test_cmp missing-objects-before missing-objects-after |
> +			grep "^[-+]?" >found-and-new-objects &&
> +		# We should not have any NEW missing objects
> +		! grep ^+ found-and-new-objects &&
> +		# Fetched 2+1=3 objects, so should have 3 fewer missing objects
> +		test_line_count = 3 found-and-new-objects
> +	)
> +'

Generally, this idea of using trace2 to determine internal activity that affects performance is a good one. The balance to strike is to ensure that future changes don't suffer too much due to the how rigid the tests are. The dots are the only example of a way to relax these tests a bit more.

Thanks, -Stolee

Previous: Elijah Newren via GitGitGadgetNext: Elijah Newren via GitGitGadget
Message 7 of 30 in “Optimization batch 13: partial clone optimizations for merge-ort”
  1. 0/5 Optimization batch 13: partial clone optimizations for merge-ortElijah Newren via GitGitGadget, Jun 5, 2021
  2. 1/5 promisor-remote: output trace2 statistics for number of objects fetchedElijah Newren via GitGitGadget, Jun 5, 2021
  3. Derrick StoleeJun 9, 2021
  4. 5/5 merge-ort: add prefetching for content mergesElijah Newren via GitGitGadget, Jun 5, 2021
  5. 3/5 diffcore-rename: allow different missing_object_cb functionsElijah Newren via GitGitGadget, Jun 5, 2021
  6. 2/5 t6421: add tests checking for excessive object downloads during mergeElijah Newren via GitGitGadget, Jun 5, 2021
  7. Derrick StoleeJun 9, 2021
  8. 4/5 diffcore-rename: use a different prefetch for basename comparisonsElijah Newren via GitGitGadget, Jun 5, 2021
  9. 0/5 Optimization batch 13: partial clone optimizations for merge-ortElijah Newren via GitGitGadget, Jun 15, 2021
  10. 1/5 promisor-remote: output trace2 statistics for number of objects fetchedElijah Newren via GitGitGadget, Jun 15, 2021
  11. 2/5 t6421: add tests checking for excessive object downloads during mergeElijah Newren via GitGitGadget, Jun 15, 2021
  12. Junio C HamanoJun 17, 2021
  13. 4/5 diffcore-rename: use a different prefetch for basename comparisonsElijah Newren via GitGitGadget, Jun 15, 2021
  14. 3/5 diffcore-rename: allow different missing_object_cb functionsElijah Newren via GitGitGadget, Jun 15, 2021
  15. 5/5 merge-ort: add prefetching for content mergesElijah Newren via GitGitGadget, Jun 15, 2021
  16. Junio C HamanoJun 17, 2021
  17. Elijah NewrenJun 22, 2021
  18. Derrick StoleeJun 16, 2021
  19. Junio C HamanoJun 17, 2021
  20. 0/5 Optimization batch 13: partial clone optimizations for merge-ortElijah Newren via GitGitGadget, Jun 22, 2021
  21. 1/5 promisor-remote: output trace2 statistics for number of objects fetchedElijah Newren via GitGitGadget, Jun 22, 2021
  22. 3/5 diffcore-rename: allow different missing_object_cb functionsElijah Newren via GitGitGadget, Jun 22, 2021
  23. 2/5 t6421: add tests checking for excessive object downloads during mergeElijah Newren via GitGitGadget, Jun 22, 2021
  24. 4/5 diffcore-rename: use a different prefetch for basename comparisonsElijah Newren via GitGitGadget, Jun 22, 2021
  25. 5/5 merge-ort: add prefetching for content mergesElijah Newren via GitGitGadget, Jun 22, 2021
  26. Derrick StoleeJun 22, 2021
  27. Elijah NewrenJun 22, 2021
  28. Derrick StoleeJun 23, 2021
  29. Elijah NewrenJun 23, 2021
  30. Elijah NewrenJun 23, 2021

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.