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

Re: [PATCH 2/5] gc: convert to using the_hash_algo

From
Jeff King <peff@peff.net>
Date
Mar 14, 2019, 00:26 UTC
Message-ID
<20190314002648.GG31968@sigill.intra.peff.net>
In-Reply-To
<20190313235439.30439-3-avarab@gmail.com>
On Thu, Mar 14, 2019 at 12:54:36AM +0100, Ævar Arnfjörð Bjarmason wrote:
Show 8 quoted lines
> There's been a lot of changing of the hardcoded "40" values to
> the_hash_algo->hexsz, but we've so far missed this one where we
> hardcoded 38 for the loose object file length.
> 
> This is because a SHA-1 like abcde[...] gets turned into
> objects/ab/cde[...]. There's no reason to suppose the same won't be
> the case for SHA-256, and reading between the lines in
> hash-function-transition.txt the format is planned to be the same.
Yep, makes sense.
Show 9 quoted lines
> However, we may want to modify this code for the hash function
> transition. There's a potential pathological case here where we'll
> only consider the loose objects for the currently active hash, but
> objects for that hash will share a directory storage with the other
> hash.
> 
> Thus we could theoretically have 1k SHA-1 loose objects, and say 1
> million SHA-256 objects, and not notice because we're currently using
> SHA-1.

I agree that we may end up needing to touch this, but I think this patch doesn't make anything worse in that respect (and likely makes it better, since we at least know this "38" is supposed to be a hash).

Show 10 quoted lines
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 8c2312681c..9c2c63276d 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -156,6 +156,8 @@ static int too_many_loose_objects(void)
>  	int auto_threshold;
>  	int num_loose = 0;
>  	int needed = 0;
> +	const unsigned hexsz = the_hash_algo->hexsz;
> +	const unsigned hexsz_loose = hexsz - 2;

It doesn't look like hexsz gets used anywhere else; is it worth having the extra variable? (Admittedly this quite a nit).

-Peff
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 50 of 60 in “BUG: Race condition due to reflog expiry in "gc"”
  1. Ævar Arnfjörð BjarmasonMar 12, 2019
  2. Junio C HamanoMar 13, 2019
  3. Ævar Arnfjörð BjarmasonMar 13, 2019
  4. Jeff KingMar 13, 2019
  5. Ævar Arnfjörð BjarmasonMar 13, 2019
  6. 0/5 gc: minor code cleanup + contention fixesÆvar Arnfjörð Bjarmason, Mar 13, 2019
  7. 0/7 gc: minor code cleanup + contention fixesÆvar Arnfjörð Bjarmason, Mar 14, 2019
  8. 0/8 gc: minor code cleanup + contention fixesÆvar Arnfjörð Bjarmason, Mar 15, 2019
  9. Junio C HamanoMar 18, 2019
  10. 0/7 gc: tests and handle reflog expire configÆvar Arnfjörð Bjarmason, Mar 28, 2019
  11. 1/7 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 28, 2019
  12. 2/7 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 28, 2019
  13. 3/7 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 28, 2019
  14. 5/7 reflog tests: test for the "points nowhere" warningÆvar Arnfjörð Bjarmason, Mar 28, 2019
  15. 4/7 reflog tests: make use of "test_config" idiomÆvar Arnfjörð Bjarmason, Mar 28, 2019
  16. 7/7 gc: handle & check gc.reflogExpire configÆvar Arnfjörð Bjarmason, Mar 28, 2019
  17. 6/7 reflog tests: assert lack of early exit with expiry="never"Ævar Arnfjörð Bjarmason, Mar 28, 2019
  18. 1/8 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 15, 2019
  19. 2/8 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 15, 2019
  20. 3/8 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 15, 2019
  21. 4/8 reflog tests: make use of "test_config" idiomÆvar Arnfjörð Bjarmason, Mar 15, 2019
  22. 5/8 reflog tests: test for the "points nowhere" warningÆvar Arnfjörð Bjarmason, Mar 15, 2019
  23. 6/8 reflog tests: assert lack of early exit with expiry="never"Ævar Arnfjörð Bjarmason, Mar 15, 2019
  24. Jeff KingMar 19, 2019
  25. 7/8 gc: handle & check gc.reflogExpire configÆvar Arnfjörð Bjarmason, Mar 15, 2019
  26. 8/8 reflog expire: don't assert the OID when locking refsÆvar Arnfjörð Bjarmason, Mar 15, 2019
  27. Jeff KingMar 19, 2019
  28. 1/7 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 14, 2019
  29. 2/7 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 14, 2019
  30. Duy NguyenMar 15, 2019
  31. Ævar Arnfjörð BjarmasonMar 15, 2019
  32. Johannes SchindelinMar 15, 2019
  33. 3/7 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 14, 2019
  34. 4/7 reflog tests: make use of "test_config" idiomÆvar Arnfjörð Bjarmason, Mar 14, 2019
  35. 5/7 reflog: exit early if there's no work to doÆvar Arnfjörð Bjarmason, Mar 14, 2019
  36. Duy NguyenMar 15, 2019
  37. Ævar Arnfjörð BjarmasonMar 15, 2019
  38. 7/7 reflog expire: don't assert the OID when locking refsÆvar Arnfjörð Bjarmason, Mar 14, 2019
  39. Duy NguyenMar 15, 2019
  40. Ævar Arnfjörð BjarmasonMar 15, 2019
  41. 6/7 gc: don't run "reflog expire" when keeping reflogsÆvar Arnfjörð Bjarmason, Mar 14, 2019
  42. Duy NguyenMar 15, 2019
  43. Ævar Arnfjörð BjarmasonMar 15, 2019
  44. Duy NguyenMar 15, 2019
  45. Johannes SchindelinMar 15, 2019
  46. Junio C HamanoMar 18, 2019
  47. 1/5 gc: remove redundant check for gc_auto_thresholdÆvar Arnfjörð Bjarmason, Mar 13, 2019
  48. Jeff KingMar 14, 2019
  49. 2/5 gc: convert to using the_hash_algoÆvar Arnfjörð Bjarmason, Mar 13, 2019
  50. Jeff KingMar 14, 2019
  51. 3/5 gc: refactor a "call me once" patternÆvar Arnfjörð Bjarmason, Mar 13, 2019
  52. Jeff KingMar 14, 2019
  53. 4/5 gc: don't run "reflog expire" when keeping reflogsÆvar Arnfjörð Bjarmason, Mar 13, 2019
  54. Jeff KingMar 14, 2019
  55. Junio C HamanoMar 14, 2019
  56. Jeff KingMar 14, 2019
  57. 5/5 reflog expire: don't assert the OID when locking refsÆvar Arnfjörð Bjarmason, Mar 13, 2019
  58. Jeff KingMar 14, 2019
  59. Jeff KingMar 14, 2019
  60. Ævar Arnfjörð BjarmasonMar 21, 2019

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.