From d8465500c3d5ced194585eea05b2a6dccfaa6366 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 19 Nov 2018 22:11:47 -0800 Subject: [PATCH 1/3] eoie: default to not writing EOIE section Since 3b1d9e04 (eoie: add End of Index Entry (EOIE) extension, 2018-10-10) Git defaults to writing the new EOIE section when writing out an index file. Usually that is a good thing because it improves threaded performance, but when a Git repository is shared with older versions of Git, it produces a confusing warning: $ git status ignoring EOIE extension HEAD detached at 371ed0defa nothing to commit, working tree clean Let's introduce the new index extension more gently. First we'll roll out the new version of Git that understands it, and then once sufficiently many users are using such a version, we can flip the default to writing it by default. Introduce a '[index] recordEndOfIndexEntries' configuration variable to allow interested users to benefit from this index extension early. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Documentation/config/index.txt | 7 +++++++ read-cache.c | 11 ++++++++++- t/t1700-split-index.sh | 11 +++++++---- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt index 4b94b6bedc..8e138aba7a 100644 --- a/Documentation/config/index.txt +++ b/Documentation/config/index.txt @@ -1,3 +1,10 @@ +index.recordEndOfIndexEntries:: + Specifies whether the index file should include an "End Of Index + Entry" section. This reduces index load time on multiprocessor + machines but produces a message "ignoring EOIE extension" when + reading the index using Git versions before 2.20. Defaults to + 'false'. + index.threads:: Specifies the number of threads to spawn when loading the index. This is meant to reduce index load time on multiprocessor machines. diff --git a/read-cache.c b/read-cache.c index 4ca81286c0..1e9c772603 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2689,6 +2689,15 @@ void update_index_if_able(struct index_state *istate, struct lock_file *lockfile rollback_lock_file(lockfile); } +static int record_eoie(void) +{ + int val; + + if (!git_config_get_bool("index.recordendofindexentries", &val)) + return val; + return 0; +} + /* * On success, `tempfile` is closed. If it is the temporary file * of a `struct lock_file`, we will therefore effectively perform @@ -2936,7 +2945,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, * read. Write it out regardless of the strip_extensions parameter as we need it * when loading the shared index. */ - if (offset) { + if (offset && record_eoie()) { struct strbuf sb = STRBUF_INIT; write_eoie_extension(&sb, &eoie_c, offset); diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh index 2ac47aa0e4..0cbac64e28 100755 --- a/t/t1700-split-index.sh +++ b/t/t1700-split-index.sh @@ -25,14 +25,17 @@ test_expect_success 'enable split index' ' git update-index --split-index && test-tool dump-split-index .git/index >actual && indexversion=$(test-tool index-version <.git/index) && + + # NEEDSWORK: Stop hard-coding checksums. if test "$indexversion" = "4" then - own=3527df833c6c100d3d1d921a9a782d62a8be4b58 - base=746f7ab2ed44fb839efdfbffcf399d0b113fb4cb + own=432ef4b63f32193984f339431fd50ca796493569 + base=508851a7f0dfa8691e9f69c7f055865389012491 else - own=5e9b60117ece18da410ddecc8b8d43766a0e4204 - base=4370042739b31cd17a5c5cd6043a77c9a00df113 + own=8299b0bcd1ac364e5f1d7768efb62fa2da79a339 + base=39d890139ee5356c7ef572216cebcd27aa41f9df fi && + cat >expect <<-EOF && own $own base $base From 429160544db9cc0cf748cdc98b21bd3533ec85a3 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 19 Nov 2018 22:12:22 -0800 Subject: [PATCH 2/3] ieot: default to not writing IEOT section As with EOIE, popular versions of Git do not support the new IEOT extension yet. When accessing a Git repository written by a more modern version of Git, they correctly ignore the unrecognized section, but in the process they loudly warn ignoring IEOT extension resulting in confusion for users. Introduce the index extension more gently by not writing it yet in this first version with support for it. Soon, once sufficiently many users are running a modern version of Git, we can flip the default so users benefit from this index extension by default. Introduce a '[index] recordOffsetTable' configuration variable to control whether the new index extension is written. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Documentation/config/index.txt | 7 +++++++ read-cache.c | 11 ++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt index 8e138aba7a..de44183235 100644 --- a/Documentation/config/index.txt +++ b/Documentation/config/index.txt @@ -5,6 +5,13 @@ index.recordEndOfIndexEntries:: reading the index using Git versions before 2.20. Defaults to 'false'. +index.recordOffsetTable:: + Specifies whether the index file should include an "Index Entry + Offset Table" section. This reduces index load time on + multiprocessor machines but produces a message "ignoring IEOT + extension" when reading the index using Git versions before 2.20. + Defaults to 'false'. + index.threads:: Specifies the number of threads to spawn when loading the index. This is meant to reduce index load time on multiprocessor machines. diff --git a/read-cache.c b/read-cache.c index 1e9c772603..f3d5638d9e 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2698,6 +2698,15 @@ static int record_eoie(void) return 0; } +static int record_ieot(void) +{ + int val; + + if (!git_config_get_bool("index.recordoffsettable", &val)) + return val; + return 0; +} + /* * On success, `tempfile` is closed. If it is the temporary file * of a `struct lock_file`, we will therefore effectively perform @@ -2761,7 +2770,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, else nr_threads = 1; - if (nr_threads != 1) { + if (nr_threads != 1 && record_ieot()) { int ieot_blocks, cpus; /* From 2a9dedef2ef76916be4a314a7e739f253eaf05db Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Mon, 19 Nov 2018 22:14:26 -0800 Subject: [PATCH 3/3] index: make index.threads=true enable ieot and eoie If a user explicitly sets [index] threads = true to read the index using multiple threads, ensure that index writes include the offset table by default to make that possible. This ensures that the user's intent of turning on threading is respected. In other words, permit the following configurations: - index.threads and index.recordOffsetTable unspecified: do not write the offset table yet (to avoid alarming the user with "ignoring IEOT extension" messages when an older version of Git accesses the repository) but do make use of multiple threads to read the index if the supporting offset table is present. This can also be requested explicitly by setting index.threads=true, 0, or >1 and index.recordOffsetTable=false. - index.threads=false or 1: do not write the offset table, and do not make use of the offset table. One can set index.recordOffsetTable=false as well, to be more explicit. - index.threads=true, 0, or >1 and index.recordOffsetTable unspecified: write the offset table and make use of threads at read time. This can also be requested by setting index.threads=true, 0, >1, or unspecified and index.recordOffsetTable=true. Fortunately the complication is temporary: once most Git installations have upgraded to a version with support for the IEOT and EOIE extensions, we can flip the defaults for index.recordEndOfIndexEntries and index.recordOffsetTable to true and eliminate the settings. Helped-by: Ben Peart Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- Documentation/config/index.txt | 6 ++++-- config.c | 17 ++++++++++------- config.h | 2 +- read-cache.c | 23 +++++++++++++++++------ 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/Documentation/config/index.txt b/Documentation/config/index.txt index de44183235..f181503041 100644 --- a/Documentation/config/index.txt +++ b/Documentation/config/index.txt @@ -3,14 +3,16 @@ index.recordEndOfIndexEntries:: Entry" section. This reduces index load time on multiprocessor machines but produces a message "ignoring EOIE extension" when reading the index using Git versions before 2.20. Defaults to - 'false'. + 'true' if index.threads has been explicitly enabled, 'false' + otherwise. index.recordOffsetTable:: Specifies whether the index file should include an "Index Entry Offset Table" section. This reduces index load time on multiprocessor machines but produces a message "ignoring IEOT extension" when reading the index using Git versions before 2.20. - Defaults to 'false'. + Defaults to 'true' if index.threads has been explicitly enabled, + 'false' otherwise. index.threads:: Specifies the number of threads to spawn when loading the index. diff --git a/config.c b/config.c index 04286f7717..ff521eb27a 100644 --- a/config.c +++ b/config.c @@ -2294,22 +2294,25 @@ int git_config_get_fsmonitor(void) return 0; } -int git_config_get_index_threads(void) +int git_config_get_index_threads(int *dest) { - int is_bool, val = 0; + int is_bool, val; val = git_env_ulong("GIT_TEST_INDEX_THREADS", 0); - if (val) - return val; + if (val) { + *dest = val; + return 0; + } if (!git_config_get_bool_or_int("index.threads", &is_bool, &val)) { if (is_bool) - return val ? 0 : 1; + *dest = val ? 0 : 1; else - return val; + *dest = val; + return 0; } - return 0; /* auto */ + return 1; } NORETURN diff --git a/config.h b/config.h index a06027e69b..ee5d3fa7b4 100644 --- a/config.h +++ b/config.h @@ -246,11 +246,11 @@ extern int git_config_get_bool(const char *key, int *dest); extern int git_config_get_bool_or_int(const char *key, int *is_bool, int *dest); extern int git_config_get_maybe_bool(const char *key, int *dest); extern int git_config_get_pathname(const char *key, const char **dest); +extern int git_config_get_index_threads(int *dest); extern int git_config_get_untracked_cache(void); extern int git_config_get_split_index(void); extern int git_config_get_max_percent_split_change(void); extern int git_config_get_fsmonitor(void); -extern int git_config_get_index_threads(void); /* This dies if the configured or default date is in the future */ extern int git_config_get_expiry(const char *key, const char **output); diff --git a/read-cache.c b/read-cache.c index f3d5638d9e..42de59a163 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2176,7 +2176,8 @@ int do_read_index(struct index_state *istate, const char *path, int must_exist) src_offset = sizeof(*hdr); - nr_threads = git_config_get_index_threads(); + if (git_config_get_index_threads(&nr_threads)) + nr_threads = 1; /* TODO: does creating more threads than cores help? */ if (!nr_threads) { @@ -2695,7 +2696,13 @@ static int record_eoie(void) if (!git_config_get_bool("index.recordendofindexentries", &val)) return val; - return 0; + + /* + * As a convenience, the end of index entries extension + * used for threading is written by default if the user + * explicitly requested threaded index reads. + */ + return !git_config_get_index_threads(&val) && val != 1; } static int record_ieot(void) @@ -2704,7 +2711,13 @@ static int record_ieot(void) if (!git_config_get_bool("index.recordoffsettable", &val)) return val; - return 0; + + /* + * As a convenience, the offset table used for threading is + * written by default if the user explicitly requested + * threaded index reads. + */ + return !git_config_get_index_threads(&val) && val != 1; } /* @@ -2765,9 +2778,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile, if (ce_write(&c, newfd, &hdr, sizeof(hdr)) < 0) return -1; - if (HAVE_THREADS) - nr_threads = git_config_get_index_threads(); - else + if (!HAVE_THREADS || git_config_get_index_threads(&nr_threads)) nr_threads = 1; if (nr_threads != 1 && record_ieot()) {