diff mbox series

[v3,7/9] pack-objects: introduce pack.allowPackReuse

Message ID 20191115141541.11149-8-chriscool@tuxfamily.org (mailing list archive)
State New, archived
Headers show
Series Rewrite packfile reuse code | expand

Commit Message

Christian Couder Nov. 15, 2019, 2:15 p.m. UTC 
From: Jeff King <peff@peff.net>

Let's make it possible to configure if we want pack reuse or not.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/config/pack.txt | 4 ++++
 builtin/pack-objects.c        | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

Comments

Jeff King Dec. 9, 2019, 7:14 a.m. UTC | #1 
On Fri, Nov 15, 2019 at 03:15:39PM +0100, Christian Couder wrote:

> From: Jeff King <peff@peff.net>
> 
> Let's make it possible to configure if we want pack reuse or not.

...because? :)

I mostly used it for debugging and performance testing. I don't think
there should be any big downside to using it that would cause people to
want to turn it off. But it _might_ cause larger packs, because we
wouldn't consider the reused objects as bases for finding new deltas.

I think the documentation could mention this. And maybe make it a bit
more clear what "reuse" means.

So maybe:

diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
index 58323a351f..0dac580581 100644
--- a/Documentation/config/pack.txt
+++ b/Documentation/config/pack.txt
@@ -28,8 +28,11 @@ all existing objects. You can force recompression by passing the -F option
 to linkgit:git-repack[1].
 
 pack.allowPackReuse::
-	When true, which is the default, Git will try to reuse parts
-	of existing packfiles when preparing new packfiles.
+	When true, and when reachability bitmaps are enabled,
+	pack-objects will try to send parts of the bitmapped packfile
+	verbatim. This can reduce memory and CPU usage to serve fetches,
+	but might result in sending a slightly larger pack. Defaults to
+	true.
 
 pack.island::
 	An extended regular expression configuring a set of delta

-Peff
Christian Couder Dec. 13, 2019, 1:27 p.m. UTC | #2 
On Mon, Dec 9, 2019 at 8:14 AM Jeff King <peff@peff.net> wrote:
>
> On Fri, Nov 15, 2019 at 03:15:39PM +0100, Christian Couder wrote:
>
> > From: Jeff King <peff@peff.net>
> >
> > Let's make it possible to configure if we want pack reuse or not.
>
> ...because? :)
>
> I mostly used it for debugging and performance testing. I don't think
> there should be any big downside to using it that would cause people to
> want to turn it off. But it _might_ cause larger packs, because we
> wouldn't consider the reused objects as bases for finding new deltas.

Ok, I changed the commit message to:

    Let's make it possible to configure if we want pack reuse or not.

    The main reason it might not be wanted is probably debugging and
    performance testing, though pack reuse _might_ cause larger packs,
    because we wouldn't consider the reused objects as bases for
    finding new deltas.

> I think the documentation could mention this. And maybe make it a bit
> more clear what "reuse" means.
>
> So maybe:
>
> diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
> index 58323a351f..0dac580581 100644
> --- a/Documentation/config/pack.txt
> +++ b/Documentation/config/pack.txt
> @@ -28,8 +28,11 @@ all existing objects. You can force recompression by passing the -F option
>  to linkgit:git-repack[1].
>
>  pack.allowPackReuse::
> -       When true, which is the default, Git will try to reuse parts
> -       of existing packfiles when preparing new packfiles.
> +       When true, and when reachability bitmaps are enabled,
> +       pack-objects will try to send parts of the bitmapped packfile
> +       verbatim. This can reduce memory and CPU usage to serve fetches,
> +       but might result in sending a slightly larger pack. Defaults to
> +       true.

Yeah, great! I use the above in my current version. Thanks!
diff mbox series

Patch

diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
index 1d66f0c992..58323a351f 100644
--- a/Documentation/config/pack.txt
+++ b/Documentation/config/pack.txt
@@ -27,6 +27,10 @@  Note that changing the compression level will not automatically recompress
 all existing objects. You can force recompression by passing the -F option
 to linkgit:git-repack[1].
 
+pack.allowPackReuse::
+	When true, which is the default, Git will try to reuse parts
+	of existing packfiles when preparing new packfiles.
+
 pack.island::
 	An extended regular expression configuring a set of delta
 	islands. See "DELTA ISLANDS" in linkgit:git-pack-objects[1]
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f2c2703090..4fcfcf6097 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -96,6 +96,7 @@  static off_t reuse_packfile_offset;
 
 static int use_bitmap_index_default = 1;
 static int use_bitmap_index = -1;
+static int allow_pack_reuse = 1;
 static enum {
 	WRITE_BITMAP_FALSE = 0,
 	WRITE_BITMAP_QUIET,
@@ -2699,6 +2700,10 @@  static int git_pack_config(const char *k, const char *v, void *cb)
 		use_bitmap_index_default = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "pack.allowpackreuse")) {
+		allow_pack_reuse = git_config_bool(k, v);
+		return 0;
+	}
 	if (!strcmp(k, "pack.threads")) {
 		delta_search_threads = git_config_int(k, v);
 		if (delta_search_threads < 0)
@@ -3030,7 +3035,8 @@  static void loosen_unused_packed_objects(void)
  */
 static int pack_options_allow_reuse(void)
 {
-	return pack_to_stdout &&
+	return allow_pack_reuse &&
+	       pack_to_stdout &&
 	       allow_ofs_delta &&
 	       !ignore_packed_keep_on_disk &&
 	       !ignore_packed_keep_in_core &&