Re: [PATCH v2] refs: implement reference transaction hook

From: Taylor Blau <me@ttaylorr.com>
Date: Jun 3, 2020, 16:51 UTC
Message-ID: <20200603165142.GA24049@syl.local>
In-Reply-To: <04116cc57ab37eeb50bd51a065a7c06503493bf3.1591186875.git.ps@pks.im>

Hi Patrick,

On Wed, Jun 03, 2020 at 02:27:50PM +0200, Patrick Steinhardt wrote:

Show 13 quoted lines

> In order to test the impact on the case where we don't have any
> "reference-transaction" hook installed in the repository, this commit
> introduces a new performance test for git-update-refs(1). Run against an
> empty repository, it produces the following results:
>
>   Test                                   HEAD~             HEAD
>   ------------------------------------------------------------------------------
>   1400.2: update existing reference      2.05(1.58+0.54)   2.08(1.58+0.57) +1.5%
>   1400.3: create and destroy reference   1.79(1.38+0.49)   1.82(1.39+0.51) +1.7%
>
> So the overhead is around ~1.5%. Given that git-update-refs(1) is a
> near-direct wrapper around reference transactions, there likely is no
> other command that is impacted much worse than this.

This is a serious performance regression that is worth considering. For example, a 1.5% slow-down on average in reference transactions would cause be enough for me to bisect the regression down to see what changed.

What are ways that this could be avoided? I bet that this would work quite well with Emily's work on hooks, where we could check in the configuration first whether a hook is even configured.

Could this be integrated with that? If not, could you cache the result of whether or not the hook exists, and/or implement some mechanism to say something like, for eg., "only run reference transaction hooks core.blah = 1" is true?

I haven't myself looked seriously at your patch (although I do plan on doing so, but haven't yet had time), but I think that this should be considered thoroughly before moving forward.

Thanks, Taylor