Re: [jgit-dev] faster ref-updates where the update-type is known to be n

[Shawn Pearce <spearce@xxxxxxxxxxx>]

On Sun, Aug 25, 2013 at 5:00 AM, Roberto Tyley <roberto.tyley@xxxxxxxxx> wrote:
> When using JGit to perform a ref-update that I know is a non-fast-forward
> update, I'd like to be able to tell JGit to not perform the expensive
> revWalk.isMergedInto() calculations used to superfluously determine whether
> the update is in fact fast-forward or not.
>
> Use case: the BFG has rewritten the whole of a big repository's history, and
> needs to update the branch refs. Calling isMergedInto() can take the best
> part of a minute, and in fact it can crash the JVM with a stackoverflow
> (reported by Elliot Glaysher when using the BFG on Chromium's source).

A stack overflow during a revwalk is bad. We shouldn't do this. *sigh*

> There are two places where revWalk.isMergedInto() is called during a
> batch-ref-update:
>
> https://github.com/eclipse/jgit/blob/9b26e4bff/org.eclipse.jgit/src/org/eclipse/jgit/transport/ReceiveCommand.java#L292-L299
> https://github.com/eclipse/jgit/blob/9b26e4bff/org.eclipse.jgit/src/org/eclipse/jgit/lib/RefUpdate.java#L624
>
> I couldn't figure out a good way (using the current JGit API) to avoid these
> calls getting made (or to maybe tell JGit to not search so deep?), and
> eventually used the workaround of sub-classing RevWalk and overriding
> isMergedInto() with a short-circuit function:
>
> https://github.com/rtyley/bfg-repo-cleaner/commit/f7a8c675#diff-0
>
> This felt like a bit of a hack - does anyone have any better solutions?
>
> Although this isn't current behaviour, perhaps setting the ReceiveCommand
> update-type to UPDATE_NONFASTFORWARD before calling BatchRefUpdate.execute()
> should be enough to tell JGit that I know the update type, and don't want
> JGit to calculate it?

There isn't a clean way to do this sort of update, but there should
be. It is completely reasonable for a calling application to say it
has already vetted the update and just wants it to proceed, provided
the reference still exactly matches the expected old SHA-1 (if
supplied).

I thought it bypasses if the update-type is already set to
UPDATE_NONFASTFORWARD as you suggest, as we only run the check when
the type is the default of UPDATE.