[PATCH] rebase: move actual rebase into a single transaction

[Durham Goode]

On 3/7/17 10:08 AM, Augie Fackler wrote:
> On Sat, Mar 04, 2017 at 02:08:26PM -0800, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1488665211 28800
>> #      Sat Mar 04 14:06:51 2017 -0800
>> # Node ID 9c3ea2112952f398aaa7625f43dcfa36cfd34379
>> # Parent  b4cd912d7704cd976e1bee3a3c927e0e578ec88f
>> rebase: move actual rebase into a single transaction
>>
>> Previously, rebasing would open several transaction over the course of rebasing
>> several commits. Opening a transaction can have notable overhead (like copying
>> the dirstate) which can add up when rebasing many commits.
>>
>> This patch adds a single large transaction around the actual commit rebase
>> operation, with a catch for intervention which serializes the current state if
>> we need to drop back to the terminal for user intervention. Amazingly, almost
>> all the tests seem to pass.
>>
>> On large repos with large working copies, this can speed up rebasing 7 commits
>> by 25%. I'd expect the percentage to be a bit larger for rebasing even more
>> commits.
>
> I like it, seems like the correct thing to do in any case. However...
>
> [...]
>
>> diff --git a/tests/test-rebase-abort.t b/tests/test-rebase-abort.t
>> --- a/tests/test-rebase-abort.t
>> +++ b/tests/test-rebase-abort.t
>>
> [...]
>
>> @@ -398,7 +399,7 @@ test aborting an interrupted series (iss
>>    parent: 0:df4f53cec30a
>>     base
>>    branch: default
>> -  commit: (clean)
>> +  commit: 1 unknown (interrupted update)
>>    update: 6 new changesets (update)
>>    phases: 7 draft
>
> What's up with this? Shouldn't rebase --abort put us back in a clean state?

This is actually a bug in normal rebase.  If the rebase aborts while the 
working copy is on the commit that it started on, then it doesn't need 
to perform any update during rebase --abort, which means it doesn't 
clean up the .hg/updatestate file. The single transaction makes this 
more obvious since when the whole transaction fails, it dumps everything 
back to the way things were (i.e. you're back on the initial commit), 
but leaves the working copy dirty.  I guess we could fix this by also 
checking if status is clean when deciding whether to perform an update 
during rebase --abort.  Or maybe we just run the update every time, 
since it's extremely likely to be needed.

This raises another issue though, that the rebase state file is not 
managed by the transaction, so it is being written mid transaction and 
doesn't get rolled back appropriately.  I'll look into fixing both of 
these and resending.