From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org, git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
"Derrick Stolee" <derrickstolee@github.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Emily Shaffer" <emilyshaffer@google.com>
Subject: [PATCH v2 0/2] gc: let feature.experimental imply gc.cruftPacks
Date: Wed, 26 Oct 2022 16:13:34 -0400 [thread overview]
Message-ID: <cover.1666815209.git.me@ttaylorr.com> (raw)
This is a moderately-sized reroll of Emily's series from [1], which I
saw while sifting through old mail in my inbox.
That series was sent when I was on vacation, which is a likely
explanation for why I most likely missed it.
I'm sending a reroll out on Emily's behalf, since the series hasn't
received any activity in a few months, and I would like to see the
topic pushed forward.
Changes since last time are included in a range-diff below, but the
major points are:
- extracted common components of the test script into helper
functions
- reordered the repository creation/teardown so that
test_when_finished "rm -fr ..." happens before "git init"
- and centralized checking feature.experimental into
repo-settings.c
Thanks in advance for your review.
[1]: https://lore.kernel.org/git/20220803205721.3686361-1-emilyshaffer@google.com/
Emily Shaffer (2):
gc: add tests for --cruft and friends
config: let feature.experimental imply gc.cruftPacks=true
Documentation/config/feature.txt | 3 ++
builtin/gc.c | 7 ++-
repo-settings.c | 1 +
repository.h | 1 +
t/t6500-gc.sh | 84 ++++++++++++++++++++++++++++++++
5 files changed, 94 insertions(+), 2 deletions(-)
Range-diff against v1:
1: bf243d15c1 ! 1: 35d2c97715 gc: add tests for --cruft and friends
@@ Commit message
Add some tests to exercise these options to gc in the gc test suite.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
+ Signed-off-by: Taylor Blau <me@ttaylorr.com>
## t/t6500-gc.sh ##
@@ t/t6500-gc.sh: test_expect_success 'one of gc.reflogExpire{Unreachable,}=never does not skip "e
grep -E "^trace: (built-in|exec|run_command): git reflog expire --" trace.out
'
++prepare_cruft_history () {
++ test_commit base &&
++
++ test_commit --no-tag foo &&
++ test_commit --no-tag bar &&
++ git reset HEAD^^
++}
++
++assert_cruft_pack_exists () {
++ find .git/objects/pack -name "*.mtimes" >mtimes &&
++ sed -e 's/\.mtimes$/\.pack/g' mtimes >packs &&
++
++ test_file_not_empty packs &&
++ while read pack
++ do
++ test_path_is_file "$pack" || return 1
++ done <packs
++}
++
+test_expect_success 'gc --cruft generates a cruft pack' '
-+ git init crufts &&
+ test_when_finished "rm -fr crufts" &&
++ git init crufts &&
+ (
+ cd crufts &&
-+ test_commit base &&
-+
-+ test_commit --no-tag foo &&
-+ test_commit --no-tag bar &&
-+ git reset HEAD^^ &&
+
++ prepare_cruft_history &&
+ git gc --cruft &&
-+
-+ cruft=$(basename $(ls .git/objects/pack/pack-*.mtimes) .mtimes) &&
-+ test_path_is_file .git/objects/pack/$cruft.pack
++ assert_cruft_pack_exists
+ )
+'
+
+test_expect_success 'gc.cruftPacks=true generates a cruft pack' '
-+ git init crufts &&
+ test_when_finished "rm -fr crufts" &&
++ git init crufts &&
+ (
+ cd crufts &&
-+ test_commit base &&
-+
-+ test_commit --no-tag foo &&
-+ test_commit --no-tag bar &&
-+ git reset HEAD^^ &&
+
++ prepare_cruft_history &&
+ git -c gc.cruftPacks=true gc &&
-+
-+ cruft=$(basename $(ls .git/objects/pack/pack-*.mtimes) .mtimes) &&
-+ test_path_is_file .git/objects/pack/$cruft.pack
++ assert_cruft_pack_exists
+ )
+'
+
2: 5a242e2370 ! 2: eb151752b8 config: let feature.experimental imply gc.cruftPacks=true
@@ Commit message
config: let feature.experimental imply gc.cruftPacks=true
We are interested in exploring whether gc.cruftPacks=true should become
- the default value; to determine whether it is safe to do so, let's
- encourage more users to try it out. Users who have set
- feature.experimental=true have already volunteered to try new and
- possibly-breaking config changes, so let's try this new default with
- that set of users.
+ the default value.
+
+ To determine whether it is safe to do so, let's encourage more users to
+ try it out.
+
+ Users who have set feature.experimental=true have already volunteered to
+ try new and possibly-breaking config changes, so let's try this new
+ default with that set of users.
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
+ Signed-off-by: Taylor Blau <me@ttaylorr.com>
## Documentation/config/feature.txt ##
@@ Documentation/config/feature.txt: feature.experimental::
+
* `fetch.negotiationAlgorithm=skipping` may improve fetch negotiation times by
skipping more commits at a time, reducing the number of round trips.
+++
+* `gc.cruftPacks=true` reduces disk space used by unreachable objects during
-+garbage collection.
++garbage collection, preventing loose object explosions.
feature.manyFiles::
Enable config options that optimize for repos with many files in the
## builtin/gc.c ##
-@@ builtin/gc.c: static int gc_config_is_timestamp_never(const char *var)
- static void gc_config(void)
- {
- const char *value;
-+ int experimental = 0;
+@@ builtin/gc.c: static const char * const builtin_gc_usage[] = {
- if (!git_config_get_value("gc.packrefs", &value)) {
- if (value && !strcmp(value, "notbare"))
-@@ builtin/gc.c: static void gc_config(void)
- gc_config_is_timestamp_never("gc.reflogexpireunreachable"))
- prune_reflogs = 0;
+ static int pack_refs = 1;
+ static int prune_reflogs = 1;
+-static int cruft_packs = 0;
++static int cruft_packs = -1;
+ static int aggressive_depth = 50;
+ static int aggressive_window = 250;
+ static int gc_auto_threshold = 6700;
+@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
+ if (prune_expire && parse_expiry_date(prune_expire, &dummy))
+ die(_("failed to parse prune expiry value %s"), prune_expire);
-+ /* feature.experimental implies gc.cruftPacks=true */
-+ git_config_get_bool("feature.experimental", &experimental);
-+ if (experimental)
-+ cruft_packs = 1;
++ prepare_repo_settings(the_repository);
++ if (cruft_packs < 0)
++ cruft_packs = the_repository->settings.gc_cruft_packs;
+
- git_config_get_int("gc.aggressivewindow", &aggressive_window);
- git_config_get_int("gc.aggressivedepth", &aggressive_depth);
- git_config_get_int("gc.auto", &gc_auto_threshold);
+ if (aggressive) {
+ strvec_push(&repack, "-f");
+ if (aggressive_depth > 0)
+@@ builtin/gc.c: int cmd_gc(int argc, const char **argv, const char *prefix)
+ clean_pack_garbage();
+ }
+
+- prepare_repo_settings(the_repository);
+ if (the_repository->settings.gc_write_commit_graph == 1)
+ write_commit_graph_reachable(the_repository->objects->odb,
+ !quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,
+
+ ## repo-settings.c ##
+@@ repo-settings.c: void prepare_repo_settings(struct repository *r)
+ /* Defaults modified by feature.* */
+ if (experimental) {
+ r->settings.fetch_negotiation_algorithm = FETCH_NEGOTIATION_SKIPPING;
++ r->settings.gc_cruft_packs = 1;
+ }
+ if (manyfiles) {
+ r->settings.index_version = 4;
+
+ ## repository.h ##
+@@ repository.h: struct repo_settings {
+ int commit_graph_generation_version;
+ int commit_graph_read_changed_paths;
+ int gc_write_commit_graph;
++ int gc_cruft_packs;
+ int fetch_write_commit_graph;
+ int command_requires_full_index;
+ int sparse_index;
## t/t6500-gc.sh ##
+@@ t/t6500-gc.sh: assert_cruft_pack_exists () {
+ done <packs
+ }
+
++refute_cruft_packs_exist () {
++ find .git/objects/pack -name "*.mtimes" >mtimes &&
++ test_must_be_empty mtimes
++}
++
+ test_expect_success 'gc --cruft generates a cruft pack' '
+ test_when_finished "rm -fr crufts" &&
+ git init crufts &&
@@ t/t6500-gc.sh: test_expect_success 'gc.cruftPacks=true generates a cruft pack' '
)
'
@@ t/t6500-gc.sh: test_expect_success 'gc.cruftPacks=true generates a cruft pack' '
+ test_when_finished "rm -fr crufts" &&
+ (
+ cd crufts &&
-+ test_commit base &&
-+
-+ test_commit --no-tag foo &&
-+ test_commit --no-tag bar &&
-+ git reset HEAD^^ &&
+
++ prepare_cruft_history &&
+ git -c feature.experimental=true gc &&
-+
-+ cruft=$(basename $(ls .git/objects/pack/pack-*.mtimes) .mtimes) &&
-+ test_path_is_file .git/objects/pack/$cruft.pack
++ assert_cruft_pack_exists
+ )
+'
+
@@ t/t6500-gc.sh: test_expect_success 'gc.cruftPacks=true generates a cruft pack' '
+ test_when_finished "rm -fr crufts" &&
+ (
+ cd crufts &&
-+ test_commit base &&
-+
-+ test_commit --no-tag foo &&
-+ test_commit --no-tag bar &&
-+ git reset HEAD^^ &&
+
++ prepare_cruft_history &&
+ git -c gc.cruftPacks=true -c feature.experimental=false gc &&
-+ cruft=$(basename $(ls .git/objects/pack/pack-*.mtimes) .mtimes) &&
-+ test_path_is_file .git/objects/pack/$cruft.pack
++ assert_cruft_pack_exists
++ )
++'
++
++test_expect_success 'feature.experimental=false avoids cruft packs by default' '
++ git init crufts &&
++ test_when_finished "rm -fr crufts" &&
++ (
++ cd crufts &&
++
++ prepare_cruft_history &&
++ git -c feature.experimental=false gc &&
++ refute_cruft_packs_exist
+ )
+'
+
--
2.38.0.16.g393fd4c6db
next reply other threads:[~2022-10-26 20:13 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-26 20:13 Taylor Blau [this message]
2022-10-26 20:13 ` [PATCH v2 1/2] gc: add tests for --cruft and friends Taylor Blau
2022-10-26 21:03 ` Junio C Hamano
2022-10-26 20:13 ` [PATCH v2 2/2] config: let feature.experimental imply gc.cruftPacks=true Taylor Blau
2022-10-26 21:15 ` Junio C Hamano
2022-10-26 21:32 ` Taylor Blau
2022-10-26 20:53 ` [PATCH v2 0/2] gc: let feature.experimental imply gc.cruftPacks Junio C Hamano
2022-10-26 21:32 ` [PATCH v3 " Taylor Blau
2022-10-26 21:32 ` [PATCH v3 1/2] gc: add tests for --cruft and friends Taylor Blau
2022-10-26 21:32 ` [PATCH v3 2/2] config: let feature.experimental imply gc.cruftPacks=true Taylor Blau
2022-10-31 10:46 ` Derrick Stolee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1666815209.git.me@ttaylorr.com \
--to=me@ttaylorr.com \
--cc=avarab@gmail.com \
--cc=derrickstolee@github.com \
--cc=emilyshaffer@google.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.