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

[PATCH v2 0/7] pack-bitmap: permute existing namehash values

From
Taylor Blau <me@ttaylorr.com>
Date
Sep 14, 2021, 22:05 UTC
Message-ID
<cover.1631657157.git.me@ttaylorr.com>
In-Reply-To
<cover.1631049462.git.me@ttaylorr.com>

Here is a small-ish rerool of my series to carry forward values from an existing hash-cache when generating multi-pack reachability bitmaps.

Since last time, the performance tests in p5326 were cleaned up in order to produce timings for generating a pack when using a MIDX bitmap with the hash-cache extension.

The test-tool implementation in the second patch was also fixed to print the correct OID for each name-hash. In the previous version, the order we looked up read the hash-cache was according to pack order, when it should be in index order. This bug still allowed the tests to pass, because the two orderings are permutations of one another, so the expected and actual orderings were wrong in the same way.

It is based on the `tb/multi-pack-bitmaps` topic, which graduated to master. Thanks in advance for your review.

Taylor Blau (7):
  t/helper/test-bitmap.c: add 'dump-hashes' mode
  pack-bitmap.c: propagate namehash values from existing bitmaps
  midx.c: respect 'pack.writeBitmapHashcache' when writing bitmaps
  p5326: create missing 'perf-tag' tag
  p5326: don't set core.multiPackIndex unnecessarily
  p5326: generate pack bitmaps before writing the MIDX bitmap
  t5326: test propagating hashcache values
 Documentation/config/pack.txt      |  4 +++
 builtin/multi-pack-index.c         | 21 +++++++++++++++
 midx.c                             |  6 ++++-
 midx.h                             |  1 +
 pack-bitmap.c                      | 41 +++++++++++++++++++++++++-----
 pack-bitmap.h                      |  1 +
 t/helper/test-bitmap.c             | 10 +++++++-
 t/perf/p5326-multi-pack-bitmaps.sh |  8 +++---
 t/t5326-multi-pack-bitmaps.sh      | 32 +++++++++++++++++++++++
 9 files changed, 113 insertions(+), 11 deletions(-)
Range-diff against v1:
1:  918f9b275a ! 1:  4f2b8d9530 t/helper/test-bitmap.c: add 'dump-hashes' mode
    @@ pack-bitmap.c: int test_bitmap_commits(struct repository *r)
     +{
     +	struct bitmap_index *bitmap_git = prepare_bitmap_git(r);
     +	struct object_id oid;
    -+	uint32_t i;
    ++	uint32_t i, index_pos;
     +
     +	if (!bitmap_git->hashes)
     +		goto cleanup;
     +
     +	for (i = 0; i < bitmap_num_objects(bitmap_git); i++) {
    -+		nth_bitmap_object_oid(bitmap_git, &oid, i);
    ++		if (bitmap_is_midx(bitmap_git))
    ++			index_pos = pack_pos_to_midx(bitmap_git->midx, i);
    ++		else
    ++			index_pos = pack_pos_to_index(bitmap_git->pack, i);
    ++
    ++		nth_bitmap_object_oid(bitmap_git, &oid, index_pos);
     +
     +		printf("%s %"PRIu32"\n",
    -+		       oid_to_hex(&oid), get_be32(bitmap_git->hashes + i));
    ++		       oid_to_hex(&oid), get_be32(bitmap_git->hashes + index_pos));
     +	}
     +
     +cleanup:
2:  fa9f5633f6 = 2:  2cd2f3aa5e pack-bitmap.c: propagate namehash values from existing bitmaps
3:  be8f47e13c ! 3:  f0d8f106c2 midx.c: respect 'pack.writeBitmapHashcache' when writing bitmaps
    @@ builtin/multi-pack-index.c: static struct option *add_common_options(struct opti
     +	}
     +
     +	/*
    -+	 * No need to fall-back to 'git_default_config', since this was already
    -+	 * called in 'cmd_multi_pack_index()'.
    ++	 * We should never make a fall-back call to 'git_default_config', since
    ++	 * this was already called in 'cmd_multi_pack_index()'.
     +	 */
     +	return 0;
     +}
-:  ---------- > 4:  a8c6e845ad p5326: create missing 'perf-tag' tag
-:  ---------- > 5:  191922c8f2 p5326: don't set core.multiPackIndex unnecessarily
-:  ---------- > 6:  040bb40548 p5326: generate pack bitmaps before writing the MIDX bitmap
4:  acf3ec60cb ! 7:  fdf71432b3 t5326: test propagating hashcache values
    @@ Commit message
         the configuration to enable the hashcache at all, test that both of
         these function correctly by hardening their behavior with a test.
     
    +    Like the hash-cache in classic single-pack bitmaps, this helps more
    +    proportionally the more up-to-date your bitmap coverage is. When our
    +    bitmap coverage is out-of-date with the ref tips, we spend more time
    +    proportionally traversing, and all of that traversal gets the name-hash
    +    filled in.
    +
    +    But for the up-to-date bitmaps, this helps quite a bit. These numbers
    +    are on git.git, with `pack.threads=1` to help see the difference
    +    reflected in the overall runtime.
    +
    +        Test                            origin/tb/multi-pack-bitmaps   HEAD
    +        -------------------------------------------------------------------------------------
    +        5326.4: simulated clone         1.87(1.80+0.07)                1.46(1.42+0.03) -21.9%
    +        5326.5: simulated fetch         2.66(2.61+0.04)                1.47(1.43+0.04) -44.7%
    +        5326.6: pack to file (bitmap)   2.74(2.62+0.12)                1.89(1.82+0.07) -31.0%
    +
         Signed-off-by: Taylor Blau <me@ttaylorr.com>
     
      ## t/t5326-multi-pack-bitmaps.sh ##
-- 
2.33.0.96.g73915697e6
Previous: Junio C HamanoNext: Taylor Blau
Message 32 of 55 in “pack-bitmap: permute existing namehash values”
  1. 0/4 pack-bitmap: permute existing namehash valuesTaylor Blau, Sep 7, 2021
  2. 1/4 t/helper/test-bitmap.c: add 'dump-hashes' modeTaylor Blau, Sep 7, 2021
  3. Ævar Arnfjörð BjarmasonSep 8, 2021
  4. Taylor BlauSep 8, 2021
  5. 2/4 pack-bitmap.c: propagate namehash values from existing bitmapsTaylor Blau, Sep 7, 2021
  6. 3/4 midx.c: respect 'pack.writeBitmapHashcache' when writing bitmapsTaylor Blau, Sep 7, 2021
  7. Ævar Arnfjörð BjarmasonSep 8, 2021
  8. Taylor BlauSep 8, 2021
  9. Ævar Arnfjörð BjarmasonSep 9, 2021
  10. Ævar Arnfjörð BjarmasonSep 9, 2021
  11. Taylor BlauSep 9, 2021
  12. Ævar Arnfjörð BjarmasonSep 9, 2021
  13. Taylor BlauSep 9, 2021
  14. Taylor BlauSep 9, 2021
  15. Junio C HamanoSep 13, 2021
  16. Taylor BlauSep 14, 2021
  17. 4/4 t5326: test propagating hashcache valuesTaylor Blau, Sep 7, 2021
  18. Ævar Arnfjörð BjarmasonSep 8, 2021
  19. Taylor BlauSep 8, 2021
  20. Ævar Arnfjörð BjarmasonSep 17, 2021
  21. Taylor BlauSep 17, 2021
  22. Ævar Arnfjörð BjarmasonSep 17, 2021
  23. Junio C HamanoSep 13, 2021
  24. Taylor BlauSep 14, 2021
  25. Junio C HamanoSep 14, 2021
  26. Taylor BlauSep 14, 2021
  27. Taylor BlauSep 14, 2021
  28. Jeff KingSep 14, 2021
  29. Taylor BlauSep 14, 2021
  30. Jeff KingSep 14, 2021
  31. Junio C HamanoSep 14, 2021
  32. 0/7 pack-bitmap: permute existing namehash valuesTaylor Blau, Sep 14, 2021
  33. 1/7 t/helper/test-bitmap.c: add 'dump-hashes' modeTaylor Blau, Sep 14, 2021
  34. 2/7 pack-bitmap.c: propagate namehash values from existing bitmapsTaylor Blau, Sep 14, 2021
  35. 3/7 midx.c: respect 'pack.writeBitmapHashcache' when writing bitmapsTaylor Blau, Sep 14, 2021
  36. 4/7 p5326: create missing 'perf-tag' tagTaylor Blau, Sep 14, 2021
  37. Jeff KingSep 16, 2021
  38. Taylor BlauSep 17, 2021
  39. 5/7 p5326: don't set core.multiPackIndex unnecessarilyTaylor Blau, Sep 14, 2021
  40. Jeff KingSep 16, 2021
  41. 6/7 p5326: generate pack bitmaps before writing the MIDX bitmapTaylor Blau, Sep 14, 2021
  42. Jeff KingSep 16, 2021
  43. Taylor BlauSep 17, 2021
  44. 7/7 t5326: test propagating hashcache valuesTaylor Blau, Sep 14, 2021
  45. Jeff KingSep 16, 2021
  46. Jeff KingSep 16, 2021
  47. 0/7 pack-bitmap: permute existing namehash valuesTaylor Blau, Sep 17, 2021
  48. 1/7 t/helper/test-bitmap.c: add 'dump-hashes' modeTaylor Blau, Sep 17, 2021
  49. 2/7 pack-bitmap.c: propagate namehash values from existing bitmapsTaylor Blau, Sep 17, 2021
  50. 3/7 midx.c: respect 'pack.writeBitmapHashcache' when writing bitmapsTaylor Blau, Sep 17, 2021
  51. 4/7 p5326: create missing 'perf-tag' tagTaylor Blau, Sep 17, 2021
  52. 5/7 p5326: don't set core.multiPackIndex unnecessarilyTaylor Blau, Sep 17, 2021
  53. 6/7 p5326: generate pack bitmaps before writing the MIDX bitmapTaylor Blau, Sep 17, 2021
  54. 7/7 t5326: test propagating hashcache valuesTaylor Blau, Sep 17, 2021
  55. Jeff KingSep 17, 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.