| Message ID | 20190314123439.4347-6-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gc: minor code cleanup + contention fixes | expand |
On Thu, Mar 14, 2019 at 7:35 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > When gc.reflogExpire and gc.reflogExpireUnreachable are set to "never" > and --stale-fix isn't in effect (covered by the first part of the "if" > statement being modified here) we can exit early without pointlessly > looping over all the reflogs. > > Signed-off-by: Jeff King <peff@peff.net> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > builtin/reflog.c | 7 +++++++ > t/t1410-reflog.sh | 4 +++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/builtin/reflog.c b/builtin/reflog.c > index 4d3430900d..d95c77ca0e 100644 > --- a/builtin/reflog.c > +++ b/builtin/reflog.c > @@ -606,6 +606,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) > mark_reachable_objects(&cb.cmd.revs, 0, 0, NULL); > if (flags & EXPIRE_REFLOGS_VERBOSE) > putchar('\n'); > + } else if (!cb.cmd.expire_total && !cb.cmd.expire_unreachable) { > + /* > + * If we're not expiring anything and not dropping > + * stale entries, there's no point in even opening the > + * reflogs, since we're guaranteed to do nothing. > + */ I'm checking should_expire_reflog_ent(). With both of these being zero, we skip most of the "return 1;" except the last one cb->cmd.recno, added in 552cecc214 (Teach "git reflog" a subcommand to delete single entries, 2007-10-17). Will this shortcut affect that use case (I haven't spent time understanding that commit yet, gotta run soon)? > + return 0; > }
On Fri, Mar 15 2019, Duy Nguyen wrote: > On Thu, Mar 14, 2019 at 7:35 PM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> >> When gc.reflogExpire and gc.reflogExpireUnreachable are set to "never" >> and --stale-fix isn't in effect (covered by the first part of the "if" >> statement being modified here) we can exit early without pointlessly >> looping over all the reflogs. >> >> Signed-off-by: Jeff King <peff@peff.net> >> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> >> --- >> builtin/reflog.c | 7 +++++++ >> t/t1410-reflog.sh | 4 +++- >> 2 files changed, 10 insertions(+), 1 deletion(-) >> >> diff --git a/builtin/reflog.c b/builtin/reflog.c >> index 4d3430900d..d95c77ca0e 100644 >> --- a/builtin/reflog.c >> +++ b/builtin/reflog.c >> @@ -606,6 +606,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) >> mark_reachable_objects(&cb.cmd.revs, 0, 0, NULL); >> if (flags & EXPIRE_REFLOGS_VERBOSE) >> putchar('\n'); >> + } else if (!cb.cmd.expire_total && !cb.cmd.expire_unreachable) { >> + /* >> + * If we're not expiring anything and not dropping >> + * stale entries, there's no point in even opening the >> + * reflogs, since we're guaranteed to do nothing. >> + */ > > I'm checking should_expire_reflog_ent(). With both of these being > zero, we skip most of the "return 1;" except the last one > cb->cmd.recno, added in 552cecc214 (Teach "git reflog" a subcommand to > delete single entries, 2007-10-17). Will this shortcut affect that use > case (I haven't spent time understanding that commit yet, gotta run > soon)? There was a bug related to this. Fixed in v3. > >> + return 0; >> }
diff --git a/builtin/reflog.c b/builtin/reflog.c index 4d3430900d..d95c77ca0e 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -606,6 +606,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) mark_reachable_objects(&cb.cmd.revs, 0, 0, NULL); if (flags & EXPIRE_REFLOGS_VERBOSE) putchar('\n'); + } else if (!cb.cmd.expire_total && !cb.cmd.expire_unreachable) { + /* + * If we're not expiring anything and not dropping + * stale entries, there's no point in even opening the + * reflogs, since we're guaranteed to do nothing. + */ + return 0; } if (do_all) { diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh index 42f5ac9ed9..b98827f082 100755 --- a/t/t1410-reflog.sh +++ b/t/t1410-reflog.sh @@ -235,7 +235,9 @@ test_expect_success 'gc.reflogexpire=never' ' test_config gc.reflogexpire never && test_config gc.reflogexpireunreachable never && - git reflog expire --verbose --all && + git reflog expire --verbose --all >output && + test_line_count = 0 output && + git reflog refs/heads/master >output && test_line_count = 4 output '