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

[Taylor Blau <me@ttaylorr.com>]

Hi Patrick,

On Wed, Jun 03, 2020 at 02:27:50PM +0200, Patrick Steinhardt wrote:
> 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