threads / patch / 38214

v2, 3 partsDocumentation: add missing article in rev-list-options.txt

Subject: [PATCH v2 1/3] Documentation: add missing article in rev-list-options.txt

## tl;dr

4 messages between Dec 20, 2014 and Dec 20, 2014. Diffs are folded; open one to read it.

replies: 3people: 1as markdown or json

brian m. carlson· Dec 20, 2014, 22:51 UTC · lore

[PATCH v2 0/3] Improve push performance with lots of refs

This series contains patches to address a significant push performance regression in repositories with large amounts of refs. It avoids performing expensive edge marking unless the repository is shallow.

The first patch in the series is a fix for a minor typo I discovered when editing the documentation. The second patch implements git rev-list --objects-edge-aggressive, and the final patch ensures it's used for shallow repos only. As the final patch was suggested by Junio, it will need his sign-off.

I considered Junio's suggestion for a --thin-aggressive, but felt that it was better to have the fix localized to a single location to improve maintainability and that --thin-aggressive would be uncommonly used outside of automatic invocations.

Also, as I understand it, the goal of thin packs is to improve performance by sending fewer objects. In most cases, the benefit of the decreased time spent marking edges push times will dwarf the increase in time it takes for a slightly larger pack to go over the wire, so it doesn't make sense to make this a tunable.

The original fix was suggested by Duy Nguyen.
brian m. carlson (3):
  Documentation: add missing article in rev-list-options.txt
  rev-list: add an option to mark fewer edges as uninteresting
  pack-objects: use --objects-edge-aggressive only for shallow repos
 Documentation/git-rev-list.txt     | 3 ++-
 Documentation/rev-list-options.txt | 7 ++++++-
 builtin/pack-objects.c             | 4 +++-
 list-objects.c                     | 4 ++--
 revision.c                         | 6 ++++++
 revision.h                         | 1 +
 6 files changed, 20 insertions(+), 5 deletions(-)
-- 
2.2.1.209.g41e5f3a
brian m. carlson· Dec 20, 2014, 22:51 UTC · re: brian m. carlson · lore
Add the missing article "a".
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/rev-list-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to Documentation/rev-list-options.txt +1 −1
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index afccfdc..2277fcb 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -653,7 +653,7 @@ These options are mostly targeted for packing of Git repositories.
 --objects-edge::
 	Similar to `--objects`, but also print the IDs of excluded
 	commits prefixed with a ``-'' character.  This is used by
-	linkgit:git-pack-objects[1] to build ``thin'' pack, which records
+	linkgit:git-pack-objects[1] to build a ``thin'' pack, which records
 	objects in deltified form based on objects contained in these
 	excluded commits to reduce network traffic.
 
-- 
2.2.1.209.g41e5f3a
brian m. carlson· Dec 20, 2014, 22:51 UTC · re: brian m. carlson · lore

[PATCH v2 2/3] rev-list: add an option to mark fewer edges as uninteresting

In commit fbd4a70 (list-objects: mark more commits as edges in mark_edges_uninteresting - 2013-08-16), we marked an increasing number of edges uninteresting. This change, and the subsequent change to make this conditional on --objects-edge, are used by --thin to make much smaller packs for shallow clones.

Unfortunately, they cause a significant performance regression when pushing non-shallow clones with lots of refs (23.322 seconds vs. 4.785 seconds with 22400 refs). Add an option to git rev-list, --objects-edge-aggressive, that preserves this more aggressive behavior, while leaving --objects-edge to provide more performant behavior.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/git-rev-list.txt     | 3 ++-
 Documentation/rev-list-options.txt | 4 ++++
 list-objects.c                     | 4 ++--
 revision.c                         | 6 ++++++
 revision.h                         | 1 +
 5 files changed, 15 insertions(+), 3 deletions(-)
Show changes to 5 files +15 −3

Documentation/git-rev-list.txt, Documentation/rev-list-options.txt, list-objects.c, revision.c, revision.h

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index fd7f8b5..5b11922 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -46,7 +46,8 @@ SYNOPSIS
 	     [ \--extended-regexp | -E ]
 	     [ \--fixed-strings | -F ]
 	     [ \--date=(local|relative|default|iso|iso-strict|rfc|short) ]
-	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
+	     [ [ \--objects | \--objects-edge | \--objects-edge-aggressive ]
+	       [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
 	     [ \--bisect ]
 	     [ \--bisect-vars ]
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 2277fcb..8cb6f92 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -657,6 +657,10 @@ These options are mostly targeted for packing of Git repositories.
 	objects in deltified form based on objects contained in these
 	excluded commits to reduce network traffic.
 
+--objects-edge-aggressive::
+	Similar to `--objects-edge`, but it tries harder to find excluded
+	commits at the cost of increased time.
+
 --unpacked::
 	Only useful with `--objects`; print the object IDs that are not
 	in packs.
diff --git a/list-objects.c b/list-objects.c
index 2910bec..2a139b6 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -157,7 +157,7 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge)
 
 		if (commit->object.flags & UNINTERESTING) {
 			mark_tree_uninteresting(commit->tree);
-			if (revs->edge_hint && !(commit->object.flags & SHOWN)) {
+			if (revs->edge_hint_aggressive && !(commit->object.flags & SHOWN)) {
 				commit->object.flags |= SHOWN;
 				show_edge(commit);
 			}
@@ -165,7 +165,7 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge)
 		}
 		mark_edge_parents_uninteresting(commit, revs, show_edge);
 	}
-	if (revs->edge_hint) {
+	if (revs->edge_hint_aggressive) {
 		for (i = 0; i < revs->cmdline.nr; i++) {
 			struct object *obj = revs->cmdline.rev[i].item;
 			struct commit *commit = (struct commit *)obj;
diff --git a/revision.c b/revision.c
index 75dda92..753dd2f 100644
--- a/revision.c
+++ b/revision.c
@@ -1853,6 +1853,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->tree_objects = 1;
 		revs->blob_objects = 1;
 		revs->edge_hint = 1;
+	} else if (!strcmp(arg, "--objects-edge-aggressive")) {
+		revs->tag_objects = 1;
+		revs->tree_objects = 1;
+		revs->blob_objects = 1;
+		revs->edge_hint = 1;
+		revs->edge_hint_aggressive = 1;
 	} else if (!strcmp(arg, "--verify-objects")) {
 		revs->tag_objects = 1;
 		revs->tree_objects = 1;
diff --git a/revision.h b/revision.h
index 9cb5adc..033a244 100644
--- a/revision.h
+++ b/revision.h
@@ -93,6 +93,7 @@ struct rev_info {
 			blob_objects:1,
 			verify_objects:1,
 			edge_hint:1,
+			edge_hint_aggressive:1,
 			limited:1,
 			unpacked:1,
 			boundary:2,
-- 
2.2.1.209.g41e5f3a
brian m. carlson· Dec 20, 2014, 22:51 UTC · re: brian m. carlson · lore

[PATCH v2 3/3] pack-objects: use --objects-edge-aggressive only for shallow repos

Using --objects-edge-aggressive is important for shallow repos, as it can result in a much smaller pack (in some cases, 10% of the size). However, it performs poorly on large non-shallow repositories with many refs. Since shallow repositories are less likely to have many refs (due to having less history), the smaller pack size is advantageous there. Adjust pack-objects to use --objects-edge-aggressive only for shallow repositories.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/rev-list-options.txt | 3 ++-
 builtin/pack-objects.c             | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)
Show changes to 2 files +5 −2

Documentation/rev-list-options.txt, builtin/pack-objects.c

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 8cb6f92..2984f40 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -659,7 +659,8 @@ These options are mostly targeted for packing of Git repositories.
 
 --objects-edge-aggressive::
 	Similar to `--objects-edge`, but it tries harder to find excluded
-	commits at the cost of increased time.
+	commits at the cost of increased time.  This is used instead of
+	`--objects-edge` to build ``thin'' packs for shallow repositories.
 
 --unpacked::
 	Only useful with `--objects`; print the object IDs that are not
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 3f9f5c7..f3ba861 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2711,7 +2711,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	argv_array_push(&rp, "pack-objects");
 	if (thin) {
 		use_internal_rev_list = 1;
-		argv_array_push(&rp, "--objects-edge");
+		argv_array_push(&rp, is_repository_shallow()
+				? "--objects-edge-aggressive"
+				: "--objects-edge");
 	} else
 		argv_array_push(&rp, "--objects");
 
-- 
2.2.1.209.g41e5f3a

← back to recent threads