Re: [PATCHv2 5/5] refs.c: enable large transactions

[Michael Haggerty <mhagger@alum.mit.edu>]

On 01/22/2015 02:10 PM, Jeff King wrote:
> On Thu, Jan 22, 2015 at 12:24:23PM +0100, Michael Haggerty wrote:
> 
>> I can't figure out where to apply this series or where to fetch it from,
>> so I can't see these changes in context, so maybe I'm misunderstanding
>> something. It looks like this code is doing
>>
>>     open(), close(), open(), fdopen(), write(), fclose(), rename()
>>
>> on each lockfile. But don't we have enough information to write the
>> SHA-1 into the lockfile the first time we touch it? I.e., couldn't we
>> reduce this to
>>
>>     open(), fdopen(), write(), fclose(), rename()
>>
>> , where the first four calls all happen in the initial loop? If a
>> problem is discovered when writing a later reference, we would roll back
>> the transaction anyway.
>>
>> I understand that this would require a bigger rewrite, so maybe it is
>> not worth it.
> 
> I had a nagging feeling on the multiple-open thing, too, and would much
> prefer to just write out the contents early (since we know what they
> are). It looks like we would just need to split write_ref_sha1() into
> its two halves:
> 
>   1. Write out the lockfile
> 
>   2. Commit the change
> 
> And then call them at the appropriate spots from ref_transaction_commit().
> 
> I guess that is maybe a step backwards for abstracted ref backends,
> though.

Nah, the implementation of ref_transaction_commit() will have to differ
between backends anyway. I don't think this would be a step backwards.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu