@matheusbsbs / matheus-bagliano / commits / fc62e033cd

pack-objects: add --name-hash-version option

The previous change introduced a new pack_name_hash_v2() function that intends to satisfy much of the hash locality features of the existing pack_name_hash() function while also distinguishing paths with similar final components of their paths. This change adds a new --name-hash-version option for 'git pack-objects' to allow users to select their preferred function version. This use of an integer version allows for future expansion and a direct way to later store a name hash version in the .bitmap format. For now, let's consider how effective this mechanism is when repacking a repository with different name hash versions. Specifically, we will execute 'git pack-objects' the same way a 'git repack -adf' process would, except we include --name-hash-version=<n> for testing. On the Git repository, we do not expect much difference. All path names are short. This is backed by our results: | Stage | Pack Size | Repack Time | |-----------------------|-----------|-------------| | After clone | 260 MB | N/A | | --name-hash-version=1 | 127 MB | 129s | | --name-hash-version=2 | 127 MB | 112s | This example demonstrates how there is some natural overhead coming from the cloned copy because the server is hosting many forks and has not optimized for exactly this set of reachable objects. But the full repack has similar characteristics for both versions. Let's consider some repositories that are hitting too many collisions with version 1. First, let's explore the kinds of paths that are commonly causing these collisions: * "/CHANGELOG.json" is 15 characters, and is created by the beachball [1] tool. Only the final character of the parent directory can differentiate different versions of this file, but also only the two most-significant digits. If that character is a letter, then this is always a collision. Similar issues occur with the similar "/CHANGELOG.md" path, though there is more opportunity for differences In the parent directory. * Localization files frequently have common filenames but differentiates via parent directories. In C#, the name "/strings.resx.lcl" is used for these localization files and they will all collide in name-hash. [1] https://github.com/microsoft/beachball I've come across many other examples where some internal tool uses a common name across multiple directories and is causing Git to repack poorly due to name-hash collisions. One open-source example is the fluentui [2] repo, which uses beachball to generate CHANGELOG.json and CHANGELOG.md files, and these files have very poor delta characteristics when comparing against versions across parent directories. | Stage | Pack Size | Repack Time | |-----------------------|-----------|-------------| | After clone | 694 MB | N/A | | --name-hash-version=1 | 438 MB | 728s | | --name-hash-version=2 | 168 MB | 142s | [2] https://github.com/microsoft/fluentui In this example, we see significant gains in the compressed packfile size as well as the time taken to compute the packfile. Using a collection of repositories that use the beachball tool, I was able to make similar comparisions with dramatic results. While the fluentui repo is public, the others are private so cannot be shared for reproduction. The results are so significant that I find it important to share here: | Repo | --name-hash-version=1 | --name-hash-version=2 | |----------|-----------------------|-----------------------| | fluentui | 440 MB | 161 MB | | Repo B | 6,248 MB | 856 MB | | Repo C | 37,278 MB | 6,755 MB | | Repo D | 131,204 MB | 7,463 MB | Future changes could include making --name-hash-version implied by a config value or even implied by default during a full repack. It is important to point out that the name hash value is stored in the .bitmap file format, so we must force --name-hash-version=1 when bitmaps are being read or written. Later, the bitmap format could be updated to be aware of the name hash version so deltas can be quickly computed across the bitmapped/not-bitmapped boundary. To promote the safety of this parameter, the validate_name_hash_version() method will die() if the given name-hash version is incorrect and will disable newer versions if not yet compatible with other features, such as --write-bitmap-index. Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Jan 27, 2025 at 19:02 UTC fc62e033cd93ff6b93e312d89bfb5683a4c6f90c

3 files changed +109 -6

Documentation/git-pack-objects.txt

+31 -1

@@ -15,7 +15,8 @@ SYNOPSIS

15 [--revs [--unpacked | --all]] [--keep-pack=<pack-name>]

16 [--cruft] [--cruft-expiration=<time>]

17 [--stdout [--filter=<filter-spec>] | <base-name>]

18 - [--shallow] [--keep-true-parents] [--[no-]sparse] < <object-list>

18 + [--shallow] [--keep-true-parents] [--[no-]sparse]

19 + [--name-hash-version=<n>] < <object-list>

22 DESCRIPTION

@@ -345,6 +346,35 @@ raise an error.

346 Restrict delta matches based on "islands". See DELTA ISLANDS

347 below.

348

349 +--name-hash-version=<n>::

350 + While performing delta compression, Git groups objects that may be

351 + similar based on heuristics using the path to that object. While

352 + grouping objects by an exact path match is good for paths with

353 + many versions, there are benefits for finding delta pairs across

354 + different full paths. Git collects objects by type and then by a

355 + "name hash" of the path and then by size, hoping to group objects

356 + that will compress well together.

357 ++

358 +The default name hash version is `1`, which prioritizes hash locality by

359 +considering the final bytes of the path as providing the maximum magnitude

360 +to the hash function. This version excels at distinguishing short paths

361 +and finding renames across directories. However, the hash function depends

362 +primarily on the final 16 bytes of the path. If there are many paths in

363 +the repo that have the same final 16 bytes and differ only by parent

364 +directory, then this name-hash may lead to too many collisions and cause

365 +poor results. At the moment, this version is required when writing

366 +reachability bitmap files with `--write-bitmap-index`.

367 ++

368 +The name hash version `2` has similar locality features as version `1`,

369 +except it considers each path component separately and overlays the hashes

370 +with a shift. This still prioritizes the final bytes of the path, but also

371 +"salts" the lower bits of the hash using the parent directory names. This

372 +method allows for some of the locality benefits of version `1` while

373 +breaking most of the collisions from a similarly-named file appearing in

374 +many different directories. At the moment, this version is not allowed

375 +when writing reachability bitmap files with `--write-bitmap-index` and it

376 +will be automatically changed to version `1`.

377 +

378

379 DELTA ISLANDS

380 -------------

builtin/pack-objects.c

+47 -5

@@ -266,6 +266,35 @@ struct configured_exclusion {

266 static struct oidmap configured_exclusions;

267

268 static struct oidset excluded_by_config;

269 +static int name_hash_version = 1;

270 +

271 +/**

272 + * Check whether the name_hash_version chosen by user input is appropriate,

273 + * and also validate whether it is compatible with other features.

274 + */

275 +static void validate_name_hash_version(void)

276 +{

277 + if (name_hash_version < 1 || name_hash_version > 2)

278 + die(_("invalid --name-hash-version option: %d"), name_hash_version);

279 + if (write_bitmap_index && name_hash_version != 1) {

280 + warning(_("currently, --write-bitmap-index requires --name-hash-version=1"));

281 + name_hash_version = 1;

282 + }

283 +}

284 +

285 +static inline uint32_t pack_name_hash_fn(const char *name)

286 +{

287 + switch (name_hash_version) {

288 + case 1:

289 + return pack_name_hash(name);

290 +

291 + case 2:

292 + return pack_name_hash_v2((const unsigned char *)name);

293 +

294 + default:

295 + BUG("invalid name-hash version: %d", name_hash_version);

296 + }

297 +}

298

299 /*

300 * stats

@@ -1698,7 +1727,7 @@ static int add_object_entry(const struct object_id *oid, enum object_type type,

1727 return 0;

1728 }

1729

1701 - create_object_entry(oid, type, pack_name_hash(name),

1730 + create_object_entry(oid, type, pack_name_hash_fn(name),

1731 exclude, name && no_try_delta(name),

1732 found_pack, found_offset);

1733 return 1;

@@ -1912,7 +1941,7 @@ static void add_preferred_base_object(const char *name)

1941 {

1942 struct pbase_tree *it;

1943 size_t cmplen;

1915 - unsigned hash = pack_name_hash(name);

1944 + unsigned hash = pack_name_hash_fn(name);

1945

1946 if (!num_preferred_base || check_pbase_path(hash))

1947 return;

@@ -3422,7 +3451,7 @@ static void show_object_pack_hint(struct object *object, const char *name,

3451 * here using a now in order to perhaps improve the delta selection

3452 * process.

3453 */

3425 - oe->hash = pack_name_hash(name);

3454 + oe->hash = pack_name_hash_fn(name);

3455 oe->no_try_delta = name && no_try_delta(name);

3456

3457 stdin_packs_hints_nr++;

@@ -3572,7 +3601,7 @@ static void add_cruft_object_entry(const struct object_id *oid, enum object_type

3601 entry = packlist_find(&to_pack, oid);

3602 if (entry) {

3603 if (name) {

3575 - entry->hash = pack_name_hash(name);

3604 + entry->hash = pack_name_hash_fn(name);

3605 entry->no_try_delta = no_try_delta(name);

3606 }

3607 } else {

@@ -3595,7 +3624,7 @@ static void add_cruft_object_entry(const struct object_id *oid, enum object_type

3624 return;

3625 }

3626

3598 - entry = create_object_entry(oid, type, pack_name_hash(name),

3627 + entry = create_object_entry(oid, type, pack_name_hash_fn(name),

3628 0, name && no_try_delta(name),

3629 pack, offset);

3630 }

@@ -4069,6 +4098,15 @@ static int get_object_list_from_bitmap(struct rev_info *revs)

4098 if (!(bitmap_git = prepare_bitmap_walk(revs, 0)))

4099 return -1;

4100

4101 + /*

4102 + * For now, force the name-hash version to be 1 since that

4103 + * is the version implied by the bitmap format. Later, the

4104 + * format can include this version explicitly in its format,

4105 + * allowing readers to know the version that was used during

4106 + * the bitmap write.

4107 + */

4108 + name_hash_version = 1;

4109 +

4110 if (pack_options_allow_reuse())

4111 reuse_partial_packfile_from_bitmap(bitmap_git,

4112 &reuse_packfiles,

@@ -4429,6 +4467,8 @@ int cmd_pack_objects(int argc,

4467 OPT_STRING_LIST(0, "uri-protocol", &uri_protocols,

4468 N_("protocol"),

4469 N_("exclude any configured uploadpack.blobpackfileuri with this protocol")),

4470 + OPT_INTEGER(0, "name-hash-version", &name_hash_version,

4471 + N_("use the specified name-hash function to group similar objects")),

4472 OPT_END(),

4473 };

4474

@@ -4576,6 +4616,8 @@ int cmd_pack_objects(int argc,

4616 if (pack_to_stdout || !rev_list_all)

4617 write_bitmap_index = 0;

4618

4619 + validate_name_hash_version();

4620 +

4621 if (use_delta_islands)

4622 strvec_push(&rp, "--topo-order");

4623

t/t5300-pack-object.sh

+31

@@ -674,4 +674,35 @@ do

674 '

675 done

676

677 +test_expect_success 'valid and invalid --name-hash-versions' '

678 + # Valid values are hard to verify other than "do not fail".

679 + # Performance tests will be more valuable to validate these versions.

680 + for value in 1 2

681 + do

682 + git pack-objects base --all --name-hash-version=$value || return 1

683 + done &&

684 +

685 + # Invalid values have clear post-conditions.

686 + for value in -1 0 3

687 + do

688 + test_must_fail git pack-objects base --all --name-hash-version=$value 2>err &&

689 + test_grep "invalid --name-hash-version option" err || return 1

690 + done

691 +'

692 +

693 +# The following test is not necessarily a permanent choice, but since we do not

694 +# have a "name hash version" bit in the .bitmap file format, we cannot write the

695 +# hash values into the .bitmap file without risking breakage later.

696 +#

697 +# TODO: Make these compatible in the future and replace this test with the

698 +# expected behavior when both are specified.

699 +test_expect_success '--name-hash-version=2 and --write-bitmap-index are incompatible' '

700 + git pack-objects base --all --name-hash-version=2 --write-bitmap-index 2>err &&

701 + test_grep "currently, --write-bitmap-index requires --name-hash-version=1" err &&

702 +

703 + # --stdout option silently removes --write-bitmap-index

704 + git pack-objects --stdout --all --name-hash-version=2 --write-bitmap-index >out 2>err &&

705 + ! test_grep "currently, --write-bitmap-index requires --name-hash-version=1" err

706 +'

707 +

708 test_done