[PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[Derrick Stolee via GitGitGadget]

This patch series includes a few new config options we created to speed up
certain critical commands in VFS for Git. On their own, they would
contribute little value as it is hard to discover new config variables.
Instead, I've created this RFC as a goal for probably three sequential patch
series:

 1. (Patches 1-3) Introduce a new 'core.size' config setting that takes
    'large' as a value. This enables several config values that are
    beneficial for large repos. We use a certain set in VFS for Git (see
    [1]), and most of those are applicable to any repo. This 'core.size'
    setting is intended for users to automatically receive performance
    updates as soon as they are stable, but they must opt-in to the setting
    and can always explicitly set their own config values. The settings to
    include here are core.commitGraph=true, gc.writeCommitGraph=true,
    index.version=4, pack.useSparse=true.
    
    
 2. (Patches 4-8) Introduce 'status.aheadBehind' to dictate if we use
    '--[no-]ahead-behind' during 'git status' calls. Also do some cleanup on
    the feature around porcelain formats. I adapted Jeff Hostetler's commits
    from microsoft/git for this section.
    
    
 3. (Patches 9-12) Introduce 'fetch.showForcedUpdates' and the associated
    '--[no-]show-forced-updates' option for 'git fetch' and 'git pull'
    calls. When fetching from a remote with many branches that move quickly,
    the check for forced updates can be expensive. Further, the only effects
    are a "(forced update)" indicator to stdout and a single bit in the
    reflog. The reflog bit is unfortunate to lose, but it is never trusted
    for important actions. These changes are likely to be more controversial
    than the others.
    
    

Hopefully this direction is amenable to allow "early adopters" gain access
to new performance features even if they are not necessary reading every
line of the release notes.

Thanks, -Stolee

[1] 
https://github.com/microsoft/VFSForGit/blob/6a7fd2ff50056b73b347b882d2b8d52939bd6419/GVFS/GVFS/CommandLine/GVFSVerb.cs#L122-L152
This code includes the settings we enable by default in VFS for Git
enlistments.

Derrick Stolee (8):
  repo-settings: create repo.size=large setting
  repo-settings: use index.version=4 by default
  repo-settings: pack.useSparse=true
  repo-settings: status.aheadBehind=false
  fetch: add --[no-]show-forced-updates argument
  fetch: warn about forced updates after branch list
  pull: add --[no-]show-forced-updates passthrough to fetch
  repo-settings: fetch.showForcedUpdates=false

Jeff Hostetler (3):
  status: add status.aheadbehind setting
  status: add warning when a/b calculation takes too long for
    long/normal format
  status: ignore status.aheadbehind in porcelain formats

 Documentation/config/core.txt   | 31 ++++++++++++++-
 Documentation/config/fetch.txt  |  5 +++
 Documentation/config/gc.txt     |  4 +-
 Documentation/config/index.txt  |  1 +
 Documentation/config/pack.txt   |  3 +-
 Documentation/config/status.txt |  6 +++
 Documentation/fetch-options.txt | 13 +++++++
 Makefile                        |  1 +
 advice.c                        |  2 +
 advice.h                        |  1 +
 builtin/commit.c                | 19 ++++++++-
 builtin/fetch.c                 | 34 ++++++++++++++++-
 builtin/gc.c                    |  6 +--
 builtin/pack-objects.c          |  9 +++--
 builtin/pull.c                  |  7 ++++
 commit-graph.c                  |  7 ++--
 read-cache.c                    | 12 +++---
 repo-settings.c                 | 68 +++++++++++++++++++++++++++++++++
 repo-settings.h                 | 17 +++++++++
 repository.h                    |  3 ++
 t/t6040-tracking-info.sh        | 31 +++++++++++++++
 t/t7064-wtstatus-pv2.sh         |  8 ++++
 wt-status.c                     | 17 +++++++++
 23 files changed, 283 insertions(+), 22 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h


base-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-254%2Fderrickstolee%2Fconfig-large%2Fupstream-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-254/derrickstolee/config-large/upstream-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/254
-- 
gitgitgadget

[PATCH 01/11] repo-settings: create repo.size=large setting

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

Several advanced config settings are highly recommended for clients
using large repositories. Power users learn these one-by-one and
enable them as they see fit. This could be made simpler, to allow
more users to have access to these almost-always beneficial features
(and more beneficial in larger repos).

Create a 'repo.size' config setting whose only accepted value is
'large'. When a repo.size=large is given, change the default values
of some config settings. If the setting is given explicitly, then
take the explicit value.

This change adds these two defaults to the repo.size=large setting:

 * core.commitGraph=true
 * gc.writeCommitGraph=true

To centralize these config options and properly set the defaults,
create a repo_settings that contains chars for each config variable.
Use -1 as "unset", with 0 for false and 1 for true.

The prepare_repo_settings() method ensures that this settings
struct has been initialized, and avoids double-scanning the config
settings.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt | 16 +++++++++++--
 Documentation/config/gc.txt   |  4 ++--
 Makefile                      |  1 +
 builtin/gc.c                  |  6 ++---
 commit-graph.c                |  7 +++---
 repo-settings.c               | 44 +++++++++++++++++++++++++++++++++++
 repo-settings.h               | 13 +++++++++++
 repository.h                  |  3 +++
 8 files changed, 84 insertions(+), 10 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 75538d27e7..1a188db620 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -577,8 +577,9 @@ the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
 core.commitGraph::
 	If true, then git will read the commit-graph file (if it exists)
-	to parse the graph structure of commits. Defaults to false. See
-	linkgit:git-commit-graph[1] for more information.
+	to parse the graph structure of commits. Defaults to false, unless
+	`core.size=large`. See linkgit:git-commit-graph[1] for more
+	information.
 
 core.useReplaceRefs::
 	If set to `false`, behave as if the `--no-replace-objects`
@@ -601,3 +602,14 @@ core.abbrev::
 	in your repository, which hopefully is enough for
 	abbreviated object names to stay unique for some time.
 	The minimum length is 4.
+
+core.size::
+	When specified as "large", change the default values of some config
+	variables to improve performance in a large repository. If a variable
+	is specified explicitly, the explicit value will override these
+	defaults:
++
+* `core.commitGraph=true` enables reading commit-graph files.
++
+* `gc.writeCommitGraph=true` eneables writing commit-graph files during
+`git gc`.
diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 02b92b18b5..680721ebbb 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -63,8 +63,8 @@ gc.writeCommitGraph::
 	If true, then gc will rewrite the commit-graph file when
 	linkgit:git-gc[1] is run. When using `git gc --auto`
 	the commit-graph will be updated if housekeeping is
-	required. Default is false. See linkgit:git-commit-graph[1]
-	for details.
+	required. Default is false, unless `core.size=large`.
+	See linkgit:git-commit-graph[1] for details.
 
 gc.logExpiry::
 	If the file gc.log exists, then `git gc --auto` will print
diff --git a/Makefile b/Makefile
index 8a7e235352..2d3499d7ac 100644
--- a/Makefile
+++ b/Makefile
@@ -967,6 +967,7 @@ LIB_OBJS += refspec.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace-object.o
+LIB_OBJS += repo-settings.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
diff --git a/builtin/gc.c b/builtin/gc.c
index 8943bcc300..6281aad961 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -27,6 +27,7 @@
 #include "pack-objects.h"
 #include "blob.h"
 #include "tree.h"
+#include "repo-settings.h"
 
 #define FAILED_RUN "failed to run %s"
 
@@ -41,7 +42,6 @@ static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
 static int detach_auto = 1;
 static timestamp_t gc_log_expire_time;
 static const char *gc_log_expire = "1.day.ago";
@@ -148,7 +148,6 @@ static void gc_config(void)
 	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
 	git_config_get_int("gc.auto", &gc_auto_threshold);
 	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
-	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
 	git_config_get_bool("gc.autodetach", &detach_auto);
 	git_config_get_expiry("gc.pruneexpire", &prune_expire);
 	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
@@ -685,7 +684,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		clean_pack_garbage();
 	}
 
-	if (gc_write_commit_graph)
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings->gc_write_commit_graph == 1)
 		write_commit_graph_reachable(get_object_directory(), 0,
 					     !quiet && !daemonized);
 
diff --git a/commit-graph.c b/commit-graph.c
index 7c5e54875f..b09c465a7a 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -16,6 +16,7 @@
 #include "hashmap.h"
 #include "replace-object.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
 #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
@@ -311,7 +312,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
 static int prepare_commit_graph(struct repository *r)
 {
 	struct object_directory *odb;
-	int config_value;
 
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 		die("dying as requested by the '%s' variable on commit-graph load!",
@@ -321,9 +321,10 @@ static int prepare_commit_graph(struct repository *r)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
+	prepare_repo_settings(r);
+
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
-	    !config_value))
+	    r->settings->core_commit_graph != 1)
 		/*
 		 * This repository is not configured to use commit graphs, so
 		 * do not load one. (But report commit_graph_attempted anyway
diff --git a/repo-settings.c b/repo-settings.c
new file mode 100644
index 0000000000..6f5e18d92e
--- /dev/null
+++ b/repo-settings.c
@@ -0,0 +1,44 @@
+#include "cache.h"
+#include "repository.h"
+#include "config.h"
+#include "repo-settings.h"
+
+
+#define UPDATE_DEFAULT(s,v) if (s != -1) { s = v; }
+
+static int git_repo_config(const char *key, const char *value, void *cb)
+{
+	struct repo_settings *rs = (struct repo_settings *)cb;
+
+	if (!strcmp(key, "core.size")) {
+		if (!strcmp(value, "large")) {
+			UPDATE_DEFAULT(rs->core_commit_graph, 1);
+			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+		}
+		return 0;
+	}
+	if (!strcmp(key, "core.commitgraph")) {
+		rs->core_commit_graph = git_config_bool(key, value);
+		return 0;
+	}
+	if (!strcmp(key, "gc.writecommitgraph")) {
+		rs->gc_write_commit_graph = git_config_bool(key, value);
+		return 0;
+	}
+
+	return 1;
+}
+
+void prepare_repo_settings(struct repository *r)
+{
+	if (r->settings)
+		return;
+
+	r->settings = xmalloc(sizeof(*r->settings));
+
+	/* Defaults */
+	r->settings->core_commit_graph = -1;
+	r->settings->gc_write_commit_graph = -1;
+
+	repo_config(r, git_repo_config, r->settings);
+}
diff --git a/repo-settings.h b/repo-settings.h
new file mode 100644
index 0000000000..11d08648e1
--- /dev/null
+++ b/repo-settings.h
@@ -0,0 +1,13 @@
+#ifndef REPO_SETTINGS_H
+#define REPO_SETTINGS_H
+
+struct repo_settings {
+	char core_commit_graph;
+	char gc_write_commit_graph;
+};
+
+struct repository;
+
+void prepare_repo_settings(struct repository *r);
+
+#endif /* REPO_SETTINGS_H */
diff --git a/repository.h b/repository.h
index 4fb6a5885f..352afc9cd8 100644
--- a/repository.h
+++ b/repository.h
@@ -4,6 +4,7 @@
 #include "path.h"
 
 struct config_set;
+struct repo_settings;
 struct git_hash_algo;
 struct index_state;
 struct lock_file;
@@ -72,6 +73,8 @@ struct repository {
 	 */
 	char *submodule_prefix;
 
+	struct repo_settings *settings;
+
 	/* Subsystems */
 	/*
 	 * Repository's config which contains key-value pairs from the usual
-- 
gitgitgadget

[PATCH 02/11] repo-settings: use index.version=4 by default

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

If a repo is large, it likely has many paths in its working directory.
This means the index could be compressed using version 4. Set this as
a default when core.size=large.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt  |  3 +++
 Documentation/config/index.txt |  1 +
 read-cache.c                   | 12 +++++++-----
 repo-settings.c                |  6 ++++++
 repo-settings.h                |  1 +
 5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 1a188db620..ea64f675fa 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -613,3 +613,6 @@ core.size::
 +
 * `gc.writeCommitGraph=true` eneables writing commit-graph files during
 `git gc`.
++
+* `index.version=4` uses prefix-compression to reduce the size of the
+.git/index file.
diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt
index f181503041..d4b56925c4 100644
--- a/Documentation/config/index.txt
+++ b/Documentation/config/index.txt
@@ -24,3 +24,4 @@ index.threads::
 index.version::
 	Specify the version with which new index files should be
 	initialized.  This does not affect existing repositories.
+	If `core.size=large`, then the default value is 4.
diff --git a/read-cache.c b/read-cache.c
index 22e7b9944e..7fab8ff748 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -25,6 +25,7 @@
 #include "fsmonitor.h"
 #include "thread-utils.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -1599,16 +1600,17 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate,
 
 #define INDEX_FORMAT_DEFAULT 3
 
-static unsigned int get_index_format_default(void)
+static unsigned int get_index_format_default(struct repository *r)
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
 	char *endp;
-	int value;
 	unsigned int version = INDEX_FORMAT_DEFAULT;
 
 	if (!envversion) {
-		if (!git_config_get_int("index.version", &value))
-			version = value;
+		prepare_repo_settings(r);
+
+		if (r->settings->index_version >= 0)
+			version = r->settings->index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
 			warning(_("index.version set, but the value is invalid.\n"
 				  "Using version %i"), INDEX_FORMAT_DEFAULT);
@@ -2765,7 +2767,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
 	}
 
 	if (!istate->version) {
-		istate->version = get_index_format_default();
+		istate->version = get_index_format_default(the_repository);
 		if (git_env_bool("GIT_TEST_SPLIT_INDEX", 0))
 			init_split_index(istate);
 	}
diff --git a/repo-settings.c b/repo-settings.c
index 6f5e18d92e..7e6e65d60c 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -14,6 +14,7 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		if (!strcmp(value, "large")) {
 			UPDATE_DEFAULT(rs->core_commit_graph, 1);
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+			UPDATE_DEFAULT(rs->index_version, 4);
 		}
 		return 0;
 	}
@@ -25,6 +26,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->gc_write_commit_graph = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "index.version")) {
+		rs->index_version = git_config_int(key, value);
+		return 0;
+	}
 
 	return 1;
 }
@@ -39,6 +44,7 @@ void prepare_repo_settings(struct repository *r)
 	/* Defaults */
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
+	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
 }
diff --git a/repo-settings.h b/repo-settings.h
index 11d08648e1..9b8104042e 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -4,6 +4,7 @@
 struct repo_settings {
 	char core_commit_graph;
 	char gc_write_commit_graph;
+	int index_version;
 };
 
 struct repository;
-- 
gitgitgadget

[PATCH 03/11] repo-settings: pack.useSparse=true

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

If a repo is large, then it probably has a very large working
directory. In this case, a typical developer's edits usually impact
many fewer paths than the full path set. The sparse treewalk
algorithm is optimized for this case, speeding up 'git push' calls.

Use pack.useSparse=true when core.size=large.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt | 4 ++++
 Documentation/config/pack.txt | 3 ++-
 builtin/pack-objects.c        | 9 +++++----
 repo-settings.c               | 6 ++++++
 repo-settings.h               | 1 +
 5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index ea64f675fa..df357f5af5 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -616,3 +616,7 @@ core.size::
 +
 * `index.version=4` uses prefix-compression to reduce the size of the
 .git/index file.
++
+* `pack.useSparse=true` uses the sparse tree-walk algorithm, which is
+optimized for enumerating objects during linkgit:git-push[1] from a
+client machine.
diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
index 9cdcfa7324..e6f44de104 100644
--- a/Documentation/config/pack.txt
+++ b/Documentation/config/pack.txt
@@ -112,7 +112,8 @@ pack.useSparse::
 	objects. This can have significant performance benefits when
 	computing a pack to send a small change. However, it is possible
 	that extra objects are added to the pack-file if the included
-	commits contain certain types of direct renames.
+	commits contain certain types of direct renames. Defaults to
+	false, unless `core.size=large`.
 
 pack.writeBitmaps (deprecated)::
 	This is a deprecated synonym for `repack.writeBitmaps`.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 41d7fc5983..f26b3f2892 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -34,6 +34,7 @@
 #include "dir.h"
 #include "midx.h"
 #include "trace2.h"
+#include "repo-settings.h"
 
 #define IN_PACK(obj) oe_in_pack(&to_pack, obj)
 #define SIZE(obj) oe_size(&to_pack, obj)
@@ -2707,10 +2708,6 @@ 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.usesparse")) {
-		sparse = 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)
@@ -3330,6 +3327,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	read_replace_refs = 0;
 
 	sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
+	prepare_repo_settings(the_repository);
+	if (!sparse && the_repository->settings->pack_use_sparse != -1)
+		sparse = the_repository->settings->pack_use_sparse;
+
 	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 
diff --git a/repo-settings.c b/repo-settings.c
index 7e6e65d60c..026ab9c1a0 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -14,6 +14,7 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		if (!strcmp(value, "large")) {
 			UPDATE_DEFAULT(rs->core_commit_graph, 1);
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+			UPDATE_DEFAULT(rs->pack_use_sparse, 1);
 			UPDATE_DEFAULT(rs->index_version, 4);
 		}
 		return 0;
@@ -26,6 +27,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->gc_write_commit_graph = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "pack.usesparse")) {
+		rs->pack_use_sparse = git_config_bool(key, value);
+		return 0;
+	}
 	if (!strcmp(key, "index.version")) {
 		rs->index_version = git_config_int(key, value);
 		return 0;
@@ -44,6 +49,7 @@ void prepare_repo_settings(struct repository *r)
 	/* Defaults */
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
+	r->settings->pack_use_sparse = -1;
 	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
diff --git a/repo-settings.h b/repo-settings.h
index 9b8104042e..b50228f992 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -4,6 +4,7 @@
 struct repo_settings {
 	char core_commit_graph;
 	char gc_write_commit_graph;
+	char pack_use_sparse;
 	int index_version;
 };
 
-- 
gitgitgadget

[PATCH 04/11] status: add status.aheadbehind setting

[Jeff Hostetler via GitGitGadget]

From: Jeff Hostetler <jeffhost@microsoft.com>

Add "status.aheadbehind" config setting to change the default
behavior of ALL git status formats.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/status.txt |  5 +++++
 builtin/commit.c                | 17 ++++++++++++++++-
 t/t6040-tracking-info.sh        | 31 +++++++++++++++++++++++++++++++
 t/t7064-wtstatus-pv2.sh         |  4 ++++
 4 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
index ed72fa7dae..0fc704ab80 100644
--- a/Documentation/config/status.txt
+++ b/Documentation/config/status.txt
@@ -12,6 +12,11 @@ status.branch::
 	Set to true to enable --branch by default in linkgit:git-status[1].
 	The option --no-branch takes precedence over this variable.
 
+status.aheadBehind::
+	Set to true to enable `--ahead-behind` and false to enable
+	`--no-ahead-behind` by default in linkgit:git-status[1] for
+	non-porcelain status formats.  Defaults to true.
+
 status.displayCommentPrefix::
 	If set to true, linkgit:git-status[1] will insert a comment
 	prefix before each output line (starting with
diff --git a/builtin/commit.c b/builtin/commit.c
index 1c9e8e2228..71305073ad 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1078,9 +1078,11 @@ static const char *read_commit_message(const char *name)
 static struct status_deferred_config {
 	enum wt_status_format status_format;
 	int show_branch;
+	enum ahead_behind_flags ahead_behind;
 } status_deferred_config = {
 	STATUS_FORMAT_UNSPECIFIED,
-	-1 /* unspecified */
+	-1, /* unspecified */
+	AHEAD_BEHIND_UNSPECIFIED,
 };
 
 static void finalize_deferred_config(struct wt_status *s)
@@ -1107,6 +1109,15 @@ static void finalize_deferred_config(struct wt_status *s)
 	if (s->show_branch < 0)
 		s->show_branch = 0;
 
+	/*
+	 * If the user did not give a "--[no]-ahead-behind" command
+	 * line argument, then we inherit the a/b config setting.
+	 * If is not set, then we inherit _FULL for backwards
+	 * compatibility.
+	 */
+	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+		s->ahead_behind_flags = status_deferred_config.ahead_behind;
+
 	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
 		s->ahead_behind_flags = AHEAD_BEHIND_FULL;
 }
@@ -1246,6 +1257,10 @@ static int git_status_config(const char *k, const char *v, void *cb)
 		status_deferred_config.show_branch = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "status.aheadbehind")) {
+		status_deferred_config.ahead_behind = git_config_bool(k, v);
+		return 0;
+	}
 	if (!strcmp(k, "status.showstash")) {
 		s->show_stash = git_config_bool(k, v);
 		return 0;
diff --git a/t/t6040-tracking-info.sh b/t/t6040-tracking-info.sh
index 716283b274..febf63f28a 100755
--- a/t/t6040-tracking-info.sh
+++ b/t/t6040-tracking-info.sh
@@ -159,6 +159,19 @@ test_expect_success 'status -s -b --no-ahead-behind (diverged from upstream)' '
 	test_i18ncmp expect actual
 '
 
+cat >expect <<\EOF
+## b1...origin/master [different]
+EOF
+
+test_expect_success 'status.aheadbehind=false status -s -b (diverged from upstream)' '
+	(
+		cd test &&
+		git checkout b1 >/dev/null &&
+		git -c status.aheadbehind=false status -s -b | head -1
+	) >actual &&
+	test_i18ncmp expect actual
+'
+
 cat >expect <<\EOF
 On branch b1
 Your branch and 'origin/master' have diverged,
@@ -174,6 +187,15 @@ test_expect_success 'status --long --branch' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'status --long --branch' '
+	(
+		cd test &&
+		git checkout b1 >/dev/null &&
+		git -c status.aheadbehind=true status --long -b | head -3
+	) >actual &&
+	test_i18ncmp expect actual
+'
+
 cat >expect <<\EOF
 On branch b1
 Your branch and 'origin/master' refer to different commits.
@@ -188,6 +210,15 @@ test_expect_success 'status --long --branch --no-ahead-behind' '
 	test_i18ncmp expect actual
 '
 
+test_expect_success 'status.aheadbehind=false status --long --branch' '
+	(
+		cd test &&
+		git checkout b1 >/dev/null &&
+		git -c status.aheadbehind=false status --long -b | head -2
+	) >actual &&
+	test_i18ncmp expect actual
+'
+
 cat >expect <<\EOF
 ## b5...brokenbase [gone]
 EOF
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index 11eccc231a..a0baf6e8b0 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -436,6 +436,10 @@ test_expect_success 'verify --[no-]ahead-behind with V2 format' '
 		git status --no-ahead-behind --porcelain=v2 --branch --untracked-files=all >actual &&
 		test_cmp expect actual &&
 
+		# Confirmat that "status.aheadbehind" works on V2 format.
+		git -c status.aheadbehind=false status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
 		# Confirm --ahead-behind reports traditional branch.ab with 1/0.
 		cat >expect <<-EOF &&
 		# branch.oid $HUF
-- 
gitgitgadget

[PATCH 05/11] status: add warning when a/b calculation takes too long for long/normal format

[Jeff Hostetler via GitGitGadget]

From: Jeff Hostetler <jeffhost@microsoft.com>

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 advice.c    |  2 ++
 advice.h    |  1 +
 wt-status.c | 17 +++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/advice.c b/advice.c
index ce5f374ecd..54f8dea30c 100644
--- a/advice.c
+++ b/advice.c
@@ -12,6 +12,7 @@ int advice_push_needs_force = 1;
 int advice_push_unqualified_ref_name = 1;
 int advice_status_hints = 1;
 int advice_status_u_option = 1;
+int advice_status_ahead_behind_warning = 1;
 int advice_commit_before_merge = 1;
 int advice_reset_quiet_warning = 1;
 int advice_resolve_conflict = 1;
@@ -68,6 +69,7 @@ static struct {
 	{ "pushUnqualifiedRefName", &advice_push_unqualified_ref_name },
 	{ "statusHints", &advice_status_hints },
 	{ "statusUoption", &advice_status_u_option },
+	{ "statusAheadBehindWarning", &advice_status_ahead_behind_warning },
 	{ "commitBeforeMerge", &advice_commit_before_merge },
 	{ "resetQuiet", &advice_reset_quiet_warning },
 	{ "resolveConflict", &advice_resolve_conflict },
diff --git a/advice.h b/advice.h
index e50f02cdfe..c86de9b9b8 100644
--- a/advice.h
+++ b/advice.h
@@ -12,6 +12,7 @@ extern int advice_push_needs_force;
 extern int advice_push_unqualified_ref_name;
 extern int advice_status_hints;
 extern int advice_status_u_option;
+extern int advice_status_ahead_behind_warning;
 extern int advice_commit_before_merge;
 extern int advice_reset_quiet_warning;
 extern int advice_resolve_conflict;
diff --git a/wt-status.c b/wt-status.c
index d2a1bec226..c94d43879a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -19,6 +19,8 @@
 #include "lockfile.h"
 #include "sequencer.h"
 
+#define AB_DELAY_WARNING_IN_MS (2 * 1000)
+
 static const char cut_line[] =
 "------------------------ >8 ------------------------\n";
 
@@ -1085,14 +1087,29 @@ static void wt_longstatus_print_tracking(struct wt_status *s)
 	struct branch *branch;
 	char comment_line_string[3];
 	int i;
+	uint64_t t_begin = 0;
 
 	assert(s->branch && !s->is_initial);
 	if (!skip_prefix(s->branch, "refs/heads/", &branch_name))
 		return;
 	branch = branch_get(branch_name);
+
+	t_begin = getnanotime();
+
 	if (!format_tracking_info(branch, &sb, s->ahead_behind_flags))
 		return;
 
+	if (advice_status_ahead_behind_warning &&
+	    s->ahead_behind_flags == AHEAD_BEHIND_FULL) {
+		uint64_t t_delta_in_ms = (getnanotime() - t_begin) / 1000000;
+		if (t_delta_in_ms > AB_DELAY_WARNING_IN_MS) {
+			strbuf_addf(&sb, _("\n"
+					   "It took %.2f seconds to compute the branch ahead/behind values.\n"
+					   "You can use '--no-ahead-behind' to avoid this.\n"),
+				    t_delta_in_ms / 1000.0);
+		}
+	}
+
 	i = 0;
 	if (s->display_comment_prefix) {
 		comment_line_string[i++] = comment_line_char;
-- 
gitgitgadget

[PATCH 07/11] repo-settings: status.aheadBehind=false

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

When a repo has many active developers, the commit history can grow
very quickly. This can lead remote branches from being very far from
their local copies.

Set stats.aheadBehind=false by default when core.size=large, so all
'git status' calls have an implied '--no-ahead-behind' argument.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt   |  3 +++
 Documentation/config/status.txt |  3 ++-
 builtin/commit.c                | 12 ++++++------
 repo-settings.c                 |  6 ++++++
 repo-settings.h                 |  1 +
 5 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index df357f5af5..6bed956a08 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -620,3 +620,6 @@ core.size::
 * `pack.useSparse=true` uses the sparse tree-walk algorithm, which is
 optimized for enumerating objects during linkgit:git-push[1] from a
 client machine.
++
+* `status.aheadBehind=false` enables `--no-ahead-behind` by default during
+linkgit:git-status[1] calls, saving time in a fast-moving commit history.
diff --git a/Documentation/config/status.txt b/Documentation/config/status.txt
index 0fc704ab80..3e39019810 100644
--- a/Documentation/config/status.txt
+++ b/Documentation/config/status.txt
@@ -15,7 +15,8 @@ status.branch::
 status.aheadBehind::
 	Set to true to enable `--ahead-behind` and false to enable
 	`--no-ahead-behind` by default in linkgit:git-status[1] for
-	non-porcelain status formats.  Defaults to true.
+	non-porcelain status formats.  Defaults to true, unless
+	`core.size=large`.
 
 status.displayCommentPrefix::
 	If set to true, linkgit:git-status[1] will insert a comment
diff --git a/builtin/commit.c b/builtin/commit.c
index 79cb238d87..246a802167 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -36,6 +36,7 @@
 #include "help.h"
 #include "commit-reach.h"
 #include "commit-graph.h"
+#include "repo-settings.h"
 
 static const char * const builtin_commit_usage[] = {
 	N_("git commit [<options>] [--] <pathspec>..."),
@@ -1117,8 +1118,11 @@ static void finalize_deferred_config(struct wt_status *s)
 	 * in particular), we inherit _FULL for backwards compatibility.
 	 */
 	if (use_deferred_config &&
-	    s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
-		s->ahead_behind_flags = status_deferred_config.ahead_behind;
+	    s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED) {
+		prepare_repo_settings(the_repository);
+		if (the_repository->settings->status_ahead_behind != -1)
+			s->ahead_behind_flags = the_repository->settings->status_ahead_behind;
+	}
 
 	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
 		s->ahead_behind_flags = AHEAD_BEHIND_FULL;
@@ -1259,10 +1263,6 @@ static int git_status_config(const char *k, const char *v, void *cb)
 		status_deferred_config.show_branch = git_config_bool(k, v);
 		return 0;
 	}
-	if (!strcmp(k, "status.aheadbehind")) {
-		status_deferred_config.ahead_behind = git_config_bool(k, v);
-		return 0;
-	}
 	if (!strcmp(k, "status.showstash")) {
 		s->show_stash = git_config_bool(k, v);
 		return 0;
diff --git a/repo-settings.c b/repo-settings.c
index 026ab9c1a0..b3d4b50b72 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -15,6 +15,7 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 			UPDATE_DEFAULT(rs->core_commit_graph, 1);
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
 			UPDATE_DEFAULT(rs->pack_use_sparse, 1);
+			UPDATE_DEFAULT(rs->status_ahead_behind, 1);
 			UPDATE_DEFAULT(rs->index_version, 4);
 		}
 		return 0;
@@ -31,6 +32,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->pack_use_sparse = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "status.aheadbehind")) {
+		rs->status_ahead_behind = git_config_bool(key, value);
+		return 0;
+	}
 	if (!strcmp(key, "index.version")) {
 		rs->index_version = git_config_int(key, value);
 		return 0;
@@ -50,6 +55,7 @@ void prepare_repo_settings(struct repository *r)
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
 	r->settings->pack_use_sparse = -1;
+	r->settings->status_ahead_behind = -1;
 	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
diff --git a/repo-settings.h b/repo-settings.h
index b50228f992..cc358a083a 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -5,6 +5,7 @@ struct repo_settings {
 	char core_commit_graph;
 	char gc_write_commit_graph;
 	char pack_use_sparse;
+	char status_ahead_behind;
 	int index_version;
 };
 
-- 
gitgitgadget

[PATCH 06/11] status: ignore status.aheadbehind in porcelain formats

[Jeff Hostetler via GitGitGadget]

From: Jeff Hostetler <jeffhost@microsoft.com>

Teach porcelain V[12] formats to ignore the status.aheadbehind
config setting. They only respect the --[no-]ahead-behind
command line argument.  This is for backwards compatibility
with existing scripts.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/commit.c        | 10 ++++++----
 t/t7064-wtstatus-pv2.sh | 12 ++++++++----
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 71305073ad..79cb238d87 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1111,11 +1111,13 @@ static void finalize_deferred_config(struct wt_status *s)
 
 	/*
 	 * If the user did not give a "--[no]-ahead-behind" command
-	 * line argument, then we inherit the a/b config setting.
-	 * If is not set, then we inherit _FULL for backwards
-	 * compatibility.
+	 * line argument *AND* we will print in a human-readable format
+	 * (short, long etc.) then we inherit from the status.aheadbehind
+	 * config setting.  In all other cases (and porcelain V[12] formats
+	 * in particular), we inherit _FULL for backwards compatibility.
 	 */
-	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
+	if (use_deferred_config &&
+	    s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
 		s->ahead_behind_flags = status_deferred_config.ahead_behind;
 
 	if (s->ahead_behind_flags == AHEAD_BEHIND_UNSPECIFIED)
diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
index a0baf6e8b0..537787e598 100755
--- a/t/t7064-wtstatus-pv2.sh
+++ b/t/t7064-wtstatus-pv2.sh
@@ -436,10 +436,6 @@ test_expect_success 'verify --[no-]ahead-behind with V2 format' '
 		git status --no-ahead-behind --porcelain=v2 --branch --untracked-files=all >actual &&
 		test_cmp expect actual &&
 
-		# Confirmat that "status.aheadbehind" works on V2 format.
-		git -c status.aheadbehind=false status --porcelain=v2 --branch --untracked-files=all >actual &&
-		test_cmp expect actual &&
-
 		# Confirm --ahead-behind reports traditional branch.ab with 1/0.
 		cat >expect <<-EOF &&
 		# branch.oid $HUF
@@ -449,6 +445,14 @@ test_expect_success 'verify --[no-]ahead-behind with V2 format' '
 		EOF
 
 		git status --ahead-behind --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
+		# Confirm that "status.aheadbehind" DOES NOT work on V2 format.
+		git -c status.aheadbehind=false status --porcelain=v2 --branch --untracked-files=all >actual &&
+		test_cmp expect actual &&
+
+		# Confirm that "status.aheadbehind" DOES NOT work on V2 format.
+		git -c status.aheadbehind=true status --porcelain=v2 --branch --untracked-files=all >actual &&
 		test_cmp expect actual
 	)
 '
-- 
gitgitgadget

[PATCH 08/11] fetch: add --[no-]show-forced-updates argument

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

After updating a set of remove refs during a 'git fetch', we walk the
commits in the new ref value and not in the old ref value to discover
if the update was a forced update. This results in two things happening
during the command:

 1. The line including the ref update has an additional "(forced-update)"
    marker at the end.

 2. The ref log for that remote branch includes a bit saying that update
    is a forced update.

For many situations, this forced-update message happens infrequently, or
is a small bit of information among many ref updates. Many users ignore
these messages, but the calculation required here slows down their fetches
significantly. Keep in mind that they do not have the opportunity to
calculate a commit-graph file containing the newly-fetched commits, so
these comparisons can be very slow.

Add a '--[no-]show-forced-updates' option that allows a user to skip this
calculation. The only permanent result is dropping the forced-update bit
in the reflog.

Include a new fetch.showForcedUpdates config setting that allows this
behavior without including the argument in every command. The config
setting is overridden by the command-line arguments.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/fetch.txt  |  5 +++++
 Documentation/fetch-options.txt | 13 +++++++++++++
 builtin/fetch.c                 | 11 ++++++++++-
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
index cbfad6cdbb..ba890b5884 100644
--- a/Documentation/config/fetch.txt
+++ b/Documentation/config/fetch.txt
@@ -63,3 +63,8 @@ fetch.negotiationAlgorithm::
 	Unknown values will cause 'git fetch' to error out.
 +
 See also the `--negotiation-tip` option for linkgit:git-fetch[1].
+
+fetch.showForcedUpdates::
+	Set to false to enable `--no-show-forced-updates` in
+	linkgit:git-fetch[1] and linkgit:git-pull[1] commands.
+	Defaults to true.
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 91c47752ec..5801d23ae4 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -221,6 +221,19 @@ endif::git-pull[]
 	When multiple `--server-option=<option>` are given, they are all
 	sent to the other side in the order listed on the command line.
 
+--show-forced-updates::
+	By default, git checks if a branch is force-updated during
+	fetch. This can be disabled through fetch.showForcedUpdates, but
+	the --show-forced-updates option guarantees this check occurs.
+	See linkgit:git-config[1].
+
+--no-show-forced-updates::
+	By default, git checks if a branch is force-updated during
+	fetch. Pass --no-show-forced-updates or set fetch.showForcedUpdates
+	to false to skip this check for performance reasons. If used during
+	'git-pull' the --ff-only option will still check for forced updates
+	before attempting a fast-forward update. See linkgit:git-config[1].
+
 -4::
 --ipv4::
 	Use IPv4 addresses only, ignoring IPv6 addresses.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4ba63d5ac6..571c255218 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -39,6 +39,7 @@ enum {
 };
 
 static int fetch_prune_config = -1; /* unspecified */
+static int fetch_show_forced_updates = 1;
 static int prune = -1; /* unspecified */
 #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */
 
@@ -79,6 +80,11 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(k, "fetch.showforcedupdates")) {
+		fetch_show_forced_updates = git_config_bool(k, v);
+		return 0;
+	}
+
 	if (!strcmp(k, "submodule.recurse")) {
 		int r = git_config_bool(k, v) ?
 			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
@@ -169,6 +175,8 @@ static struct option builtin_fetch_options[] = {
 	OPT_STRING_LIST(0, "negotiation-tip", &negotiation_tip, N_("revision"),
 			N_("report that we have only objects reachable from this object")),
 	OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
+	OPT_BOOL(0, "show-forced-updates", &fetch_show_forced_updates,
+		 N_("check for forced-updates on all updated branches")),
 	OPT_END()
 };
 
@@ -773,9 +781,10 @@ static int update_local_ref(struct ref *ref,
 		return r;
 	}
 
-	if (in_merge_bases(current, updated)) {
+	if (!fetch_show_forced_updates || in_merge_bases(current, updated)) {
 		struct strbuf quickref = STRBUF_INIT;
 		int r;
+
 		strbuf_add_unique_abbrev(&quickref, &current->object.oid, DEFAULT_ABBREV);
 		strbuf_addstr(&quickref, "..");
 		strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
-- 
gitgitgadget

[PATCH 09/11] fetch: warn about forced updates after branch list

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/fetch.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 571c255218..f8ff98fdaf 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -24,6 +24,8 @@
 #include "list-objects-filter-options.h"
 #include "commit-reach.h"
 
+#define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
+
 static const char * const builtin_fetch_usage[] = {
 	N_("git fetch [<options>] [<repository> [<refspec>...]]"),
 	N_("git fetch [<options>] <group>"),
@@ -40,6 +42,7 @@ enum {
 
 static int fetch_prune_config = -1; /* unspecified */
 static int fetch_show_forced_updates = 1;
+static uint64_t forced_updates_ms = 0;
 static int prune = -1; /* unspecified */
 #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */
 
@@ -707,6 +710,7 @@ static int update_local_ref(struct ref *ref,
 	enum object_type type;
 	struct branch *current_branch = branch_get(NULL);
 	const char *pretty_ref = prettify_refname(ref->name);
+	int fast_forward = 0;
 
 	type = oid_object_info(the_repository, &ref->new_oid, NULL);
 	if (type < 0)
@@ -781,7 +785,15 @@ static int update_local_ref(struct ref *ref,
 		return r;
 	}
 
-	if (!fetch_show_forced_updates || in_merge_bases(current, updated)) {
+	if (fetch_show_forced_updates) {
+		uint64_t t_before = getnanotime();
+		fast_forward = in_merge_bases(current, updated);
+		forced_updates_ms += (getnanotime() - t_before) / 1000000;
+	} else {
+		fast_forward = 1;
+	}
+
+	if (fast_forward) {
 		struct strbuf quickref = STRBUF_INIT;
 		int r;
 
@@ -980,6 +992,17 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 		      " 'git remote prune %s' to remove any old, conflicting "
 		      "branches"), remote_name);
 
+	if (!fetch_show_forced_updates) {
+		warning(_("Fetch normally indicates which branches had a forced update, but that check has been disabled."));
+		warning(_("To re-enable, use '--show-forced-updates' flag or run 'git config fetch.showForcedUpdates true'."));
+	}
+	if (fetch_show_forced_updates &&
+	    forced_updates_ms > FORCED_UPDATES_DELAY_WARNING_IN_MS) {
+		warning(_("It took %.2f seconds to check forced updates. You can use '--no-show-forced-updates'\n"),
+			forced_updates_ms / 1000.0);
+		warning(_("or run 'git config fetch.showForcedUpdates false' to avoid this check.\n"));
+	}
+
  abort:
 	strbuf_release(&note);
 	free(url);
-- 
gitgitgadget

[PATCH 10/11] pull: add --[no-]show-forced-updates passthrough to fetch

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 builtin/pull.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/builtin/pull.c b/builtin/pull.c
index 9dd32a115b..f1eaf6e6ed 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -128,6 +128,7 @@ static char *opt_update_shallow;
 static char *opt_refmap;
 static char *opt_ipv4;
 static char *opt_ipv6;
+static int opt_show_forced_updates = -1;
 
 static struct option pull_options[] = {
 	/* Shared options */
@@ -240,6 +241,8 @@ static struct option pull_options[] = {
 	OPT_PASSTHRU('6',  "ipv6", &opt_ipv6, NULL,
 		N_("use IPv6 addresses only"),
 		PARSE_OPT_NOARG),
+	OPT_BOOL(0, "show-forced-updates", &opt_show_forced_updates,
+		 N_("check for forced-updates on all updated branches")),
 
 	OPT_END()
 };
@@ -549,6 +552,10 @@ static int run_fetch(const char *repo, const char **refspecs)
 		argv_array_push(&args, opt_ipv4);
 	if (opt_ipv6)
 		argv_array_push(&args, opt_ipv6);
+	if (opt_show_forced_updates > 0)
+		argv_array_push(&args, "--show-forced-updates");
+	else if (opt_show_forced_updates == 0)
+		argv_array_push(&args, "--no-show-forced-updates");
 
 	if (repo) {
 		argv_array_push(&args, repo);
-- 
gitgitgadget

[PATCH 11/11] repo-settings: fetch.showForcedUpdates=false

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

When a repo has many active contributors, the commit history can
grow very quickly and there can be many branches. During 'git fetch',
after downloading the pack from the remote, the default behavior
checks each remote ref for a forced update. This presents a
somewhat quadratic performance hit, as the time it takes to walk these
commits can be on the order of the number of branches times the
"commit velocity" per branch.

Set fetch.showForcedUpdates=false when core.size=large to enable
the --no-show-forced-updates option by default for fetch and pull
commands.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt  |  5 +++++
 Documentation/config/fetch.txt |  2 +-
 builtin/fetch.c                | 10 +++++-----
 repo-settings.c                |  6 ++++++
 repo-settings.h                |  1 +
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 6bed956a08..5733128d48 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -623,3 +623,8 @@ client machine.
 +
 * `status.aheadBehind=false` enables `--no-ahead-behind` by default during
 linkgit:git-status[1] calls, saving time in a fast-moving commit history.
++
+* `fetch.showForcedUpdates=false` enables `--no-show-forced-updates` by
+default during linkgit:git-fetch[1] and linkgit:git-pull[1] calls, saving
+time in a fast-moving commit history. This has a small side-effect of not
+updating the forced-update bit in the reflog.
diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
index ba890b5884..b7a3b08854 100644
--- a/Documentation/config/fetch.txt
+++ b/Documentation/config/fetch.txt
@@ -67,4 +67,4 @@ See also the `--negotiation-tip` option for linkgit:git-fetch[1].
 fetch.showForcedUpdates::
 	Set to false to enable `--no-show-forced-updates` in
 	linkgit:git-fetch[1] and linkgit:git-pull[1] commands.
-	Defaults to true.
+	Defaults to true, unless `core.size=large`.
diff --git a/builtin/fetch.c b/builtin/fetch.c
index f8ff98fdaf..56683ece17 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -23,6 +23,7 @@
 #include "packfile.h"
 #include "list-objects-filter-options.h"
 #include "commit-reach.h"
+#include "repo-settings.h"
 
 #define FORCED_UPDATES_DELAY_WARNING_IN_MS (10 * 1000)
 
@@ -83,11 +84,6 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
 		return 0;
 	}
 
-	if (!strcmp(k, "fetch.showforcedupdates")) {
-		fetch_show_forced_updates = git_config_bool(k, v);
-		return 0;
-	}
-
 	if (!strcmp(k, "submodule.recurse")) {
 		int r = git_config_bool(k, v) ?
 			RECURSE_SUBMODULES_ON : RECURSE_SUBMODULES_OFF;
@@ -1618,6 +1614,10 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	fetch_config_from_gitmodules(&max_children, &recurse_submodules);
 	git_config(git_fetch_config, NULL);
 
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings->fetch_show_forced_updates != -1)
+		fetch_show_forced_updates = the_repository->settings->fetch_show_forced_updates;
+
 	argc = parse_options(argc, argv, prefix,
 			     builtin_fetch_options, builtin_fetch_usage, 0);
 
diff --git a/repo-settings.c b/repo-settings.c
index b3d4b50b72..84e87913a5 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -16,6 +16,7 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
 			UPDATE_DEFAULT(rs->pack_use_sparse, 1);
 			UPDATE_DEFAULT(rs->status_ahead_behind, 1);
+			UPDATE_DEFAULT(rs->fetch_show_forced_updates, 1);
 			UPDATE_DEFAULT(rs->index_version, 4);
 		}
 		return 0;
@@ -36,6 +37,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->status_ahead_behind = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "fetch.showforcedupdates")) {
+		rs->fetch_show_forced_updates = git_config_bool(key, value);
+		return 0;
+	}
 	if (!strcmp(key, "index.version")) {
 		rs->index_version = git_config_int(key, value);
 		return 0;
@@ -56,6 +61,7 @@ void prepare_repo_settings(struct repository *r)
 	r->settings->gc_write_commit_graph = -1;
 	r->settings->pack_use_sparse = -1;
 	r->settings->status_ahead_behind = -1;
+	r->settings->fetch_show_forced_updates = -1;
 	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
diff --git a/repo-settings.h b/repo-settings.h
index cc358a083a..ea7d27138f 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -6,6 +6,7 @@ struct repo_settings {
 	char gc_write_commit_graph;
 	char pack_use_sparse;
 	char status_ahead_behind;
+	char fetch_show_forced_updates;
 	int index_version;
 };
 
-- 
gitgitgadget

Re: [PATCH 01/11] repo-settings: create repo.size=large setting

[Jeff Hostetler]

On 6/3/2019 4:18 PM, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@microsoft.com>
> 
> Several advanced config settings are highly recommended for clients
> using large repositories. Power users learn these one-by-one and
> enable them as they see fit. This could be made simpler, to allow
> more users to have access to these almost-always beneficial features
> (and more beneficial in larger repos).
> 
> Create a 'repo.size' config setting whose only accepted value is
> 'large'. When a repo.size=large is given, change the default values
> of some config settings. If the setting is given explicitly, then
> take the explicit value.
> 
> This change adds these two defaults to the repo.size=large setting:
> 
>   * core.commitGraph=true
>   * gc.writeCommitGraph=true
> 
> To centralize these config options and properly set the defaults,
> create a repo_settings that contains chars for each config variable.
> Use -1 as "unset", with 0 for false and 1 for true.
> 
> The prepare_repo_settings() method ensures that this settings
> struct has been initialized, and avoids double-scanning the config
> settings.
> 
> Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
[...]
> diff --git a/repo-settings.c b/repo-settings.c
> new file mode 100644
> index 0000000000..6f5e18d92e
> --- /dev/null
> +++ b/repo-settings.c
> @@ -0,0 +1,44 @@
> +#include "cache.h"
> +#include "repository.h"
> +#include "config.h"
> +#include "repo-settings.h"
> +
> +
> +#define UPDATE_DEFAULT(s,v) if (s != -1) { s = v; }

We should guard this with a "do { ... } while (0)"

> +
> +static int git_repo_config(const char *key, const char *value, void *cb)
> +{
> +	struct repo_settings *rs = (struct repo_settings *)cb;
> +
> +	if (!strcmp(key, "core.size")) {
> +		if (!strcmp(value, "large")) {
> +			UPDATE_DEFAULT(rs->core_commit_graph, 1);
> +			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
> +		}
> +		return 0;
> +	}
> +	if (!strcmp(key, "core.commitgraph")) {
> +		rs->core_commit_graph = git_config_bool(key, value);
> +		return 0;
> +	}
> +	if (!strcmp(key, "gc.writecommitgraph")) {
> +		rs->gc_write_commit_graph = git_config_bool(key, value);
> +		return 0;
> +	}
> +
> +	return 1;
> +}
[...]

Jeff

Re: [PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[Derrick Stolee]

On 6/3/2019 4:18 PM, Derrick Stolee via GitGitGadget wrote:
>  1. (Patches 1-3) Introduce a new 'core.size' config setting that takes
>     'large' as a value. 

I do want to point out that this "core.size=large" option is probably a
terrible name and could easily be replaced with something better. Please
consider alternatives that better describe the goals at hand (helping users
get performance boosts on upgrade without needing to pay close attention).

Thanks,
-Stolee

Re: [PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[Johannes Schindelin]

Hi Stolee,

On Mon, 3 Jun 2019, Derrick Stolee via GitGitGadget wrote:

>  1. (Patches 1-3) Introduce a new 'core.size' config setting that takes
>     'large' as a value. This enables several config values that are
>     beneficial for large repos.

I find `core.size` a bit non-descriptive. Maybe `repository.size` instead?

Ciao,
Dscho

Re: [PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[Derrick Stolee]

On 6/4/2019 10:43 AM, Johannes Schindelin wrote:
> Hi Stolee,
> 
> On Mon, 3 Jun 2019, Derrick Stolee via GitGitGadget wrote:
> 
>>  1. (Patches 1-3) Introduce a new 'core.size' config setting that takes
>>     'large' as a value. This enables several config values that are
>>     beneficial for large repos.
> 
> I find `core.size` a bit non-descriptive. Maybe `repository.size` instead?

Thanks for the suggestion! If the "repository." doesn't make sense as a top-
level category, then maybe "core.repositorySize" would work?

A thought I had overnight that may broaden our options would be to think of
this as a tolerance for experimental features. Maybe "core.adoptionRing" with
options for "slow" and "fast", where "slow" takes things that have been cooking
a long while (index.version=4, core.commitGraph=gc.writeCommitGraph=1) and the
"fast" option gets all of those values plus the more experimental options
(status.aheadBehind=false, fetch.showForcedUpdates=false).

Alternate names with this slow/fast idea could be:

* core.experimentTolerance={none,low,high}

* core.autoConfig={none,some,all}

Hopefully these options can trigger some creativity to decide on a good
name that an experienced Git user could understand.

Thanks,
-Stolee

Re: [PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This patch series includes a few new config options we created to speed up
> certain critical commands in VFS for Git. On their own, they would
> contribute little value as it is hard to discover new config variables.
> Instead, I've created this RFC as a goal for probably three sequential patch
> series:
>
>  1. (Patches 1-3) Introduce a new 'core.size' config setting that takes
>     'large' as a value. This enables several config values that are
>     beneficial for large repos. We use a certain set in VFS for Git (see
>     [1]), and most of those are applicable to any repo. This 'core.size'
>     setting is intended for users to automatically receive performance
>     updates as soon as they are stable, but they must opt-in to the setting
>     and can always explicitly set their own config values. The settings to
>     include here are core.commitGraph=true, gc.writeCommitGraph=true,
>     index.version=4, pack.useSparse=true.

... and not the configuration introduced by the other two points in
this list?

"If you set this, these other configuration variables are set to
these default values" is a very valuable usability feature.  It
looks a lot more "meta" or "macro", and certainly is not a good idea
to call it as if it sits next to variables in any existing hierarchy.

I also wonder if this is something we would want to support in
general; random things that come to mind are:

 - should such a "macro" configuration be limited to boolean
   (e.g. the above core.size that takes 'large' is a boolean between
   'large' and 'not large'), or can it be an enum (e.g. choose among
   'large', 'medium' and 'small', and core.bigFileThreshold will be
   set to 1G, 512M and 128M respectively---this silly example is for
   illustration purposes only), and if so, can we express what these
   default values are for each choice without writing a lot of code?

 - if we were to have more than just this 'core.size' macro, can two
   otherwise orthogonal macros both control the same underlying
   variable, and if so, how do we express their interactions?
   "using these two at the same time is forbidden" is a perfectly
   acceptable answer for the first round until we figure out the
   desired semantics, of course.

 - perhaps we may eventually want to allow end users (via their
   ~/.gitconfig) and system administrators (via /etc/gitconfig)
   define such a macro setting (e.g. setting macro.largeRepoSetting
   sets pack.usebitmaps=true, pack.useSpars=true, etc.) *after* we
   figure out what we want to do to the other points in this list.

 - even if we do not allow end users and system administrators futz
   with custom macros, can we specify the macros we ship without
   casting them in code?

Re: [PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[Derrick Stolee]

On 6/5/2019 4:39 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> This patch series includes a few new config options we created to speed up
>> certain critical commands in VFS for Git. On their own, they would
>> contribute little value as it is hard to discover new config variables.
>> Instead, I've created this RFC as a goal for probably three sequential patch
>> series:
>>
>>  1. (Patches 1-3) Introduce a new 'core.size' config setting that takes
>>     'large' as a value. This enables several config values that are
>>     beneficial for large repos. We use a certain set in VFS for Git (see
>>     [1]), and most of those are applicable to any repo. This 'core.size'
>>     setting is intended for users to automatically receive performance
>>     updates as soon as they are stable, but they must opt-in to the setting
>>     and can always explicitly set their own config values. The settings to
>>     include here are core.commitGraph=true, gc.writeCommitGraph=true,
>>     index.version=4, pack.useSparse=true.
> 
> ... and not the configuration introduced by the other two points in
> this list?

They are added to the config setting after they are introduced. See
patches 7 (status.aheadBehind) and 11 (fetch.showForcedUpdates).

> "If you set this, these other configuration variables are set to
> these default values" is a very valuable usability feature.  It
> looks a lot more "meta" or "macro", and certainly is not a good idea
> to call it as if it sits next to variables in any existing hierarchy.
> 
> I also wonder if this is something we would want to support in
> general; random things that come to mind are:
> 
>  - should such a "macro" configuration be limited to boolean
>    (e.g. the above core.size that takes 'large' is a boolean between
>    'large' and 'not large'), or can it be an enum (e.g. choose among
>    'large', 'medium' and 'small', and core.bigFileThreshold will be
>    set to 1G, 512M and 128M respectively---this silly example is for
>    illustration purposes only), and if so, can we express what these
>    default values are for each choice without writing a lot of code?

That's a good point that we could include recommended values for
other non-boolean variables if our "meta" config setting is also
non-boolean. This fits in with the "ring" ideas discussed earlier [1].
Taking in a few ideas from your message, perhaps we create a new "meta"
category for this setting and use an integer value for "how big do I
think my repo is?" and we can apply different settings based on thresholds:

 0: no config defaults changed
 3: safe defaults (core.commitGraph, index.version=4)
 6: behavior-modifying defaults (status.aheadBehind, fetch.noShowForcedUpdates)

Using 3 and 6 here to allow for finer gradients at a later date.

[1] https://public-inbox.org/git/xmqqftonsr6a.fsf@gitster-ct.c.googlers.com/T/#m8dbaedc016ce7301b9d80e5ceb6a82edfa7bafac

>  - if we were to have more than just this 'core.size' macro, can two
>    otherwise orthogonal macros both control the same underlying
>    variable, and if so, how do we express their interactions?
>    "using these two at the same time is forbidden" is a perfectly
>    acceptable answer for the first round until we figure out the
>    desired semantics, of course.

To borrow from linear algebra, I would recommend that two orthogonal
config settings have disjoint _bases_ (i.e. the set of config settings
they use are disjoint). Of course, this can be discussed in more
detail when someone suggests a second meta-config setting. Such a
second setting would need justification for why it doesn't work with
our first setting.
 
>  - perhaps we may eventually want to allow end users (via their
>    ~/.gitconfig) and system administrators (via /etc/gitconfig)
>    define such a macro setting (e.g. setting macro.largeRepoSetting
>    sets pack.usebitmaps=true, pack.useSpars=true, etc.) *after* we
>    figure out what we want to do to the other points in this list.
>
>  - even if we do not allow end users and system administrators futz
>    with custom macros, can we specify the macros we ship without
>    casting them in code?

Are you suggesting that we allow some config values to be pulled from
the repo contents? If we could identify some config options as "safe"
to include in the Git data, then a repo administrator could commit a
"/.gitconfig" file _and_ some existing config option says "look at the
config in the repo".

I see value in making some "safe" settings available in the repo, but
also see that it can be very tricky to get right. Further, I think it
is independent of the current direction. In fact, I would imagine the
meta-config setting be one of the "safe" settings that we could put in
this committed config file.

Thanks,
-Stolee
 

Re: [PATCH 00/11] [RFC] Create 'core.size=large' setting to update config defaults

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

>>  - perhaps we may eventually want to allow end users (via their
>>    ~/.gitconfig) and system administrators (via /etc/gitconfig)
>>    define such a macro setting (e.g. setting macro.largeRepoSetting
>>    sets pack.usebitmaps=true, pack.useSpars=true, etc.) *after* we
>>    figure out what we want to do to the other points in this list.
>>
>>  - even if we do not allow end users and system administrators futz
>>    with custom macros, can we specify the macros we ship without
>>    casting them in code?
>
> Are you suggesting that we allow some config values to be pulled from
> the repo contents?

Not at all.  As far as the configuration is concerned, what project
ships is tainted data that should not be used blindly.

What I had in mind is parallel to the idea of pushing "static struct
userdiff_driver builtin_drivers[]" out of the compiled-in code and
instead have a text file shipped in /usr/share/git/ somewhere.  So,
instead of having "core.size==large means these other four variables
are set to these values" in the code, we invent a general mechanism
to read such "macro" specification out of a text file, and that
would be the only code change---the specific "core.size==large
affects X, Y and Z" would not be in the code, but would be in the
text file we ship and read by the mechanism.

If the list of allowed "meta" configuration variables and the
configuration variables whose default each of them affects can be
expressed in our usual ".gitconfig" file format, then the system
administrators can add their own in /etc/gitconfig, too, to help
their users.

That is what I meant by the last item.  Note that I was "wondering
if it makes sense" and what I wrote above in this message is merely
clarifying what I meant---I am not making further/more arguments to
claim it is a good idea (at least not yet).

Thanks.

[PATCH v2 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Derrick Stolee via GitGitGadget]

Here is a second run at this RFC, which aims to create a "meta" config
setting that automatically turns on other settings according to a user's
willingness to trade new Git behavior or new feature risk for performance
benefits. The new name for the setting is "core.featureAdoptionRate" and is
an integer scale from 0 to 10. There will be multiple "categories" of
settings, and the intention is to allow more granular levels as necessary.

The first category is "3 or higher" which means that the user is willing to
adopt features that have been tested in multiple major releases. The
settings to include here are core.commitGraph=true,
gc.writeCommitGraph=true, and index.version=4.

The second category is "5 or higher" which means the user is willing to
adopt features that have not been out for multiple major releases. The
setting included here is pack.useSparse=true.

In the future, I would add a "7 or higher" setting which means the user is
willing to have a change of behavior in exchange for performance benefits.
The two settings to place here are 'status.aheadBehind=false' and
'fetch.showForcedUpdates=false'. Instead of including these settings in the
current series, I've submitted them independently for full review [1, 2].

Hopefully this direction is amenable to allow "early adopters" gain access
to new performance features even if they are not necessary reading every
line of the release notes.

Thanks, -Stolee

[1] https://public-inbox.org/git/pull.272.git.gitgitgadget@gmail.com/

[2] https://public-inbox.org/git/pull.273.git.gitgitgadget@gmail.com/

Derrick Stolee (3):
  repo-settings: create core.featureAdoptionRate setting
  repo-settings: use index.version=4 by default
  repo-settings: pack.useSparse=true

 Documentation/config/core.txt  | 33 ++++++++++++++++++-
 Documentation/config/gc.txt    |  4 +--
 Documentation/config/index.txt |  2 ++
 Documentation/config/pack.txt  |  3 +-
 Makefile                       |  1 +
 builtin/gc.c                   |  6 ++--
 builtin/pack-objects.c         |  9 +++---
 commit-graph.c                 |  7 ++--
 read-cache.c                   | 12 ++++---
 repo-settings.c                | 58 ++++++++++++++++++++++++++++++++++
 repo-settings.h                | 15 +++++++++
 repository.h                   |  3 ++
 12 files changed, 134 insertions(+), 19 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h


base-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-254%2Fderrickstolee%2Fconfig-large%2Fupstream-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-254/derrickstolee/config-large/upstream-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/254

Range-diff vs v1:

  1:  704613f448 !  1:  bdaee3ea9d repo-settings: create repo.size=large setting
     @@ -1,6 +1,6 @@
      Author: Derrick Stolee <dstolee@microsoft.com>
      
     -    repo-settings: create repo.size=large setting
     +    repo-settings: create core.featureAdoptionRate setting
      
          Several advanced config settings are highly recommended for clients
          using large repositories. Power users learn these one-by-one and
     @@ -8,16 +8,31 @@
          more users to have access to these almost-always beneficial features
          (and more beneficial in larger repos).
      
     -    Create a 'repo.size' config setting whose only accepted value is
     -    'large'. When a repo.size=large is given, change the default values
     -    of some config settings. If the setting is given explicitly, then
     -    take the explicit value.
     +    Create a 'core.featureAdoptionRate' config setting that allows integer
     +    values. This is a rating from 0 to 10 for the user's willingness to
     +    adopt new or experimental features that improve Git performance.
     +    The default is 0, meaning "don't change anything!" A value of 10
     +    would mean "I'm willing for some behavior to change to get the
     +    best performance I can get, and can take experimental features
     +    in their first release." As we integrate this with more config
     +    settings, we will make this scale more clear.
      
     -    This change adds these two defaults to the repo.size=large setting:
     +    This config setting only changes the default values of other config
     +    settings. If the setting is given explicitly, then take the
     +    explicit value.
     +
     +    This change adds these two defaults when core.featureAdoptionRate
     +    is at least three:
      
           * core.commitGraph=true
           * gc.writeCommitGraph=true
      
     +    The use of "three or higher" for these settings means that a value
     +    of 3 means "I'm willing to add optional features that can augment
     +    the data on disk in favor of improved performance, but those
     +    features should be stable after being included in multiple major
     +    releases."
     +
          To centralize these config options and properly set the defaults,
          create a repo_settings that contains chars for each config variable.
          Use -1 as "unset", with 0 for false and 1 for true.
     @@ -36,24 +51,29 @@
       core.commitGraph::
       	If true, then git will read the commit-graph file (if it exists)
      -	to parse the graph structure of commits. Defaults to false. See
     --	linkgit:git-commit-graph[1] for more information.
      +	to parse the graph structure of commits. Defaults to false, unless
     -+	`core.size=large`. See linkgit:git-commit-graph[1] for more
     -+	information.
     ++	`core.featureAdoptionRate` is at least three. See
     + 	linkgit:git-commit-graph[1] for more information.
       
       core.useReplaceRefs::
     - 	If set to `false`, behave as if the `--no-replace-objects`
      @@
       	in your repository, which hopefully is enough for
       	abbreviated object names to stay unique for some time.
       	The minimum length is 4.
      +
     -+core.size::
     -+	When specified as "large", change the default values of some config
     -+	variables to improve performance in a large repository. If a variable
     ++core.featureAdoptionRate::
     ++	Set an integer value on a scale from 0 to 10 describing your
     ++	desire to adopt new performance features. Defaults to 0. As
     ++	the value increases, features are enabled by changing the
     ++	default values of other config settings. If a config variable
      +	is specified explicitly, the explicit value will override these
      +	defaults:
      ++
     ++If the value is at least 3, then the following defaults are modified.
     ++These represent relatively new features that have existed for multiple
     ++major releases, and present significant performance benefits. They do
     ++not modify the user-facing output of porcelain commands.
     +++
      +* `core.commitGraph=true` enables reading commit-graph files.
      ++
      +* `gc.writeCommitGraph=true` eneables writing commit-graph files during
     @@ -68,8 +88,8 @@
       	the commit-graph will be updated if housekeeping is
      -	required. Default is false. See linkgit:git-commit-graph[1]
      -	for details.
     -+	required. Default is false, unless `core.size=large`.
     -+	See linkgit:git-commit-graph[1] for details.
     ++	required. Default is false, unless `core.featureAdoptionRage`
     ++	is at least three. See linkgit:git-commit-graph[1] for details.
       
       gc.logExpiry::
       	If the file gc.log exists, then `git gc --auto` will print
     @@ -167,15 +187,15 @@
      +#include "config.h"
      +#include "repo-settings.h"
      +
     -+
     -+#define UPDATE_DEFAULT(s,v) if (s != -1) { s = v; }
     ++#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
      +
      +static int git_repo_config(const char *key, const char *value, void *cb)
      +{
      +	struct repo_settings *rs = (struct repo_settings *)cb;
      +
     -+	if (!strcmp(key, "core.size")) {
     -+		if (!strcmp(value, "large")) {
     ++	if (!strcmp(key, "core.featureadoptionrate")) {
     ++		int rate = git_config_int(key, value);
     ++		if (rate >= 3) {
      +			UPDATE_DEFAULT(rs->core_commit_graph, 1);
      +			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
      +		}
  2:  d5f5d7453c !  2:  02c89415fe repo-settings: use index.version=4 by default
     @@ -4,7 +4,7 @@
      
          If a repo is large, it likely has many paths in its working directory.
          This means the index could be compressed using version 4. Set this as
     -    a default when core.size=large.
     +    a default when core.featureAdoptionRate is at least three.
      
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
      
     @@ -26,7 +26,8 @@
       index.version::
       	Specify the version with which new index files should be
       	initialized.  This does not affect existing repositories.
     -+	If `core.size=large`, then the default value is 4.
     ++	If `core.featureAdoptionRate` is at least three, then the
     ++	default value is 4.
      
       diff --git a/read-cache.c b/read-cache.c
       --- a/read-cache.c
     @@ -75,7 +76,7 @@
       --- a/repo-settings.c
       +++ b/repo-settings.c
      @@
     - 		if (!strcmp(value, "large")) {
     + 		if (rate >= 3) {
       			UPDATE_DEFAULT(rs->core_commit_graph, 1);
       			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
      +			UPDATE_DEFAULT(rs->index_version, 4);
  3:  f3ea4e3f27 !  3:  5bba9062f4 repo-settings: pack.useSparse=true
     @@ -7,7 +7,10 @@
          many fewer paths than the full path set. The sparse treewalk
          algorithm is optimized for this case, speeding up 'git push' calls.
      
     -    Use pack.useSparse=true when core.size=large.
     +    Use pack.useSparse=true when core.featureAdoptionRate is at least
     +    five. This is the first setting where the feature has only been
     +    out for a single major version. This could be moved to the "at
     +    least three" category after another major version.
      
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
      
     @@ -19,6 +22,11 @@
       * `index.version=4` uses prefix-compression to reduce the size of the
       .git/index file.
      ++
     ++If the value is at least 5, then all of the defaults above are included,
     ++plus the defaults below. These represent new features that present
     ++significant performance benefits, but may not have been released for
     ++multiple major versions.
     +++
      +* `pack.useSparse=true` uses the sparse tree-walk algorithm, which is
      +optimized for enumerating objects during linkgit:git-push[1] from a
      +client machine.
     @@ -32,7 +40,7 @@
       	that extra objects are added to the pack-file if the included
      -	commits contain certain types of direct renames.
      +	commits contain certain types of direct renames. Defaults to
     -+	false, unless `core.size=large`.
     ++	false, unless `core.featureAdoptionRate` is at least five.
       
       pack.writeBitmaps (deprecated)::
       	This is a deprecated synonym for `repack.writeBitmaps`.
     @@ -75,13 +83,15 @@
       --- a/repo-settings.c
       +++ b/repo-settings.c
      @@
     - 		if (!strcmp(value, "large")) {
     - 			UPDATE_DEFAULT(rs->core_commit_graph, 1);
       			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
     -+			UPDATE_DEFAULT(rs->pack_use_sparse, 1);
       			UPDATE_DEFAULT(rs->index_version, 4);
       		}
     ++		if (rate >= 5) {
     ++			UPDATE_DEFAULT(rs->pack_use_sparse, 1);
     ++		}
       		return 0;
     + 	}
     + 	if (!strcmp(key, "core.commitgraph")) {
      @@
       		rs->gc_write_commit_graph = git_config_bool(key, value);
       		return 0;
  4:  671cf092fd <  -:  ---------- status: add status.aheadbehind setting
  5:  d2e5cf1857 <  -:  ---------- status: add warning when a/b calculation takes too long for long/normal format
  6:  82ae00e495 <  -:  ---------- status: ignore status.aheadbehind in porcelain formats
  7:  936fae31b7 <  -:  ---------- repo-settings: status.aheadBehind=false
  8:  2d6bf8513d <  -:  ---------- fetch: add --[no-]show-forced-updates argument
  9:  6a80cfa5a5 <  -:  ---------- fetch: warn about forced updates after branch list
 10:  637aebc8ac <  -:  ---------- pull: add --[no-]show-forced-updates passthrough to fetch
 11:  d4ff987ad9 <  -:  ---------- repo-settings: fetch.showForcedUpdates=false

-- 
gitgitgadget

[PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

Several advanced config settings are highly recommended for clients
using large repositories. Power users learn these one-by-one and
enable them as they see fit. This could be made simpler, to allow
more users to have access to these almost-always beneficial features
(and more beneficial in larger repos).

Create a 'core.featureAdoptionRate' config setting that allows integer
values. This is a rating from 0 to 10 for the user's willingness to
adopt new or experimental features that improve Git performance.
The default is 0, meaning "don't change anything!" A value of 10
would mean "I'm willing for some behavior to change to get the
best performance I can get, and can take experimental features
in their first release." As we integrate this with more config
settings, we will make this scale more clear.

This config setting only changes the default values of other config
settings. If the setting is given explicitly, then take the
explicit value.

This change adds these two defaults when core.featureAdoptionRate
is at least three:

 * core.commitGraph=true
 * gc.writeCommitGraph=true

The use of "three or higher" for these settings means that a value
of 3 means "I'm willing to add optional features that can augment
the data on disk in favor of improved performance, but those
features should be stable after being included in multiple major
releases."

To centralize these config options and properly set the defaults,
create a repo_settings that contains chars for each config variable.
Use -1 as "unset", with 0 for false and 1 for true.

The prepare_repo_settings() method ensures that this settings
struct has been initialized, and avoids double-scanning the config
settings.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt | 21 ++++++++++++++++-
 Documentation/config/gc.txt   |  4 ++--
 Makefile                      |  1 +
 builtin/gc.c                  |  6 ++---
 commit-graph.c                |  7 +++---
 repo-settings.c               | 44 +++++++++++++++++++++++++++++++++++
 repo-settings.h               | 13 +++++++++++
 repository.h                  |  3 +++
 8 files changed, 90 insertions(+), 9 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 75538d27e7..6a9f707815 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -577,7 +577,8 @@ the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
 core.commitGraph::
 	If true, then git will read the commit-graph file (if it exists)
-	to parse the graph structure of commits. Defaults to false. See
+	to parse the graph structure of commits. Defaults to false, unless
+	`core.featureAdoptionRate` is at least three. See
 	linkgit:git-commit-graph[1] for more information.
 
 core.useReplaceRefs::
@@ -601,3 +602,21 @@ core.abbrev::
 	in your repository, which hopefully is enough for
 	abbreviated object names to stay unique for some time.
 	The minimum length is 4.
+
+core.featureAdoptionRate::
+	Set an integer value on a scale from 0 to 10 describing your
+	desire to adopt new performance features. Defaults to 0. As
+	the value increases, features are enabled by changing the
+	default values of other config settings. If a config variable
+	is specified explicitly, the explicit value will override these
+	defaults:
++
+If the value is at least 3, then the following defaults are modified.
+These represent relatively new features that have existed for multiple
+major releases, and present significant performance benefits. They do
+not modify the user-facing output of porcelain commands.
++
+* `core.commitGraph=true` enables reading commit-graph files.
++
+* `gc.writeCommitGraph=true` eneables writing commit-graph files during
+`git gc`.
diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 02b92b18b5..898263209c 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -63,8 +63,8 @@ gc.writeCommitGraph::
 	If true, then gc will rewrite the commit-graph file when
 	linkgit:git-gc[1] is run. When using `git gc --auto`
 	the commit-graph will be updated if housekeeping is
-	required. Default is false. See linkgit:git-commit-graph[1]
-	for details.
+	required. Default is false, unless `core.featureAdoptionRage`
+	is at least three. See linkgit:git-commit-graph[1] for details.
 
 gc.logExpiry::
 	If the file gc.log exists, then `git gc --auto` will print
diff --git a/Makefile b/Makefile
index 8a7e235352..2d3499d7ac 100644
--- a/Makefile
+++ b/Makefile
@@ -967,6 +967,7 @@ LIB_OBJS += refspec.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace-object.o
+LIB_OBJS += repo-settings.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
diff --git a/builtin/gc.c b/builtin/gc.c
index 8943bcc300..6281aad961 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -27,6 +27,7 @@
 #include "pack-objects.h"
 #include "blob.h"
 #include "tree.h"
+#include "repo-settings.h"
 
 #define FAILED_RUN "failed to run %s"
 
@@ -41,7 +42,6 @@ static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
 static int detach_auto = 1;
 static timestamp_t gc_log_expire_time;
 static const char *gc_log_expire = "1.day.ago";
@@ -148,7 +148,6 @@ static void gc_config(void)
 	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
 	git_config_get_int("gc.auto", &gc_auto_threshold);
 	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
-	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
 	git_config_get_bool("gc.autodetach", &detach_auto);
 	git_config_get_expiry("gc.pruneexpire", &prune_expire);
 	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
@@ -685,7 +684,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		clean_pack_garbage();
 	}
 
-	if (gc_write_commit_graph)
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings->gc_write_commit_graph == 1)
 		write_commit_graph_reachable(get_object_directory(), 0,
 					     !quiet && !daemonized);
 
diff --git a/commit-graph.c b/commit-graph.c
index 7c5e54875f..b09c465a7a 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -16,6 +16,7 @@
 #include "hashmap.h"
 #include "replace-object.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
 #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
@@ -311,7 +312,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
 static int prepare_commit_graph(struct repository *r)
 {
 	struct object_directory *odb;
-	int config_value;
 
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 		die("dying as requested by the '%s' variable on commit-graph load!",
@@ -321,9 +321,10 @@ static int prepare_commit_graph(struct repository *r)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
+	prepare_repo_settings(r);
+
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
-	    !config_value))
+	    r->settings->core_commit_graph != 1)
 		/*
 		 * This repository is not configured to use commit graphs, so
 		 * do not load one. (But report commit_graph_attempted anyway
diff --git a/repo-settings.c b/repo-settings.c
new file mode 100644
index 0000000000..f7fc2a1959
--- /dev/null
+++ b/repo-settings.c
@@ -0,0 +1,44 @@
+#include "cache.h"
+#include "repository.h"
+#include "config.h"
+#include "repo-settings.h"
+
+#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
+
+static int git_repo_config(const char *key, const char *value, void *cb)
+{
+	struct repo_settings *rs = (struct repo_settings *)cb;
+
+	if (!strcmp(key, "core.featureadoptionrate")) {
+		int rate = git_config_int(key, value);
+		if (rate >= 3) {
+			UPDATE_DEFAULT(rs->core_commit_graph, 1);
+			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+		}
+		return 0;
+	}
+	if (!strcmp(key, "core.commitgraph")) {
+		rs->core_commit_graph = git_config_bool(key, value);
+		return 0;
+	}
+	if (!strcmp(key, "gc.writecommitgraph")) {
+		rs->gc_write_commit_graph = git_config_bool(key, value);
+		return 0;
+	}
+
+	return 1;
+}
+
+void prepare_repo_settings(struct repository *r)
+{
+	if (r->settings)
+		return;
+
+	r->settings = xmalloc(sizeof(*r->settings));
+
+	/* Defaults */
+	r->settings->core_commit_graph = -1;
+	r->settings->gc_write_commit_graph = -1;
+
+	repo_config(r, git_repo_config, r->settings);
+}
diff --git a/repo-settings.h b/repo-settings.h
new file mode 100644
index 0000000000..11d08648e1
--- /dev/null
+++ b/repo-settings.h
@@ -0,0 +1,13 @@
+#ifndef REPO_SETTINGS_H
+#define REPO_SETTINGS_H
+
+struct repo_settings {
+	char core_commit_graph;
+	char gc_write_commit_graph;
+};
+
+struct repository;
+
+void prepare_repo_settings(struct repository *r);
+
+#endif /* REPO_SETTINGS_H */
diff --git a/repository.h b/repository.h
index 4fb6a5885f..352afc9cd8 100644
--- a/repository.h
+++ b/repository.h
@@ -4,6 +4,7 @@
 #include "path.h"
 
 struct config_set;
+struct repo_settings;
 struct git_hash_algo;
 struct index_state;
 struct lock_file;
@@ -72,6 +73,8 @@ struct repository {
 	 */
 	char *submodule_prefix;
 
+	struct repo_settings *settings;
+
 	/* Subsystems */
 	/*
 	 * Repository's config which contains key-value pairs from the usual
-- 
gitgitgadget

[PATCH v2 2/3] repo-settings: use index.version=4 by default

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

If a repo is large, it likely has many paths in its working directory.
This means the index could be compressed using version 4. Set this as
a default when core.featureAdoptionRate is at least three.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt  |  3 +++
 Documentation/config/index.txt |  2 ++
 read-cache.c                   | 12 +++++++-----
 repo-settings.c                |  6 ++++++
 repo-settings.h                |  1 +
 5 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 6a9f707815..d16503a9d7 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -620,3 +620,6 @@ not modify the user-facing output of porcelain commands.
 +
 * `gc.writeCommitGraph=true` eneables writing commit-graph files during
 `git gc`.
++
+* `index.version=4` uses prefix-compression to reduce the size of the
+.git/index file.
diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt
index f181503041..98a88c30be 100644
--- a/Documentation/config/index.txt
+++ b/Documentation/config/index.txt
@@ -24,3 +24,5 @@ index.threads::
 index.version::
 	Specify the version with which new index files should be
 	initialized.  This does not affect existing repositories.
+	If `core.featureAdoptionRate` is at least three, then the
+	default value is 4.
diff --git a/read-cache.c b/read-cache.c
index 22e7b9944e..7fab8ff748 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -25,6 +25,7 @@
 #include "fsmonitor.h"
 #include "thread-utils.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -1599,16 +1600,17 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate,
 
 #define INDEX_FORMAT_DEFAULT 3
 
-static unsigned int get_index_format_default(void)
+static unsigned int get_index_format_default(struct repository *r)
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
 	char *endp;
-	int value;
 	unsigned int version = INDEX_FORMAT_DEFAULT;
 
 	if (!envversion) {
-		if (!git_config_get_int("index.version", &value))
-			version = value;
+		prepare_repo_settings(r);
+
+		if (r->settings->index_version >= 0)
+			version = r->settings->index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
 			warning(_("index.version set, but the value is invalid.\n"
 				  "Using version %i"), INDEX_FORMAT_DEFAULT);
@@ -2765,7 +2767,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
 	}
 
 	if (!istate->version) {
-		istate->version = get_index_format_default();
+		istate->version = get_index_format_default(the_repository);
 		if (git_env_bool("GIT_TEST_SPLIT_INDEX", 0))
 			init_split_index(istate);
 	}
diff --git a/repo-settings.c b/repo-settings.c
index f7fc2a1959..5753153a84 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -14,6 +14,7 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		if (rate >= 3) {
 			UPDATE_DEFAULT(rs->core_commit_graph, 1);
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+			UPDATE_DEFAULT(rs->index_version, 4);
 		}
 		return 0;
 	}
@@ -25,6 +26,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->gc_write_commit_graph = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "index.version")) {
+		rs->index_version = git_config_int(key, value);
+		return 0;
+	}
 
 	return 1;
 }
@@ -39,6 +44,7 @@ void prepare_repo_settings(struct repository *r)
 	/* Defaults */
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
+	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
 }
diff --git a/repo-settings.h b/repo-settings.h
index 11d08648e1..9b8104042e 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -4,6 +4,7 @@
 struct repo_settings {
 	char core_commit_graph;
 	char gc_write_commit_graph;
+	int index_version;
 };
 
 struct repository;
-- 
gitgitgadget

[PATCH v2 3/3] repo-settings: pack.useSparse=true

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

If a repo is large, then it probably has a very large working
directory. In this case, a typical developer's edits usually impact
many fewer paths than the full path set. The sparse treewalk
algorithm is optimized for this case, speeding up 'git push' calls.

Use pack.useSparse=true when core.featureAdoptionRate is at least
five. This is the first setting where the feature has only been
out for a single major version. This could be moved to the "at
least three" category after another major version.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt | 9 +++++++++
 Documentation/config/pack.txt | 3 ++-
 builtin/pack-objects.c        | 9 +++++----
 repo-settings.c               | 8 ++++++++
 repo-settings.h               | 1 +
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index d16503a9d7..77fdd02660 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -623,3 +623,12 @@ not modify the user-facing output of porcelain commands.
 +
 * `index.version=4` uses prefix-compression to reduce the size of the
 .git/index file.
++
+If the value is at least 5, then all of the defaults above are included,
+plus the defaults below. These represent new features that present
+significant performance benefits, but may not have been released for
+multiple major versions.
++
+* `pack.useSparse=true` uses the sparse tree-walk algorithm, which is
+optimized for enumerating objects during linkgit:git-push[1] from a
+client machine.
diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
index 9cdcfa7324..9c4f8ea9ff 100644
--- a/Documentation/config/pack.txt
+++ b/Documentation/config/pack.txt
@@ -112,7 +112,8 @@ pack.useSparse::
 	objects. This can have significant performance benefits when
 	computing a pack to send a small change. However, it is possible
 	that extra objects are added to the pack-file if the included
-	commits contain certain types of direct renames.
+	commits contain certain types of direct renames. Defaults to
+	false, unless `core.featureAdoptionRate` is at least five.
 
 pack.writeBitmaps (deprecated)::
 	This is a deprecated synonym for `repack.writeBitmaps`.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 41d7fc5983..f26b3f2892 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -34,6 +34,7 @@
 #include "dir.h"
 #include "midx.h"
 #include "trace2.h"
+#include "repo-settings.h"
 
 #define IN_PACK(obj) oe_in_pack(&to_pack, obj)
 #define SIZE(obj) oe_size(&to_pack, obj)
@@ -2707,10 +2708,6 @@ 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.usesparse")) {
-		sparse = 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)
@@ -3330,6 +3327,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	read_replace_refs = 0;
 
 	sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
+	prepare_repo_settings(the_repository);
+	if (!sparse && the_repository->settings->pack_use_sparse != -1)
+		sparse = the_repository->settings->pack_use_sparse;
+
 	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 
diff --git a/repo-settings.c b/repo-settings.c
index 5753153a84..c700edc286 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -16,6 +16,9 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
 			UPDATE_DEFAULT(rs->index_version, 4);
 		}
+		if (rate >= 5) {
+			UPDATE_DEFAULT(rs->pack_use_sparse, 1);
+		}
 		return 0;
 	}
 	if (!strcmp(key, "core.commitgraph")) {
@@ -26,6 +29,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->gc_write_commit_graph = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "pack.usesparse")) {
+		rs->pack_use_sparse = git_config_bool(key, value);
+		return 0;
+	}
 	if (!strcmp(key, "index.version")) {
 		rs->index_version = git_config_int(key, value);
 		return 0;
@@ -44,6 +51,7 @@ void prepare_repo_settings(struct repository *r)
 	/* Defaults */
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
+	r->settings->pack_use_sparse = -1;
 	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
diff --git a/repo-settings.h b/repo-settings.h
index 9b8104042e..b50228f992 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -4,6 +4,7 @@
 struct repo_settings {
 	char core_commit_graph;
 	char gc_write_commit_graph;
+	char pack_use_sparse;
 	int index_version;
 };
 
-- 
gitgitgadget

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +core.featureAdoptionRate::
> +	Set an integer value on a scale from 0 to 10 describing your
> +	desire to adopt new performance features. Defaults to 0. As
> +	the value increases, features are enabled by changing the
> +	default values of other config settings. If a config variable
> +	is specified explicitly, the explicit value will override these
> +	defaults:
> ++
> +If the value is at least 3, then the following defaults are modified.
> +These represent relatively new features that have existed for multiple
> +major releases, and present significant performance benefits. They do
> +not modify the user-facing output of porcelain commands.
> ++
> +* `core.commitGraph=true` enables reading commit-graph files.
> ++
> +* `gc.writeCommitGraph=true` eneables writing commit-graph files during
> +`git gc`.

I was re-reading the whole series, and found that the phrase
"present significant benefits" was somewhat overselling.  Wouldn't
that claim largely depend on the end-user's workflow?  The same
comment applies to the description of "at least 5" level, too.

I would not mind if we say "enabling this may present performance
benefits", with or without "significant" before "performance
benefits", and with or without ", depending how your repository is
used" at the end.

Thanks.

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Derrick Stolee]

On 6/28/2019 4:50 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> +core.featureAdoptionRate::
>> +	Set an integer value on a scale from 0 to 10 describing your
>> +	desire to adopt new performance features. Defaults to 0. As
>> +	the value increases, features are enabled by changing the
>> +	default values of other config settings. If a config variable
>> +	is specified explicitly, the explicit value will override these
>> +	defaults:
>> ++
>> +If the value is at least 3, then the following defaults are modified.
>> +These represent relatively new features that have existed for multiple
>> +major releases, and present significant performance benefits. They do
>> +not modify the user-facing output of porcelain commands.
>> ++
>> +* `core.commitGraph=true` enables reading commit-graph files.
>> ++
>> +* `gc.writeCommitGraph=true` eneables writing commit-graph files during
>> +`git gc`.
> 
> I was re-reading the whole series, and found that the phrase
> "present significant benefits" was somewhat overselling.  Wouldn't
> that claim largely depend on the end-user's workflow?  The same
> comment applies to the description of "at least 5" level, too.
> 
> I would not mind if we say "enabling this may present performance
> benefits", with or without "significant" before "performance
> benefits", and with or without ", depending how your repository is
> used" at the end.

Thanks for taking such a close look. Indeed, it is not appropriate
to over-sell here. I will take another stab at this documentation
next week.

-Stolee

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Junio C Hamano]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> @@ -41,7 +42,6 @@ static int aggressive_depth = 50;
>  static int aggressive_window = 250;
>  static int gc_auto_threshold = 6700;
>  static int gc_auto_pack_limit = 50;
> -static int gc_write_commit_graph;
>  static int detach_auto = 1;
>  static timestamp_t gc_log_expire_time;
>  static const char *gc_log_expire = "1.day.ago";
> @@ -148,7 +148,6 @@ static void gc_config(void)
>  	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
>  	git_config_get_int("gc.auto", &gc_auto_threshold);
>  	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
> -	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
>  	git_config_get_bool("gc.autodetach", &detach_auto);
>  	git_config_get_expiry("gc.pruneexpire", &prune_expire);
>  	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
> @@ -685,7 +684,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>  		clean_pack_garbage();
>  	}
>  
> -	if (gc_write_commit_graph)
> +	prepare_repo_settings(the_repository);
> +	if (the_repository->settings->gc_write_commit_graph == 1)
>  		write_commit_graph_reachable(get_object_directory(), 0,
>  					     !quiet && !daemonized);

OK, so the general idea is to move per-subsystem local variables to
new fields in the repository structure, stop parsing the configuration
in per-subsystem config callback, which is how the configuration for
the writing side is done above, and ...

>  
> diff --git a/commit-graph.c b/commit-graph.c
> index 7c5e54875f..b09c465a7a 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -16,6 +16,7 @@
>  #include "hashmap.h"
>  #include "replace-object.h"
>  #include "progress.h"
> +#include "repo-settings.h"
>  
>  #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
>  #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
> @@ -311,7 +312,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
>  static int prepare_commit_graph(struct repository *r)
>  {
>  	struct object_directory *odb;
> -	int config_value;
>  
>  	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
>  		die("dying as requested by the '%s' variable on commit-graph load!",
> @@ -321,9 +321,10 @@ static int prepare_commit_graph(struct repository *r)
>  		return !!r->objects->commit_graph;
>  	r->objects->commit_graph_attempted = 1;
>  
> +	prepare_repo_settings(r);
> +
>  	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
> -	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
> -	    !config_value))
> +	    r->settings->core_commit_graph != 1)
>  		/*
>  		 * This repository is not configured to use commit graphs, so
>  		 * do not load one. (But report commit_graph_attempted anyway

... how the reading side is done here.  And then instead let the new
repo-settings module read them ...

> diff --git a/repo-settings.c b/repo-settings.c
> new file mode 100644
> index 0000000000..f7fc2a1959
> --- /dev/null
> +++ b/repo-settings.c
> @@ -0,0 +1,44 @@
> +#include "cache.h"
> +#include "repository.h"
> +#include "config.h"
> +#include "repo-settings.h"
> +
> +#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
> +
> +static int git_repo_config(const char *key, const char *value, void *cb)
> +{
> +	struct repo_settings *rs = (struct repo_settings *)cb;
> +
> +	if (!strcmp(key, "core.featureadoptionrate")) {
> +		int rate = git_config_int(key, value);
> +		if (rate >= 3) {
> +			UPDATE_DEFAULT(rs->core_commit_graph, 1);
> +			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
> +		}
> +		return 0;
> +	}
> +	if (!strcmp(key, "core.commitgraph")) {
> +		rs->core_commit_graph = git_config_bool(key, value);
> +		return 0;
> +	}
> +	if (!strcmp(key, "gc.writecommitgraph")) {
> +		rs->gc_write_commit_graph = git_config_bool(key, value);
> +		return 0;
> +	}
> +
> +	return 1;
> +}

... like this.  If a concrete/underlying configuration variable
appears in the configuration data stream, the fields would get their
values overwritten, and if the adoption rate setting appears
_before_ concreate ones, they affect the value in the fields.  So
the assignment to these fields when the adoption rate setting is
seen survives only when the underlying variable does not appear
anywhere in the configuration dasta stream at all.

Which is exactly what we want.  Nicely written.

> +void prepare_repo_settings(struct repository *r)
> +{
> +	if (r->settings)
> +		return;
> +
> +	r->settings = xmalloc(sizeof(*r->settings));
> +
> +	/* Defaults */
> +	r->settings->core_commit_graph = -1;
> +	r->settings->gc_write_commit_graph = -1;
> +
> +	repo_config(r, git_repo_config, r->settings);
> +}
> diff --git a/repo-settings.h b/repo-settings.h
> new file mode 100644
> index 0000000000..11d08648e1
> --- /dev/null
> +++ b/repo-settings.h
> @@ -0,0 +1,13 @@
> +#ifndef REPO_SETTINGS_H
> +#define REPO_SETTINGS_H
> +
> +struct repo_settings {
> +	char core_commit_graph;
> +	char gc_write_commit_graph;
> +};

I do not see a particular reason to favor type "char" here. "char"
is wider than e.g. "signed int :2", if you wanted to save space
compared to a more obvious and naive type, e.g. "int".  More
importantly, the language does not guarantee signedness of "char",
so the sentinel value of "-1" is a bit tricky to use, as you have to
be prepared to see your code used on an "unsigned char" platform.

Use of "signed char" would be OK, but this is a singleton instance
per repository, so I am not sure how much it matters to save a few
words here by not using the most natural "int" type.

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Derrick Stolee]

On 6/28/2019 5:42 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> +struct repo_settings {
>> +	char core_commit_graph;
>> +	char gc_write_commit_graph;
>> +};
> 
> I do not see a particular reason to favor type "char" here. "char"
> is wider than e.g. "signed int :2", if you wanted to save space
> compared to a more obvious and naive type, e.g. "int".  More
> importantly, the language does not guarantee signedness of "char",
> so the sentinel value of "-1" is a bit tricky to use, as you have to
> be prepared to see your code used on an "unsigned char" platform.

I was unaware that platforms could change the signedness of "char".
Thanks for guarding against it.

You're right that it probably isn't worth saving space here, as these
values are replacing existing globals somewhere anyway. If we start
worrying about these being present for each of thousands of submodules
then we probably have bigger problems.

> Use of "signed char" would be OK, but this is a singleton instance
> per repository, so I am not sure how much it matters to save a few
> words here by not using the most natural "int" type.

I'll use 'int' in v2.

Thanks,
-Stolee

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Carlo Arenas]

On Fri, Jun 28, 2019 at 6:44 PM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 6/28/2019 5:42 PM, Junio C Hamano wrote:
> > "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > Use of "signed char" would be OK, but this is a singleton instance
> > per repository, so I am not sure how much it matters to save a few
> > words here by not using the most natural "int" type.
>
> I'll use 'int' in v2.

FWIW, this broke the build in (at least) Linux AArch64, unless the following
is applied on top of ds/early-access

Carlo
-- >8 --
Subject: [PATCH] repo-settings: explicitly make flags using char as signed

used as a three state boolean and therefore could also be set/compared
with -1, which will break in architectures that use unsigned char by
default (ex: ARM)

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
---
 repo-settings.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/repo-settings.h b/repo-settings.h
index b50228f992..544873fff4 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -2,9 +2,9 @@
 #define REPO_SETTINGS_H

 struct repo_settings {
-       char core_commit_graph;
-       char gc_write_commit_graph;
-       char pack_use_sparse;
+       signed char core_commit_graph;
+       signed char gc_write_commit_graph;
+       signed char pack_use_sparse;
        int index_version;
 };

-- 
2.22.0

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Derrick Stolee]

On 6/30/2019 2:35 PM, Carlo Arenas wrote:
> On Fri, Jun 28, 2019 at 6:44 PM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> On 6/28/2019 5:42 PM, Junio C Hamano wrote:
>>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>>
>>> Use of "signed char" would be OK, but this is a singleton instance
>>> per repository, so I am not sure how much it matters to save a few
>>> words here by not using the most natural "int" type.
>>
>> I'll use 'int' in v2.
> 
> FWIW, this broke the build in (at least) Linux AArch64, unless the following
> is applied on top of ds/early-access

Thanks, Carlo, for reporting this. I'm glad we have someone running early
builds on platforms with these special cases!

I'm working on rerolling to use int.

-Stolee

[PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Derrick Stolee via GitGitGadget]

Here is a second run at this RFC, which aims to create a "meta" config
setting that automatically turns on other settings according to a user's
willingness to trade new Git behavior or new feature risk for performance
benefits. The new name for the setting is "core.featureAdoptionRate" and is
an integer scale from 0 to 10. There will be multiple "categories" of
settings, and the intention is to allow more granular levels as necessary.

The first category is "3 or higher" which means that the user is willing to
adopt features that have been tested in multiple major releases. The
settings to include here are core.commitGraph=true,
gc.writeCommitGraph=true, and index.version=4.

The second category is "5 or higher" which means the user is willing to
adopt features that have not been out for multiple major releases. The
setting included here is pack.useSparse=true.

In the future, I would add a "7 or higher" setting which means the user is
willing to have a change of behavior in exchange for performance benefits.
The two settings to place here are 'status.aheadBehind=false' and
'fetch.showForcedUpdates=false'. Instead of including these settings in the
current series, I've submitted them independently for full review [1, 2].

Hopefully this direction is amenable to allow "early adopters" gain access
to new performance features even if they are not necessary reading every
line of the release notes.

Thanks, -Stolee

[1] https://public-inbox.org/git/pull.272.git.gitgitgadget@gmail.com/

[2] https://public-inbox.org/git/pull.273.git.gitgitgadget@gmail.com/

Derrick Stolee (3):
  repo-settings: create core.featureAdoptionRate setting
  repo-settings: use index.version=4 by default
  repo-settings: pack.useSparse=true

 Documentation/config/core.txt  | 34 +++++++++++++++++++-
 Documentation/config/gc.txt    |  4 +--
 Documentation/config/index.txt |  2 ++
 Documentation/config/pack.txt  |  3 +-
 Makefile                       |  1 +
 builtin/gc.c                   |  6 ++--
 builtin/pack-objects.c         |  9 +++---
 commit-graph.c                 |  7 ++--
 read-cache.c                   | 12 ++++---
 repo-settings.c                | 58 ++++++++++++++++++++++++++++++++++
 repo-settings.h                | 15 +++++++++
 repository.h                   |  3 ++
 t/t1600-index.sh               | 34 +++++++++++++++++---
 13 files changed, 164 insertions(+), 24 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h


base-commit: aa25c82427ae70aebf3b8f970f2afd54e9a2a8c6
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-254%2Fderrickstolee%2Fconfig-large%2Fupstream-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-254/derrickstolee/config-large/upstream-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/254

Range-diff vs v2:

 1:  bdaee3ea9d ! 1:  13b9e71b38 repo-settings: create core.featureAdoptionRate setting
     @@ -71,8 +71,9 @@
      ++
      +If the value is at least 3, then the following defaults are modified.
      +These represent relatively new features that have existed for multiple
     -+major releases, and present significant performance benefits. They do
     -+not modify the user-facing output of porcelain commands.
     ++major releases, and may present performance benefits. These benefits
     ++depend on the amount and kind of data in your repo and how you use it.
     ++The settings do not modify the user-facing output of porcelain commands.
      ++
      +* `core.commitGraph=true` enables reading commit-graph files.
      ++
     @@ -236,8 +237,8 @@
      +#define REPO_SETTINGS_H
      +
      +struct repo_settings {
     -+	char core_commit_graph;
     -+	char gc_write_commit_graph;
     ++	int core_commit_graph;
     ++	int gc_write_commit_graph;
      +};
      +
      +struct repository;
 2:  02c89415fe ! 2:  4fe896e423 repo-settings: use index.version=4 by default
     @@ -6,6 +6,12 @@
          This means the index could be compressed using version 4. Set this as
          a default when core.featureAdoptionRate is at least three.
      
     +    Since the index version is written to a file, this is an excellent
     +    opportunity to test that the config settings are working correctly
     +    with the different precedence rules. Adapt a test from t1600-index.sh
     +    to verify the version is set properly with different values of
     +    index.version config, core.featureAdoptionRate, and GIT_INDEX_VERSION.
     +
          Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
      
       diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
     @@ -108,9 +114,60 @@
       +++ b/repo-settings.h
      @@
       struct repo_settings {
     - 	char core_commit_graph;
     - 	char gc_write_commit_graph;
     + 	int core_commit_graph;
     + 	int gc_write_commit_graph;
      +	int index_version;
       };
       
       struct repository;
     +
     + diff --git a/t/t1600-index.sh b/t/t1600-index.sh
     + --- a/t/t1600-index.sh
     + +++ b/t/t1600-index.sh
     +@@
     + 	)
     + '
     + 
     +-test_expect_success 'GIT_INDEX_VERSION takes precedence over config' '
     ++test_index_version () {
     ++	INDEX_VERSION_CONFIG=$1 &&
     ++	REPO_ADOPTION_RATE=$2 &&
     ++	ENV_VAR_VERSION=$3
     ++	EXPECTED_OUTPUT_VERSION=$4 &&
     + 	(
     + 		rm -f .git/index &&
     +-		GIT_INDEX_VERSION=4 &&
     +-		export GIT_INDEX_VERSION &&
     +-		git config --add index.version 2 &&
     ++		rm -f .git/config &&
     ++		if test "$INDEX_VERSION_CONFIG" -ne 0
     ++		then
     ++			git config --add index.version $INDEX_VERSION_CONFIG
     ++		fi &&
     ++		if test "$REPO_ADOPTION_RATE" -ne 0
     ++		then
     ++			git config --add core.featureAdoptionRate $REPO_ADOPTION_RATE
     ++		fi &&
     ++		if test "$ENV_VAR_VERSION" -ne 0
     ++		then
     ++			GIT_INDEX_VERSION=$ENV_VAR_VERSION &&
     ++			export GIT_INDEX_VERSION
     ++		else
     ++			unset GIT_INDEX_VERSION
     ++		fi &&
     + 		git add a 2>&1 &&
     +-		echo 4 >expect &&
     ++		echo $EXPECTED_OUTPUT_VERSION >expect &&
     + 		test-tool index-version <.git/index >actual &&
     + 		test_cmp expect actual
     + 	)
     ++}
     ++
     ++test_expect_success 'index version config precedence' '
     ++	test_index_version 2 0 4 4 &&
     ++	test_index_version 2 3 0 2 &&
     ++	test_index_version 0 3 0 4 &&
     ++	test_index_version 0 3 2 2
     + '
     + 
     + test_done
 3:  5bba9062f4 ! 3:  d080065a92 repo-settings: pack.useSparse=true
     @@ -117,9 +117,9 @@
       +++ b/repo-settings.h
      @@
       struct repo_settings {
     - 	char core_commit_graph;
     - 	char gc_write_commit_graph;
     -+	char pack_use_sparse;
     + 	int core_commit_graph;
     + 	int gc_write_commit_graph;
     ++	int pack_use_sparse;
       	int index_version;
       };
       

-- 
gitgitgadget

[PATCH v3 1/3] repo-settings: create core.featureAdoptionRate setting

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

Several advanced config settings are highly recommended for clients
using large repositories. Power users learn these one-by-one and
enable them as they see fit. This could be made simpler, to allow
more users to have access to these almost-always beneficial features
(and more beneficial in larger repos).

Create a 'core.featureAdoptionRate' config setting that allows integer
values. This is a rating from 0 to 10 for the user's willingness to
adopt new or experimental features that improve Git performance.
The default is 0, meaning "don't change anything!" A value of 10
would mean "I'm willing for some behavior to change to get the
best performance I can get, and can take experimental features
in their first release." As we integrate this with more config
settings, we will make this scale more clear.

This config setting only changes the default values of other config
settings. If the setting is given explicitly, then take the
explicit value.

This change adds these two defaults when core.featureAdoptionRate
is at least three:

 * core.commitGraph=true
 * gc.writeCommitGraph=true

The use of "three or higher" for these settings means that a value
of 3 means "I'm willing to add optional features that can augment
the data on disk in favor of improved performance, but those
features should be stable after being included in multiple major
releases."

To centralize these config options and properly set the defaults,
create a repo_settings that contains chars for each config variable.
Use -1 as "unset", with 0 for false and 1 for true.

The prepare_repo_settings() method ensures that this settings
struct has been initialized, and avoids double-scanning the config
settings.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt | 22 +++++++++++++++++-
 Documentation/config/gc.txt   |  4 ++--
 Makefile                      |  1 +
 builtin/gc.c                  |  6 ++---
 commit-graph.c                |  7 +++---
 repo-settings.c               | 44 +++++++++++++++++++++++++++++++++++
 repo-settings.h               | 13 +++++++++++
 repository.h                  |  3 +++
 8 files changed, 91 insertions(+), 9 deletions(-)
 create mode 100644 repo-settings.c
 create mode 100644 repo-settings.h

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 75538d27e7..bfe647c76f 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -577,7 +577,8 @@ the `GIT_NOTES_REF` environment variable.  See linkgit:git-notes[1].
 
 core.commitGraph::
 	If true, then git will read the commit-graph file (if it exists)
-	to parse the graph structure of commits. Defaults to false. See
+	to parse the graph structure of commits. Defaults to false, unless
+	`core.featureAdoptionRate` is at least three. See
 	linkgit:git-commit-graph[1] for more information.
 
 core.useReplaceRefs::
@@ -601,3 +602,22 @@ core.abbrev::
 	in your repository, which hopefully is enough for
 	abbreviated object names to stay unique for some time.
 	The minimum length is 4.
+
+core.featureAdoptionRate::
+	Set an integer value on a scale from 0 to 10 describing your
+	desire to adopt new performance features. Defaults to 0. As
+	the value increases, features are enabled by changing the
+	default values of other config settings. If a config variable
+	is specified explicitly, the explicit value will override these
+	defaults:
++
+If the value is at least 3, then the following defaults are modified.
+These represent relatively new features that have existed for multiple
+major releases, and may present performance benefits. These benefits
+depend on the amount and kind of data in your repo and how you use it.
+The settings do not modify the user-facing output of porcelain commands.
++
+* `core.commitGraph=true` enables reading commit-graph files.
++
+* `gc.writeCommitGraph=true` eneables writing commit-graph files during
+`git gc`.
diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
index 02b92b18b5..898263209c 100644
--- a/Documentation/config/gc.txt
+++ b/Documentation/config/gc.txt
@@ -63,8 +63,8 @@ gc.writeCommitGraph::
 	If true, then gc will rewrite the commit-graph file when
 	linkgit:git-gc[1] is run. When using `git gc --auto`
 	the commit-graph will be updated if housekeeping is
-	required. Default is false. See linkgit:git-commit-graph[1]
-	for details.
+	required. Default is false, unless `core.featureAdoptionRage`
+	is at least three. See linkgit:git-commit-graph[1] for details.
 
 gc.logExpiry::
 	If the file gc.log exists, then `git gc --auto` will print
diff --git a/Makefile b/Makefile
index 8a7e235352..2d3499d7ac 100644
--- a/Makefile
+++ b/Makefile
@@ -967,6 +967,7 @@ LIB_OBJS += refspec.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace-object.o
+LIB_OBJS += repo-settings.o
 LIB_OBJS += repository.o
 LIB_OBJS += rerere.o
 LIB_OBJS += resolve-undo.o
diff --git a/builtin/gc.c b/builtin/gc.c
index 8943bcc300..6281aad961 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -27,6 +27,7 @@
 #include "pack-objects.h"
 #include "blob.h"
 #include "tree.h"
+#include "repo-settings.h"
 
 #define FAILED_RUN "failed to run %s"
 
@@ -41,7 +42,6 @@ static int aggressive_depth = 50;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
-static int gc_write_commit_graph;
 static int detach_auto = 1;
 static timestamp_t gc_log_expire_time;
 static const char *gc_log_expire = "1.day.ago";
@@ -148,7 +148,6 @@ static void gc_config(void)
 	git_config_get_int("gc.aggressivedepth", &aggressive_depth);
 	git_config_get_int("gc.auto", &gc_auto_threshold);
 	git_config_get_int("gc.autopacklimit", &gc_auto_pack_limit);
-	git_config_get_bool("gc.writecommitgraph", &gc_write_commit_graph);
 	git_config_get_bool("gc.autodetach", &detach_auto);
 	git_config_get_expiry("gc.pruneexpire", &prune_expire);
 	git_config_get_expiry("gc.worktreepruneexpire", &prune_worktrees_expire);
@@ -685,7 +684,8 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		clean_pack_garbage();
 	}
 
-	if (gc_write_commit_graph)
+	prepare_repo_settings(the_repository);
+	if (the_repository->settings->gc_write_commit_graph == 1)
 		write_commit_graph_reachable(get_object_directory(), 0,
 					     !quiet && !daemonized);
 
diff --git a/commit-graph.c b/commit-graph.c
index 7c5e54875f..b09c465a7a 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -16,6 +16,7 @@
 #include "hashmap.h"
 #include "replace-object.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 #define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */
 #define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */
@@ -311,7 +312,6 @@ static void prepare_commit_graph_one(struct repository *r, const char *obj_dir)
 static int prepare_commit_graph(struct repository *r)
 {
 	struct object_directory *odb;
-	int config_value;
 
 	if (git_env_bool(GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD, 0))
 		die("dying as requested by the '%s' variable on commit-graph load!",
@@ -321,9 +321,10 @@ static int prepare_commit_graph(struct repository *r)
 		return !!r->objects->commit_graph;
 	r->objects->commit_graph_attempted = 1;
 
+	prepare_repo_settings(r);
+
 	if (!git_env_bool(GIT_TEST_COMMIT_GRAPH, 0) &&
-	    (repo_config_get_bool(r, "core.commitgraph", &config_value) ||
-	    !config_value))
+	    r->settings->core_commit_graph != 1)
 		/*
 		 * This repository is not configured to use commit graphs, so
 		 * do not load one. (But report commit_graph_attempted anyway
diff --git a/repo-settings.c b/repo-settings.c
new file mode 100644
index 0000000000..f7fc2a1959
--- /dev/null
+++ b/repo-settings.c
@@ -0,0 +1,44 @@
+#include "cache.h"
+#include "repository.h"
+#include "config.h"
+#include "repo-settings.h"
+
+#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
+
+static int git_repo_config(const char *key, const char *value, void *cb)
+{
+	struct repo_settings *rs = (struct repo_settings *)cb;
+
+	if (!strcmp(key, "core.featureadoptionrate")) {
+		int rate = git_config_int(key, value);
+		if (rate >= 3) {
+			UPDATE_DEFAULT(rs->core_commit_graph, 1);
+			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+		}
+		return 0;
+	}
+	if (!strcmp(key, "core.commitgraph")) {
+		rs->core_commit_graph = git_config_bool(key, value);
+		return 0;
+	}
+	if (!strcmp(key, "gc.writecommitgraph")) {
+		rs->gc_write_commit_graph = git_config_bool(key, value);
+		return 0;
+	}
+
+	return 1;
+}
+
+void prepare_repo_settings(struct repository *r)
+{
+	if (r->settings)
+		return;
+
+	r->settings = xmalloc(sizeof(*r->settings));
+
+	/* Defaults */
+	r->settings->core_commit_graph = -1;
+	r->settings->gc_write_commit_graph = -1;
+
+	repo_config(r, git_repo_config, r->settings);
+}
diff --git a/repo-settings.h b/repo-settings.h
new file mode 100644
index 0000000000..7d44627bf0
--- /dev/null
+++ b/repo-settings.h
@@ -0,0 +1,13 @@
+#ifndef REPO_SETTINGS_H
+#define REPO_SETTINGS_H
+
+struct repo_settings {
+	int core_commit_graph;
+	int gc_write_commit_graph;
+};
+
+struct repository;
+
+void prepare_repo_settings(struct repository *r);
+
+#endif /* REPO_SETTINGS_H */
diff --git a/repository.h b/repository.h
index 4fb6a5885f..352afc9cd8 100644
--- a/repository.h
+++ b/repository.h
@@ -4,6 +4,7 @@
 #include "path.h"
 
 struct config_set;
+struct repo_settings;
 struct git_hash_algo;
 struct index_state;
 struct lock_file;
@@ -72,6 +73,8 @@ struct repository {
 	 */
 	char *submodule_prefix;
 
+	struct repo_settings *settings;
+
 	/* Subsystems */
 	/*
 	 * Repository's config which contains key-value pairs from the usual
-- 
gitgitgadget

[PATCH v3 2/3] repo-settings: use index.version=4 by default

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

If a repo is large, it likely has many paths in its working directory.
This means the index could be compressed using version 4. Set this as
a default when core.featureAdoptionRate is at least three.

Since the index version is written to a file, this is an excellent
opportunity to test that the config settings are working correctly
with the different precedence rules. Adapt a test from t1600-index.sh
to verify the version is set properly with different values of
index.version config, core.featureAdoptionRate, and GIT_INDEX_VERSION.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt  |  3 +++
 Documentation/config/index.txt |  2 ++
 read-cache.c                   | 12 +++++++-----
 repo-settings.c                |  6 ++++++
 repo-settings.h                |  1 +
 t/t1600-index.sh               | 34 +++++++++++++++++++++++++++++-----
 6 files changed, 48 insertions(+), 10 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index bfe647c76f..865252aba9 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -621,3 +621,6 @@ The settings do not modify the user-facing output of porcelain commands.
 +
 * `gc.writeCommitGraph=true` eneables writing commit-graph files during
 `git gc`.
++
+* `index.version=4` uses prefix-compression to reduce the size of the
+.git/index file.
diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt
index f181503041..98a88c30be 100644
--- a/Documentation/config/index.txt
+++ b/Documentation/config/index.txt
@@ -24,3 +24,5 @@ index.threads::
 index.version::
 	Specify the version with which new index files should be
 	initialized.  This does not affect existing repositories.
+	If `core.featureAdoptionRate` is at least three, then the
+	default value is 4.
diff --git a/read-cache.c b/read-cache.c
index 22e7b9944e..7fab8ff748 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -25,6 +25,7 @@
 #include "fsmonitor.h"
 #include "thread-utils.h"
 #include "progress.h"
+#include "repo-settings.h"
 
 /* Mask for the name length in ce_flags in the on-disk index */
 
@@ -1599,16 +1600,17 @@ struct cache_entry *refresh_cache_entry(struct index_state *istate,
 
 #define INDEX_FORMAT_DEFAULT 3
 
-static unsigned int get_index_format_default(void)
+static unsigned int get_index_format_default(struct repository *r)
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
 	char *endp;
-	int value;
 	unsigned int version = INDEX_FORMAT_DEFAULT;
 
 	if (!envversion) {
-		if (!git_config_get_int("index.version", &value))
-			version = value;
+		prepare_repo_settings(r);
+
+		if (r->settings->index_version >= 0)
+			version = r->settings->index_version;
 		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
 			warning(_("index.version set, but the value is invalid.\n"
 				  "Using version %i"), INDEX_FORMAT_DEFAULT);
@@ -2765,7 +2767,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
 	}
 
 	if (!istate->version) {
-		istate->version = get_index_format_default();
+		istate->version = get_index_format_default(the_repository);
 		if (git_env_bool("GIT_TEST_SPLIT_INDEX", 0))
 			init_split_index(istate);
 	}
diff --git a/repo-settings.c b/repo-settings.c
index f7fc2a1959..5753153a84 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -14,6 +14,7 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		if (rate >= 3) {
 			UPDATE_DEFAULT(rs->core_commit_graph, 1);
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
+			UPDATE_DEFAULT(rs->index_version, 4);
 		}
 		return 0;
 	}
@@ -25,6 +26,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->gc_write_commit_graph = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "index.version")) {
+		rs->index_version = git_config_int(key, value);
+		return 0;
+	}
 
 	return 1;
 }
@@ -39,6 +44,7 @@ void prepare_repo_settings(struct repository *r)
 	/* Defaults */
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
+	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
 }
diff --git a/repo-settings.h b/repo-settings.h
index 7d44627bf0..b752dfe8b4 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -4,6 +4,7 @@
 struct repo_settings {
 	int core_commit_graph;
 	int gc_write_commit_graph;
+	int index_version;
 };
 
 struct repository;
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 42962ed7d4..74f56e2769 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -59,17 +59,41 @@ test_expect_success 'out of bounds index.version issues warning' '
 	)
 '
 
-test_expect_success 'GIT_INDEX_VERSION takes precedence over config' '
+test_index_version () {
+	INDEX_VERSION_CONFIG=$1 &&
+	REPO_ADOPTION_RATE=$2 &&
+	ENV_VAR_VERSION=$3
+	EXPECTED_OUTPUT_VERSION=$4 &&
 	(
 		rm -f .git/index &&
-		GIT_INDEX_VERSION=4 &&
-		export GIT_INDEX_VERSION &&
-		git config --add index.version 2 &&
+		rm -f .git/config &&
+		if test "$INDEX_VERSION_CONFIG" -ne 0
+		then
+			git config --add index.version $INDEX_VERSION_CONFIG
+		fi &&
+		if test "$REPO_ADOPTION_RATE" -ne 0
+		then
+			git config --add core.featureAdoptionRate $REPO_ADOPTION_RATE
+		fi &&
+		if test "$ENV_VAR_VERSION" -ne 0
+		then
+			GIT_INDEX_VERSION=$ENV_VAR_VERSION &&
+			export GIT_INDEX_VERSION
+		else
+			unset GIT_INDEX_VERSION
+		fi &&
 		git add a 2>&1 &&
-		echo 4 >expect &&
+		echo $EXPECTED_OUTPUT_VERSION >expect &&
 		test-tool index-version <.git/index >actual &&
 		test_cmp expect actual
 	)
+}
+
+test_expect_success 'index version config precedence' '
+	test_index_version 2 0 4 4 &&
+	test_index_version 2 3 0 2 &&
+	test_index_version 0 3 0 4 &&
+	test_index_version 0 3 2 2
 '
 
 test_done
-- 
gitgitgadget

[PATCH v3 3/3] repo-settings: pack.useSparse=true

[Derrick Stolee via GitGitGadget]

From: Derrick Stolee <dstolee@microsoft.com>

If a repo is large, then it probably has a very large working
directory. In this case, a typical developer's edits usually impact
many fewer paths than the full path set. The sparse treewalk
algorithm is optimized for this case, speeding up 'git push' calls.

Use pack.useSparse=true when core.featureAdoptionRate is at least
five. This is the first setting where the feature has only been
out for a single major version. This could be moved to the "at
least three" category after another major version.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
---
 Documentation/config/core.txt | 9 +++++++++
 Documentation/config/pack.txt | 3 ++-
 builtin/pack-objects.c        | 9 +++++----
 repo-settings.c               | 8 ++++++++
 repo-settings.h               | 1 +
 5 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt
index 865252aba9..60356102a8 100644
--- a/Documentation/config/core.txt
+++ b/Documentation/config/core.txt
@@ -624,3 +624,12 @@ The settings do not modify the user-facing output of porcelain commands.
 +
 * `index.version=4` uses prefix-compression to reduce the size of the
 .git/index file.
++
+If the value is at least 5, then all of the defaults above are included,
+plus the defaults below. These represent new features that present
+significant performance benefits, but may not have been released for
+multiple major versions.
++
+* `pack.useSparse=true` uses the sparse tree-walk algorithm, which is
+optimized for enumerating objects during linkgit:git-push[1] from a
+client machine.
diff --git a/Documentation/config/pack.txt b/Documentation/config/pack.txt
index 9cdcfa7324..9c4f8ea9ff 100644
--- a/Documentation/config/pack.txt
+++ b/Documentation/config/pack.txt
@@ -112,7 +112,8 @@ pack.useSparse::
 	objects. This can have significant performance benefits when
 	computing a pack to send a small change. However, it is possible
 	that extra objects are added to the pack-file if the included
-	commits contain certain types of direct renames.
+	commits contain certain types of direct renames. Defaults to
+	false, unless `core.featureAdoptionRate` is at least five.
 
 pack.writeBitmaps (deprecated)::
 	This is a deprecated synonym for `repack.writeBitmaps`.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 41d7fc5983..f26b3f2892 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -34,6 +34,7 @@
 #include "dir.h"
 #include "midx.h"
 #include "trace2.h"
+#include "repo-settings.h"
 
 #define IN_PACK(obj) oe_in_pack(&to_pack, obj)
 #define SIZE(obj) oe_size(&to_pack, obj)
@@ -2707,10 +2708,6 @@ 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.usesparse")) {
-		sparse = 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)
@@ -3330,6 +3327,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	read_replace_refs = 0;
 
 	sparse = git_env_bool("GIT_TEST_PACK_SPARSE", 0);
+	prepare_repo_settings(the_repository);
+	if (!sparse && the_repository->settings->pack_use_sparse != -1)
+		sparse = the_repository->settings->pack_use_sparse;
+
 	reset_pack_idx_option(&pack_idx_opts);
 	git_config(git_pack_config, NULL);
 
diff --git a/repo-settings.c b/repo-settings.c
index 5753153a84..c700edc286 100644
--- a/repo-settings.c
+++ b/repo-settings.c
@@ -16,6 +16,9 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 			UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
 			UPDATE_DEFAULT(rs->index_version, 4);
 		}
+		if (rate >= 5) {
+			UPDATE_DEFAULT(rs->pack_use_sparse, 1);
+		}
 		return 0;
 	}
 	if (!strcmp(key, "core.commitgraph")) {
@@ -26,6 +29,10 @@ static int git_repo_config(const char *key, const char *value, void *cb)
 		rs->gc_write_commit_graph = git_config_bool(key, value);
 		return 0;
 	}
+	if (!strcmp(key, "pack.usesparse")) {
+		rs->pack_use_sparse = git_config_bool(key, value);
+		return 0;
+	}
 	if (!strcmp(key, "index.version")) {
 		rs->index_version = git_config_int(key, value);
 		return 0;
@@ -44,6 +51,7 @@ void prepare_repo_settings(struct repository *r)
 	/* Defaults */
 	r->settings->core_commit_graph = -1;
 	r->settings->gc_write_commit_graph = -1;
+	r->settings->pack_use_sparse = -1;
 	r->settings->index_version = -1;
 
 	repo_config(r, git_repo_config, r->settings);
diff --git a/repo-settings.h b/repo-settings.h
index b752dfe8b4..1151c2193a 100644
--- a/repo-settings.h
+++ b/repo-settings.h
@@ -4,6 +4,7 @@
 struct repo_settings {
 	int core_commit_graph;
 	int gc_write_commit_graph;
+	int pack_use_sparse;
 	int index_version;
 };
 
-- 
gitgitgadget

Re: [PATCH v3 1/3] repo-settings: create core.featureAdoptionRate setting

[Carlo Arenas]

On Mon, Jul 1, 2019 at 8:32 AM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> To centralize these config options and properly set the defaults,
> create a repo_settings that contains chars for each config variable.
> Use -1 as "unset", with 0 for false and 1 for true.

minor nitpick that hopefully Junio can fix: s/chars/ints

> +* `gc.writeCommitGraph=true` eneables writing commit-graph files during

typo: s/eneables/enable

Carlo

Re: [PATCH v3 1/3] repo-settings: create core.featureAdoptionRate setting

[Duy Nguyen]

On Mon, Jul 1, 2019 at 10:32 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> @@ -601,3 +602,22 @@ core.abbrev::
>         in your repository, which hopefully is enough for
>         abbreviated object names to stay unique for some time.
>         The minimum length is 4.
> +
> +core.featureAdoptionRate::
> +       Set an integer value on a scale from 0 to 10 describing your
> +       desire to adopt new performance features. Defaults to 0. As
> +       the value increases, features are enabled by changing the
> +       default values of other config settings. If a config variable
> +       is specified explicitly, the explicit value will override these
> +       defaults:

This is because I'd like to keep core.* from growing too big (it's
already big), hard to read, search and maintain. Perhaps this should
belong to a separate group? Something like tuning.something or
defaults.something.

> +If the value is at least 3, then the following defaults are modified.
> +These represent relatively new features that have existed for multiple
> +major releases, and may present performance benefits. These benefits
> +depend on the amount and kind of data in your repo and how you use it.

Then instead of numeric values, maybe the user should write some sort
description about the repo and we optimize for that, similar to gcc
-Os optimized for size, -Ofast for compiler speed (-O<n> is all about
execution speed).

We could write, for example, tuning.commitHistory = {small, medium,
large} and tuning.worktree = {small, large, medium} and maybe
tuning.refSize and use that to optimize. We can still have different
optimization levels (probably just "none", "recommended" vs
"aggressive" where agressive enables most new stuff),
-- 
Duy

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Ævar Arnfjörð Bjarmason]

On Wed, Jun 19 2019, Derrick Stolee via GitGitGadget wrote:

>  core.commitGraph::
>  	If true, then git will read the commit-graph file (if it exists)
> -	to parse the graph structure of commits. Defaults to false. See
> +	to parse the graph structure of commits. Defaults to false, unless
> +	`core.featureAdoptionRate` is at least three. See
>  	linkgit:git-commit-graph[1] for more information.
>
>  core.useReplaceRefs::
> @@ -601,3 +602,21 @@ core.abbrev::
>  	in your repository, which hopefully is enough for
>  	abbreviated object names to stay unique for some time.
>  	The minimum length is 4.
> +
> +core.featureAdoptionRate::
> +	Set an integer value on a scale from 0 to 10 describing your
> +	desire to adopt new performance features. Defaults to 0. As
> +	the value increases, features are enabled by changing the
> +	default values of other config settings. If a config variable
> +	is specified explicitly, the explicit value will override these
> +	defaults:
> ++
> +If the value is at least 3, then the following defaults are modified.
> +These represent relatively new features that have existed for multiple
> +major releases, and present significant performance benefits. They do
> +not modify the user-facing output of porcelain commands.
> ++
> +* `core.commitGraph=true` enables reading commit-graph files.
> ++
> +* `gc.writeCommitGraph=true` eneables writing commit-graph files during

I barked up a similar tree in
https://public-inbox.org/git/CACBZZX5SbYo5fVPtK6LW1FF96nR5591RHHC-5wdjW-fmg1R0EQ@mail.gmail.com/

I wonder if you've seen that & what you think about that
approach. I.e. have a core.version=2.28 (or core.version=+6) or whatever
to opt-in to features we'd make default in 2.28. Would that be your
core.featureAdoptionRate=6 (28-28 = 6)?

I admit that question is partly rhetorical, because I think it suggests
how hard it would be for users to reason about this.

The "core.version" idea also sucks, but at least it's bound to our
advertised version number, so it's obvious if you set it to e.g. +2 what
feature track you're on, and furthermore when we'd commit to making that
the default for users who don't set core.version (although we could of
course always change our minds...). It's also something that mirrors how
e.g. Perl, C compilers (with --std=*) treat this sort of thing.

So I'm all for a facility to have a setting to collectively opt-in to
new things early. But I think for such a thing we really should a) at
least in principle commit to making those things the default eventually
(if they don't suck) b) it needs to be obvious to the user how the
"rate" relates to git releases.

This "core.featureAdoptionRate" value seems more like zlib compression
values & unrelated to release numbers. It's also for "performance
features" only but squats a more general name. I suggested
"core.version" & then "core.uiVersion" (in
https://public-inbox.org/git/87pnunxz5i.fsf@evledraar.gmail.com/).

Regardless of whether we want to pin opt-in early-bird features to
version numbers in some way, which I think is a good idea, but maybe
others disagree. I think if it's "just performance" it's good to put
that in the key name in such a way that we can have "early UI" features,
or other non-UI non-performance.

Thanks for working on this!

Re: [PATCH v3 1/3] repo-settings: create core.featureAdoptionRate setting

[Ævar Arnfjörð Bjarmason]

On Tue, Jul 02 2019, Duy Nguyen wrote:

> On Mon, Jul 1, 2019 at 10:32 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> @@ -601,3 +602,22 @@ core.abbrev::
>>         in your repository, which hopefully is enough for
>>         abbreviated object names to stay unique for some time.
>>         The minimum length is 4.
>> +
>> +core.featureAdoptionRate::
>> +       Set an integer value on a scale from 0 to 10 describing your
>> +       desire to adopt new performance features. Defaults to 0. As
>> +       the value increases, features are enabled by changing the
>> +       default values of other config settings. If a config variable
>> +       is specified explicitly, the explicit value will override these
>> +       defaults:
>
> This is because I'd like to keep core.* from growing too big (it's
> already big), hard to read, search and maintain. Perhaps this should
> belong to a separate group? Something like tuning.something or
> defaults.something.

The main thing users look at is "man git-config" (or its web rendering)
which renders it all in one page anyway.

I think in general adding more things to core.* sucks less than
explaining the special-case that "tuning.*" isn't a config for
git-tuning(1) (although we have some of that already, e.g. with
trace2.*).

Documentation/config/core.txt is ~600 lines. Maybe it would be a good
idea to split it up, similar to your split of
Documentation/config/*.txt, but let's not conflate how we'd like to
maintain stuff in git.git with a config interface we expose externally.

It's going to be very confusing for users if some settings that
otherwise would be in core aren't there because a file in git.git was
"too big" at the time. Users (mostly) aren't going to know/care in what
chronological order we added config keys.

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Duy Nguyen]

On Tue, Jul 2, 2019 at 5:47 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
>
> On Wed, Jun 19 2019, Derrick Stolee via GitGitGadget wrote:
>
> >  core.commitGraph::
> >       If true, then git will read the commit-graph file (if it exists)
> > -     to parse the graph structure of commits. Defaults to false. See
> > +     to parse the graph structure of commits. Defaults to false, unless
> > +     `core.featureAdoptionRate` is at least three. See
> >       linkgit:git-commit-graph[1] for more information.
> >
> >  core.useReplaceRefs::
> > @@ -601,3 +602,21 @@ core.abbrev::
> >       in your repository, which hopefully is enough for
> >       abbreviated object names to stay unique for some time.
> >       The minimum length is 4.
> > +
> > +core.featureAdoptionRate::
> > +     Set an integer value on a scale from 0 to 10 describing your
> > +     desire to adopt new performance features. Defaults to 0. As
> > +     the value increases, features are enabled by changing the
> > +     default values of other config settings. If a config variable
> > +     is specified explicitly, the explicit value will override these
> > +     defaults:
> > ++
> > +If the value is at least 3, then the following defaults are modified.
> > +These represent relatively new features that have existed for multiple
> > +major releases, and present significant performance benefits. They do
> > +not modify the user-facing output of porcelain commands.
> > ++
> > +* `core.commitGraph=true` enables reading commit-graph files.
> > ++
> > +* `gc.writeCommitGraph=true` eneables writing commit-graph files during
>
> I barked up a similar tree in
> https://public-inbox.org/git/CACBZZX5SbYo5fVPtK6LW1FF96nR5591RHHC-5wdjW-fmg1R0EQ@mail.gmail.com/
>
> I wonder if you've seen that & what you think about that
> approach. I.e. have a core.version=2.28 (or core.version=+6) or whatever
> to opt-in to features we'd make default in 2.28. Would that be your
> core.featureAdoptionRate=6 (28-28 = 6)?
>
> I admit that question is partly rhetorical, because I think it suggests
> how hard it would be for users to reason about this.
>
> The "core.version" idea also sucks, but at least it's bound to our
> advertised version number, so it's obvious if you set it to e.g. +2 what
> feature track you're on, and furthermore when we'd commit to making that
> the default for users who don't set core.version (although we could of
> course always change our minds...). It's also something that mirrors how
> e.g. Perl, C compilers (with --std=*) treat this sort of thing.
>
> So I'm all for a facility to have a setting to collectively opt-in to
> new things early. But I think for such a thing we really should a) at
> least in principle commit to making those things the default eventually

Some features may be best enabled for certain setups. This is why I
set configuration variables repo size, worktree size.. instead of just
one number.

> (if they don't suck) b) it needs to be obvious to the user how the
> "rate" relates to git releases.

I see this more like gcc =O options. And for those options, the
developers decide what to include. If you know what you want already,
you can just turn specific keys on. Otherwise you count on devs to do
the right things.

It would help if we have something like "gcc -Q -O2 --help=optimizers"
so you can see exactly what you need to turn on to achieve the same
thing. Then you can just set those have the same "per release"
settings.

Which makes me think about a slightly different implementation detail
(which I ignored because I didn't think further about per-release
stuff): since these are basically meta config to change defaults, we
can just implement them as a (builtin, or bundled) config file. The
user can see what are included much easier we have several different
config "profiles" (deep history, large worktree, bleeding-edge...) and
the user can include one or all [1].

[1] it also opens up the opportunity to have a standard (but optional)
set of aliases. But that's a touchy topic.
-- 
Duy

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Derrick Stolee]

On 7/2/2019 7:09 AM, Duy Nguyen wrote:
> On Tue, Jul 2, 2019 at 5:47 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>>
>>
>> On Wed, Jun 19 2019, Derrick Stolee via GitGitGadget wrote:
>>
>>>  core.commitGraph::
>>>       If true, then git will read the commit-graph file (if it exists)
>>> -     to parse the graph structure of commits. Defaults to false. See
>>> +     to parse the graph structure of commits. Defaults to false, unless
>>> +     `core.featureAdoptionRate` is at least three. See
>>>       linkgit:git-commit-graph[1] for more information.
>>>
>>>  core.useReplaceRefs::
>>> @@ -601,3 +602,21 @@ core.abbrev::
>>>       in your repository, which hopefully is enough for
>>>       abbreviated object names to stay unique for some time.
>>>       The minimum length is 4.
>>> +
>>> +core.featureAdoptionRate::
>>> +     Set an integer value on a scale from 0 to 10 describing your
>>> +     desire to adopt new performance features. Defaults to 0. As
>>> +     the value increases, features are enabled by changing the
>>> +     default values of other config settings. If a config variable
>>> +     is specified explicitly, the explicit value will override these
>>> +     defaults:
>>> ++
>>> +If the value is at least 3, then the following defaults are modified.
>>> +These represent relatively new features that have existed for multiple
>>> +major releases, and present significant performance benefits. They do
>>> +not modify the user-facing output of porcelain commands.
>>> ++
>>> +* `core.commitGraph=true` enables reading commit-graph files.
>>> ++
>>> +* `gc.writeCommitGraph=true` eneables writing commit-graph files during
>>
>> I barked up a similar tree in
>> https://public-inbox.org/git/CACBZZX5SbYo5fVPtK6LW1FF96nR5591RHHC-5wdjW-fmg1R0EQ@mail.gmail.com/
>>
>> I wonder if you've seen that & what you think about that
>> approach. I.e. have a core.version=2.28 (or core.version=+6) or whatever
>> to opt-in to features we'd make default in 2.28. Would that be your
>> core.featureAdoptionRate=6 (28-28 = 6)?

I had not seen that message. Thanks for the link.

However, I don't think that your idea of "give me features that will be
default soon" is enough, as some of these features will _never_ be
turned on by default. It's more about how much a user is willing to have
some slight changes (i.e. extra files during maintenance [commit-graph],
different index format, changed ahead/behind messages in status) in
exchange for an overall "better" experience. Here "better" is decided by
the community members adjusting the values on this feature.
>> I admit that question is partly rhetorical, because I think it suggests
>> how hard it would be for users to reason about this.

The intention here is to make this as simple for users as possible. I want
to be able to say "I highly recommend you set core.featureAdoptionRate=5"
and for them to not need to know what is happening. Of course, they can
learn more about the details and opt-out as things change.

>> The "core.version" idea also sucks, but at least it's bound to our
>> advertised version number, so it's obvious if you set it to e.g. +2 what
>> feature track you're on, and furthermore when we'd commit to making that
>> the default for users who don't set core.version (although we could of
>> course always change our minds...). It's also something that mirrors how
>> e.g. Perl, C compilers (with --std=*) treat this sort of thing.
>>
>> So I'm all for a facility to have a setting to collectively opt-in to
>> new things early. But I think for such a thing we really should a) at
>> least in principle commit to making those things the default eventually
> 
> Some features may be best enabled for certain setups. This is why I
> set configuration variables repo size, worktree size.. instead of just
> one number.

You are right that some features are best for different scale factors:

 * commit-graph is best with a deep history.
 * index version 4 is best with a large working directory.

These things are usually correlated, and users don't always know what
exactly is "large" for any of these variables.

Further, these features actually don't have much downside even if
a user is legitimately struggling with one of these scale factors and
not another.

>> (if they don't suck) b) it needs to be obvious to the user how the
>> "rate" relates to git releases.
> 
> I see this more like gcc =O options. And for those options, the
> developers decide what to include. If you know what you want already,
> you can just turn specific keys on. Otherwise you count on devs to do
> the right things.
> 
> It would help if we have something like "gcc -Q -O2 --help=optimizers"
> so you can see exactly what you need to turn on to achieve the same
> thing. Then you can just set those have the same "per release"
> settings.
> 
> Which makes me think about a slightly different implementation detail
> (which I ignored because I didn't think further about per-release
> stuff): since these are basically meta config to change defaults, we
> can just implement them as a (builtin, or bundled) config file. The
> user can see what are included much easier we have several different
> config "profiles" (deep history, large worktree, bleeding-edge...) and
> the user can include one or all [1].

In some sense, this is creating a list of growing config profiles that
each include the previous one:

  config-3 (core.commitGraph, gc.writeCommitGraph, index.version)
  config-5 (config-3 + pack.useSparse)
  config-7 (config-5 + status.aheadBehind, fetch.showForcedUpdates)

The issue you seem to have is that we are not creating multiple
dimensions of options. I think simplicity is key here. Anyone with
the knowledge to deeply understand multiple dimensions can just
assign the config options as they see fit. This is _not_ a feature
for power users.
 
> [1] it also opens up the opportunity to have a standard (but optional)
> set of aliases. But that's a touchy topic.

Standard aliases is an interesting topic, but tangential to the
topic at hand and I'd prefer to leave it out of the current
discussion.

-Stolee

Re: [PATCH v2 1/3] repo-settings: create core.featureAdoptionRate setting

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> So I'm all for a facility to have a setting to collectively opt-in to
>> new things early. But I think for such a thing we really should a) at
>> least in principle commit to making those things the default eventually
>
> Some features may be best enabled for certain setups. This is why I
> set configuration variables repo size, worktree size.. instead of just
> one number.

Yeah, I think the concept of core.fetureAdoptionRate is faulty at
multiple counts, and I admit I am guilty of making at least one
aspect worse by giving the topic branch to queue these patches a
mistaken name of "early-adoption".

Some tweaks, like the use of index version 4, may be something we
strive to make it eventually suitable for _all_ users.  

The effort may involve multiple iterations of things like "gee, the
prefix-compression works very well for really big tree, but sucks
for a project of medium size; lets tweak to automatically
enable/disable it based on the size of the tree", but the main point
is that we want to eventually make it good for projects of all sizes
and different access patterns.  While we do the treaking, the user
experience may be rocky, and "early adoption" model is perfectly
suitable for a thing like this.

But some other tweaks, like the ahead-behind thing, are what we
would never make it the default for everybody.  They are "Git is
never designed to be used like this, but if we disable small things
like this and that, the end user experience for those who used to
have them might suffer, but other aspect of the system becomes
usable" tradeoffs.  When we are done experimenting and know what
kind of system castration may give acceptable trade off, we know the
subset of users to whom these tweaks give benefit (and others to whom
these are not improvements).  Opting into these things is not about
"early adoption".

Also as raised in another message in this thread, I do agree that
the configuration does not belong to the "core." hierarchy.  It is
more like a macro, that flips individual configuration based on a
higher level "grouping" (e.g. my project falls into "large but
infrequently updated" category) to suit the access pattern.

> I see this more like gcc =O options. And for those options, the
> developers decide what to include. If you know what you want already,
> you can just turn specific keys on. Otherwise you count on devs to do
> the right things.

Yup.  Sorry for backing a wrong model.  And I kind of like the word
"bundled" you mention below, not as in "bundled with Git", but more
as in "these configuration settings are bundled together to serve
users of this kind of project".

> Which makes me think about a slightly different implementation detail
> (which I ignored because I didn't think further about per-release
> stuff): since these are basically meta config to change defaults, we
> can just implement them as a (builtin, or bundled) config file. The
> user can see what are included much easier we have several different
> config "profiles" (deep history, large worktree, bleeding-edge...) and
> the user can include one or all [1].
>
> [1] it also opens up the opportunity to have a standard (but optional)
> set of aliases. But that's a touchy topic.

Re: [PATCH v3 1/3] repo-settings: create core.featureAdoptionRate setting

[Jakub Narebski]

Duy Nguyen <pclouds@gmail.com> writes:
> On Mon, Jul 1, 2019 at 10:32 PM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> @@ -601,3 +602,22 @@ core.abbrev::
>>         in your repository, which hopefully is enough for
>>         abbreviated object names to stay unique for some time.
>>         The minimum length is 4.
>> +
>> +core.featureAdoptionRate::
>> +       Set an integer value on a scale from 0 to 10 describing your
>> +       desire to adopt new performance features. Defaults to 0. As
>> +       the value increases, features are enabled by changing the
>> +       default values of other config settings. If a config variable
>> +       is specified explicitly, the explicit value will override these
>> +       defaults:
>
> This is because I'd like to keep core.* from growing too big (it's
> already big), hard to read, search and maintain. Perhaps this should
> belong to a separate group? Something like tuning.something or
> defaults.something.

I'm not sure if I consider core.* too big.  Well, there are 55 or more
entries in this namespace.

>> +If the value is at least 3, then the following defaults are modified.
>> +These represent relatively new features that have existed for multiple
>> +major releases, and may present performance benefits. These benefits
>> +depend on the amount and kind of data in your repo and how you use it.
>
> Then instead of numeric values, maybe the user should write some sort
> description about the repo and we optimize for that, similar to gcc
> -Os optimized for size, -Ofast for compiler speed (-O<n> is all about
> execution speed).

I also do not like those magic numbers.

>
> We could write, for example, tuning.commitHistory = {small, medium,
> large} and tuning.worktree = {small, large, medium} and maybe
> tuning.refSize and use that to optimize. We can still have different
> optimization levels (probably just "none", "recommended" vs
> "aggressive" where agressive enables most new stuff),

I think we have three different things that are currently conflated in
one config variable and one value.

First is what we want to optimize for; is it on-disk repository size,
command performance / execution speed, or maybe convenient information.

Second is what type of repository we are dealing with.  Is there a
problem with long history, large number of files in checkout, large
and/or binary files, or all together?  The original `core.size=large`
(or proposed core.repositorySize) was all about this issue.  Another
issue that might be important is that if it is leaf developer
repository, or is it maintainer repository, etc. (which affects for
example how the push looks like).

Third is what tradeoffs we are willing to accept to get required
performance.  Are we willing to use additional stable optional features;
are we willing to use new experimental optional features; are we
willing; are we willing to sacrifice convenience (ahead/behind
information in status, information bout forced updates in push output,
etc.) for performance?  This what current proposal is about.

It may not nnned to be a separate confi variable for a separate aspect;
it may be enough to have value that is space-separated list, or
something like that.

Best,
--
Jakub Narębski

Re: [PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Derrick Stolee]

On 7/1/2019 10:29 AM, Derrick Stolee via GitGitGadget wrote:
> Here is a second run at this RFC, which aims to create a "meta" config
> setting that automatically turns on other settings according to a user's
> willingness to trade new Git behavior or new feature risk for performance
> benefits. The new name for the setting is "core.featureAdoptionRate" and is
> an integer scale from 0 to 10. There will be multiple "categories" of
> settings, and the intention is to allow more granular levels as necessary.

(Adding people who contributed feedback to CC line.)

It seems that this "Feature Adoption Rate" idea was too simplistic, and
had several issues. Time to take a different stab at this direction, but
with these clear goals in mind:

 1. We want intermediate users to be able to take advantage of new config
    options without watching every release for new config options.

 2. The config name should match the general effect of the implied
    settings.

 3. There are orthogonal settings that may not apply beneficially to
    all repos.

With this in mind, I propose instead a set of "feature.*" config settings
that form groups of "community recommended" settings (with some caveats).
In the space below, I'll list a set of possible feature names and the
implied config options.

First, the main two categories we've discussed so far: many commits and
many files. These two feature sets are for when your repo is large in
one of these dimensions. Perhaps there are other settings to include
in these?

	feature.manyFiles:
		index.version = 4
		index.threads = true
		core.untrackedCache = true

	feature.manyCommits:
		core.commitGraph = true
		gc.writeCommitGraph = true
		(future: fetch.writeSplitCommitGraph = true)

Note: the `fetch.writeSplitCommitGraph` does not exist yet, but could
be introduced in a later release to write a new commit-graph (with --split)
on fetch.

The other category that has been discussed already is that of "experimental
features that we generally think are helpful but change behavior slightly in
some cases".

	feature.experimental:
		pack.useSparse = true
		status.aheadBehind = false
		fetch.showForcedUpdates = false
		merge.directoryRenames = true
		protocol.version = 2
		fetch.negotiationAlgorithm = skipping

We have not discussed anything like the next category, but Dscho thought
a set of configs to make pretty diffs could be a fun "meta-config" setting:

	feature.prettyDiff:
		diff.color = auto
		ui.color = auto
		diff.context = 5
		diff.colorMoved = true
		diff.colorMovedWs = allow-indentation-change
		diff.algorithm = minimal

These are just a first round of suggestions. I'm sure we would enjoy a
debate around an optimal set of diff settings.

Finally, here is a kind of feature that I could imagine being helpful
in the future, but maybe is not a good idea to pursue right now. In
some cases users use "gc.auto = 0" to prevent all user-time blocking
maintenance. This can degrade performance over time as loose objects
and pack-files accumulate. The performance could mostly be recovered
by using a multi-pack-index, but there is not current way to automatically
write the file. This would not solve the space issues that happen here.

	feature.noGC:
		gc.auto = 0
		core.multiPackIndex = true
		(future: fetch.writeMultiPackIndex = true)

What do people think about this general idea? Are there any other
feature.* settings that could be useful? Any additional settings
to add to these groups?

Thanks,
-Stolee

Re: [PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Taylor Blau]

Hi Derrick,

I'm a little bit late to the part, but I think that this is a really
interesting feature with a lot of really interesting discussion so far.

I hope you don't mind me throwing in my $.02 as well :-).

On Mon, Jul 08, 2019 at 03:22:49PM -0400, Derrick Stolee wrote:
> On 7/1/2019 10:29 AM, Derrick Stolee via GitGitGadget wrote:
> > Here is a second run at this RFC, which aims to create a "meta" config
> > setting that automatically turns on other settings according to a user's
> > willingness to trade new Git behavior or new feature risk for performance
> > benefits. The new name for the setting is "core.featureAdoptionRate" and is
> > an integer scale from 0 to 10. There will be multiple "categories" of
> > settings, and the intention is to allow more granular levels as necessary.
>
> (Adding people who contributed feedback to CC line.)
>
> It seems that this "Feature Adoption Rate" idea was too simplistic, and
> had several issues. Time to take a different stab at this direction, but
> with these clear goals in mind:
>
>  1. We want intermediate users to be able to take advantage of new config
>     options without watching every release for new config options.
>
>  2. The config name should match the general effect of the implied
>     settings.
>
>  3. There are orthogonal settings that may not apply beneficially to
>     all repos.

I think that this is a clear representation of the initial reaction I
had to the 'core.featureAdoptionRate' idea. I had drafted a response to
advance these concerns before realizing that this subsequent RFC
existed, which does a nice job highlighting the concerns that I had.

> With this in mind, I propose instead a set of "feature.*" config settings
> that form groups of "community recommended" settings (with some caveats).
> In the space below, I'll list a set of possible feature names and the
> implied config options.

I think that 'feature.*' configuration settings are a good idea. They
address each of the above (3) concerns, since they are:

  1. Can be easily adopted by even novice-level users. Perhaps
     novice-users will not be setting 'feature.manyFiles = 1', but they
     can easily opt-in to organization-level features that have been
     defined to handle organization-specific concerns.

  2. This one is straightforward: I think that setting
     'feature.manyFiles = 1' is clearer than 'feature.adoptionRate = 3'.

  3. Right. Windows developers may have a different set of what features
     are interesting to adopt than, say, every-day users, and likewise
     for kernel developers, too.

> First, the main two categories we've discussed so far: many commits and
> many files. These two feature sets are for when your repo is large in
> one of these dimensions. Perhaps there are other settings to include
> in these?
>
> 	feature.manyFiles:
> 		index.version = 4
> 		index.threads = true
> 		core.untrackedCache = true
>
> 	feature.manyCommits:
> 		core.commitGraph = true
> 		gc.writeCommitGraph = true
> 		(future: fetch.writeSplitCommitGraph = true)

I think that for this *feature* (pun mostly unintended) to really shine,
we ought to adopt Junio's suggestion in [1] that we allow users to:

  * use pre-baked features that are defined within and shipped with
    Git itself.

  * define their own features and second-order features that can
    reference both pre-baked and user-defined feature groups.

I think that this will let, say, folks at Microsoft to define a set of
features that are interesting to Windows developers, that are separate
from the features that core Git thinks will be interesting to every-day
users.

>
> <snip>
>
> Thanks,
> -Stolee

Thanks,
Taylor

[1]: https://public-inbox.org/git/xmqqftonsr6a.fsf@gitster-ct.c.googlers.com/

Re: [PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> The other category that has been discussed already is that of "experimental
> features that we generally think are helpful but change behavior slightly in
> some cases".
>
> 	feature.experimental:
> 		pack.useSparse = true
> 		status.aheadBehind = false
> 		fetch.showForcedUpdates = false
> 		merge.directoryRenames = true
> 		protocol.version = 2
> 		fetch.negotiationAlgorithm = skipping

Other classes you listed I can easily support, but I have trouble
deciding if this concept itself is bad, or merely that some/many of
the sample knobs you listed above are not exactly appropriate.
Either way, I have hard time swallowing this one as-is.  You may
think aheadBehind==false is helpful, but I don't, for example, and
there may be people for and against each of the experimental knobs.

But there may be a clear set of "this is agreed to be the way to the
future, but the implementation currently is too convoluted and
suspected of bugs, so we'll let early adoptors opt into the feature,
and when that happens, eventually this knob will go away (i.e. you
won't be able to turn it off)" type of knobs.  Or it may change the
behaviour drastically, but as long as it is agreed that the future
lies in that direction, I think it is OK to throw such a knob into
this class.  The key points are (1) we are committed that in the
future everybody will be forced to have it and (2) it is not merely
"we generally think", but "the decision about the future has been
made---there won't be any other way".  The feature.experimental
becomes merely a way to let early adoptors in.  If you limit the
individual features governed by feature.experimental to that kind of
knobs, I can be easily convinced that this class is a good idea.

Re: [PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Derrick Stolee]

On 7/9/2019 3:21 PM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
> 
>> The other category that has been discussed already is that of "experimental
>> features that we generally think are helpful but change behavior slightly in
>> some cases".
>>
>> 	feature.experimental:
>> 		pack.useSparse = true
>> 		status.aheadBehind = false
>> 		fetch.showForcedUpdates = false
>> 		merge.directoryRenames = true
>> 		protocol.version = 2
>> 		fetch.negotiationAlgorithm = skipping
> 
> Other classes you listed I can easily support, but I have trouble
> deciding if this concept itself is bad, or merely that some/many of
> the sample knobs you listed above are not exactly appropriate.
> Either way, I have hard time swallowing this one as-is.  You may
> think aheadBehind==false is helpful, but I don't, for example, and
> there may be people for and against each of the experimental knobs.

Thanks for the specific note about aheadBehind. I'll drop that one
from consideration.

I suppose that fetch.showForcedUpdates is in the same category, and
it has a self-discovery mechanism (a warning message) for users who
feel the pain of checking for forced updates (i.e. it takes >10s).

> But there may be a clear set of "this is agreed to be the way to the
> future, but the implementation currently is too convoluted and
> suspected of bugs, so we'll let early adoptors opt into the feature,
> and when that happens, eventually this knob will go away (i.e. you
> won't be able to turn it off)" type of knobs.  Or it may change the
> behaviour drastically, but as long as it is agreed that the future
> lies in that direction, I think it is OK to throw such a knob into
> this class.  The key points are (1) we are committed that in the
> future everybody will be forced to have it and (2) it is not merely
> "we generally think", but "the decision about the future has been
> made---there won't be any other way".  The feature.experimental
> becomes merely a way to let early adoptors in.  If you limit the
> individual features governed by feature.experimental to that kind of
> knobs, I can be easily convinced that this class is a good idea.

From this list, do you think any of these settings are likely to
become defaults? It seems that protocol.version = 2 may be a default
now that _most_ services have an implementation, and it always falls
back to protocol v1 without extra cost.

When pack.useSparse was first introduced, I considered making it true
by default after a while. But you protested, saying you want people
knocking at the door saying it is useful. What if it lived here?

fetch.negotiationAlgorithm and merge.directoryRenames seem like
valuable features and maybe just need more time out in the world
before they could be considered defaults.

I appreciate all of the feedback, and to drive the discussion forward
I'm trying to tease out very specific opinions.

Thanks,
-Stolee

Re: [PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Junio C Hamano]

Derrick Stolee <stolee@gmail.com> writes:

> From this list, do you think any of these settings are likely to
> become defaults? It seems that protocol.version = 2 may be a default
> now that _most_ services have an implementation, and it always falls
> back to protocol v1 without extra cost.
>
> When pack.useSparse was first introduced, I considered making it true
> by default after a while. But you protested, saying you want people
> knocking at the door saying it is useful. What if it lived here?
>
> fetch.negotiationAlgorithm and merge.directoryRenames seem like
> valuable features and maybe just need more time out in the world
> before they could be considered defaults.

I mostly agree with the categorization you gave above.

I think it is perfectly fine for a knob, after proving its worth by
existing in the world without being a part of any feature.* set, to
become part of feature.experimental, and then later be ejected
without ever becoming the default in response to reactions by real
world users.  This would be easier to arrange if we had at least two
experiment levels.  One class would be "we are firmly committed to
make these default in the future and ironing kinks out---please help
by setting feature.experimental on" and is more for early adopter
testing.  The other class may be "we try this on users to see if
there are some populations of them with usage patterns we did not
anticipate, and will yank it out if it turns out to be problematic
to some users."  The more guinea pig users opt into the latter
"Highly Experimental" category, the more help they can give us to
prevent an ill-thought-out feature that does not universally help to
become a new default.

Re: [PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Jakub Narebski]

Derrick Stolee <stolee@gmail.com> writes:
> On 7/1/2019 10:29 AM, Derrick Stolee via GitGitGadget wrote:
>>
>> Here is a second run at this RFC, which aims to create a "meta" config
>> setting that automatically turns on other settings according to a user's
>> willingness to trade new Git behavior or new feature risk for performance
>> benefits. The new name for the setting is "core.featureAdoptionRate" and is
>> an integer scale from 0 to 10. There will be multiple "categories" of
>> settings, and the intention is to allow more granular levels as necessary.
>
> (Adding people who contributed feedback to CC line.)
>
> It seems that this "Feature Adoption Rate" idea was too simplistic, and
> had several issues. Time to take a different stab at this direction, but
> with these clear goals in mind:
>
>  1. We want intermediate users to be able to take advantage of new config
>     options without watching every release for new config options.
>
>  2. The config name should match the general effect of the implied
>     settings.
>
>  3. There are orthogonal settings that may not apply beneficially to
>     all repos.
>
> With this in mind, I propose instead a set of "feature.*" config settings
> that form groups of "community recommended" settings (with some caveats).
> In the space below, I'll list a set of possible feature names and the
> implied config options.

A bit of bikeshed painting: I am unsure if "feature.*" is the best name
for this category of config (meta)settings.  Perhaps "defaults.*" or
"presets.*" would be a better name -- they would certainly be more
indicative of what setting this config variable actually *does*.

> First, the main two categories we've discussed so far: many commits and
> many files. These two feature sets are for when your repo is large in
> one of these dimensions. Perhaps there are other settings to include
> in these?
>
> 	feature.manyFiles:
> 		index.version = 4
> 		index.threads = true
> 		core.untrackedCache = true
>
> 	feature.manyCommits:
> 		core.commitGraph = true
> 		gc.writeCommitGraph = true
> 		(future: fetch.writeSplitCommitGraph = true)
>
> Note: the `fetch.writeSplitCommitGraph` does not exist yet, but could
> be introduced in a later release to write a new commit-graph (with --split)
> on fetch.

That looks really nice (for a built-in set of defaults).

It would be good if the above format, or something like it, could be
used as a source of truth for this feature.

> The other category that has been discussed already is that of "experimental
> features that we generally think are helpful but change behavior slightly in
> some cases".
>
> 	feature.experimental:
> 		pack.useSparse = true
> 		status.aheadBehind = false
> 		fetch.showForcedUpdates = false
> 		merge.directoryRenames = true
> 		protocol.version = 2
> 		fetch.negotiationAlgorithm = skipping

Well... turning off by default status.aheadBehind and
fetch.showForcedUpdates makes sense only if also repository is large.
Otherwise it is not useful, and even a bad thing.

Both of status.aheadBehind and fetch.showForcedUpdates are discoverable;
as far as I remember Git would show a hint about those config options
when the related task takes too long (either 'git status' in case of
status.aheadBehind, or 'git fetch' in the latter case).

I don't know if we have discoverability in the opposite direction: do we
show some advice (which as all advice can be turned off in the config)
if either of status.aheadBehind or fetch.showForcedUpdates is false?

> We have not discussed anything like the next category, but Dscho thought
> a set of configs to make pretty diffs could be a fun "meta-config" setting:
>
> 	feature.prettyDiff:
> 		diff.color = auto
> 		ui.color = auto
> 		diff.context = 5
> 		diff.colorMoved = true
> 		diff.colorMovedWs = allow-indentation-change
> 		diff.algorithm = minimal
>
> These are just a first round of suggestions. I'm sure we would enjoy a
> debate around an optimal set of diff settings.

Maybe Git for Windows defaults would be shipped in similar form; though
I wonder if in this case it is better from simple system-wide settings.

> Finally, here is a kind of feature that I could imagine being helpful
> in the future, but maybe is not a good idea to pursue right now. In
> some cases users use "gc.auto = 0" to prevent all user-time blocking
> maintenance.

Don't we have a hook for that?

>              This can degrade performance over time as loose objects
> and pack-files accumulate. The performance could mostly be recovered
> by using a multi-pack-index, but there is not current way to automatically
> write the file. This would not solve the space issues that happen here.
>
> 	feature.noGC:
> 		gc.auto = 0
> 		core.multiPackIndex = true
> 		(future: fetch.writeMultiPackIndex = true)
>
> What do people think about this general idea? Are there any other
> feature.* settings that could be useful? Any additional settings
> to add to these groups?

Maybe feature.slowFilesystem / defaults.slowFilesystem?  Or maybe
feature.server?

Best regards,
-- 
Jakub Narębski

Re: [PATCH v3 0/3] [RFC] Create 'core.featureAdoptionRate' setting to update config defaults

[Derrick Stolee]

On 7/9/2019 6:05 PM, Junio C Hamano wrote:
> Derrick Stolee <stolee@gmail.com> writes:
> 
>> From this list, do you think any of these settings are likely to
>> become defaults? It seems that protocol.version = 2 may be a default
>> now that _most_ services have an implementation, and it always falls
>> back to protocol v1 without extra cost.
>>
>> When pack.useSparse was first introduced, I considered making it true
>> by default after a while. But you protested, saying you want people
>> knocking at the door saying it is useful. What if it lived here?
>>
>> fetch.negotiationAlgorithm and merge.directoryRenames seem like
>> valuable features and maybe just need more time out in the world
>> before they could be considered defaults.
> 
> I mostly agree with the categorization you gave above.
> 
> I think it is perfectly fine for a knob, after proving its worth by
> existing in the world without being a part of any feature.* set, to
> become part of feature.experimental, and then later be ejected
> without ever becoming the default in response to reactions by real
> world users.  This would be easier to arrange if we had at least two
> experiment levels.  One class would be "we are firmly committed to
> make these default in the future and ironing kinks out---please help
> by setting feature.experimental on" and is more for early adopter
> testing.  The other class may be "we try this on users to see if
> there are some populations of them with usage patterns we did not
> anticipate, and will yank it out if it turns out to be problematic
> to some users."  The more guinea pig users opt into the latter
> "Highly Experimental" category, the more help they can give us to
> prevent an ill-thought-out feature that does not universally help to
> become a new default.

How about "feature.preview" for defaults we expect to change in a later
version, while "feature.experimental" is for defaults we are not sure
about?

-Stolee