Re: [PATCH 34/38] refs: add method for delete_refs
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Jun 9, 2016, 15:14 UTC
- Message-ID
- <575987E1.9060200@alum.mit.edu>
- In-Reply-To
- <xmqqporsrj21.fsf@gitster.mtv.corp.google.com>
On 06/07/2016 07:43 PM, Junio C Hamano wrote:
Show 25 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes: > >> From: David Turner <dturner@twopensource.com> >> >> In the file-based backend, delete_refs has some special optimization >> to deal with packed refs. In other backends, we might be able to make >> ref deletion faster by putting all deletions into a single >> transaction. So we need a special backend function for this. >> >> Signed-off-by: David Turner <dturner@twopensource.com> >> Signed-off-by: Junio C Hamano <gitster@pobox.com> >> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> >> --- >> I think that we could get away without this method if we make >> ref_transactions a bit smarter (for example, by supporting best-effort >> updates that can fail without causing the entire transaction to be >> aborted). But that would be a significant detour, so let's leave it >> here for now. > > Hmm, I actually was wondering why 'pack without' was there while > reading 24/38; IIUC, that is one of the "special optimization" that > is very much tied to the files backend, and it may make sense to > hide it behind delete_refs() as its implementation detail. > > Which is exactly what this step is about, so I am happy ;-)
In the future I think this optimization will be even better hidden, namely within the transaction_commit method of a packed_refs_store class :-)
Show 5 quoted lines
> Unlike other changes like the ones that did read_raw_ref(), > verify_refname_available(), etc., the title does not follow the > pattern "refs: make X() virtual", even though as far as I can see > the intent is the same as others. Perhaps a minor retitle is in > order?
OK.
Michael