git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Brandon Casey <drafnel@gmail.com>
Cc: "Ævar Arnfjörð" <avarab@gmail.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH] gc: remove redundant check for gc_auto_threshold
Date: Thu, 11 Oct 2018 08:37:57 +0900	[thread overview]
Message-ID: <xmqqk1mpjru2.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CA+sFfMeOpj4V+KszboVVQSoaW2yDgpfDNGwK9ZrNfGmOOAhPtQ@mail.gmail.com> (Brandon Casey's message of "Wed, 10 Oct 2018 15:29:35 -0700")

Brandon Casey <drafnel@gmail.com> writes:

> ...  Again, I don't feel strongly about it, but I'm not
> sure this change actually improves the code.

Yeah, in the context of the current caller, this is a safe change
that does not break anybody and reduces the number of instructions
executed in this codepath.  A mistaken caller may be added in the
future that fails to check auto-threashold beforehand, but that
won't lead to anything bad like looping for a large number of times,
so as long as the API contract into this helper function is clear
that callers are responsible to check beforehand, it is still not
too bad.

So, I'd throw this into "Meh - I won't regret applying it, but it is
not the end of the world if I forget to apply it, either" pile.

I _think_ a change that actually improves the code would be to
restructure so that there is a helper that is responsible for
guestimating the number of loose objects, and another that uses the
helper to see if there are too many loose objects.  The latter is
the only one tha needs to know about auto-threashold.  But we are
not in immdiate need for such a clean-up, I guess, unless somebody
is actively looking into revamping how auto-gc works and doing a
preparatory clean-up.


  reply	other threads:[~2018-10-10 23:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-10 19:32 [PATCH] gc: remove redundant check for gc_auto_threshold Ævar Arnfjörð Bjarmason
2018-10-10 22:29 ` Brandon Casey
2018-10-10 23:37   ` Junio C Hamano [this message]
2018-10-10 23:53     ` Brandon Casey
2018-10-11  7:49     ` Ævar Arnfjörð Bjarmason
2018-10-13  7:56       ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk1mpjru2.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=drafnel@gmail.com \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).