Re: [PATCH 2/2] files-backend: don't rewrite the `packed-refs` file unnecessarily

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:

> Even when we are deleting references, we needn't overwrite the
> `packed-refs` file if the references that we are deleting are not
> present in that file. Implement this optimization as follows:

This goal I can understand.  files-transaction-prepare may see a
deletion and in order to avoid a deletion of a ref from the
file-backend to expose a stale entry in the packed-refs file, it
prepares a transaction to remove the same ref that might exist in
it.  If it turns out that there is no such ref in the packed-refs
file, then we can leave the packed-refs file as-is without
rewriting.

> * Add a function `is_packed_transaction_noop()`, which checks whether
>   a given packed-refs transaction doesn't actually have to do
>   anything. This function must be called while holding the
>   `packed-refs` lock to avoid races.

This checks three things only to cover the most trivial case (I am
perfectly happy that it punts on more complex cases).  If an update

 - checks the old value,

 - sets a new value, or

 - deletes a ref that is not on the filesystem,

it is not a trivial case.  I can understand the latter two (i.e. We
are special casing the deletion, and an update with a new value is
not.  If removing a file from the filesystem is not sufficient to
delete, we may have to rewrite the packed-refs), but not the first
one.  Is it because currently we do not say "delete this ref only
when its current value is X"?

Also it is not immediately obvious how the "is this noop" helper is
checking the absence of the same-named ref in the current
packed-refs file, which is what matters for the correctness of the
optimization.




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux