From: Junio C Hamano <gitster@pobox•com>
To: Ronnie Sahlberg <sahlberg@google•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one
Date: Thu, 10 Apr 2014 12:01:23 -0700 [thread overview]
Message-ID: <xmqqy4zdvwjg.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1397154625-11884-5-git-send-email-sahlberg@google.com> (Ronnie Sahlberg's message of "Thu, 10 Apr 2014 11:30:25 -0700")
Ronnie Sahlberg <sahlberg@google•com> writes:
> We want to make sure that we update all refs before we delete any refs
> so that there is no point in time where the tips may not be reachable
> from any ref in the system.
> We currently acheive this by first looping over all updates and applying
> them before we run a second loop and perform all the deletes.
>
> If we sort the new sha1 for all the refs so that a deleted ref,
> with sha1 0{40} comes at the end of the array, then we can just run
> a single loop and still guarantee that all updates happen before
> any deletes.
>
> Signed-off-by: Ronnie Sahlberg <sahlberg@google•com>
> ---
> refs.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 1678e12..453318e 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3309,6 +3309,15 @@ static int ref_update_compare(const void *r1, const void *r2)
> return strcmp((*u1)->ref_name, (*u2)->ref_name);
> }
>
> +static int ref_delete_compare(const void *r1, const void *r2)
> +{
> + const struct ref_update * const *u1 = r1;
> + const struct ref_update * const *u2 = r2;
> +
> + /* -strcmp so that 0{40} sorts to the end */
> + return -strcmp((*u1)->new_sha1, (*u2)->new_sha1);
Two glitches:
- Didn't you mean hashcmp()?
- Don't you have an explicit ->delete_ref field that is a more
direct way, than relying on the convention "updating to 0{40}
is to delte", to express this?
In other words, wouldn't it be more like
return !(*u1)->delete_ref - !(*u2)->delete_ref;
or something (I may be wrong about the sign, tho---I didn't think
carefully)?
> +}
> +
> static int ref_update_reject_duplicates(struct ref_update **updates, int n,
> enum action_on_err onerr)
> {
> @@ -3388,13 +3397,8 @@ int update_refs(const char *action, const struct ref_update **updates_orig,
> unlink_or_warn(git_path("logs/%s", delnames[i]));
> clear_loose_ref_cache(&ref_cache);
>
> - /* Perform updates first so live commits remain referenced */
> - for (i = 0; i < n; i++)
> - if (locks[i] && !locks[i]->delete_ref) {
> - ret |= commit_ref_lock(locks[i]);
> - locks[i] = NULL;
> - }
> - /* And finally perform all deletes */
> + /* Sort the array so that we perform all updates before any deletes */
> + qsort(updates, n, sizeof(*updates), ref_delete_compare);
> for (i = 0; i < n; i++)
> if (locks[i]) {
> ret |= commit_ref_lock(locks[i]);
next prev parent reply other threads:[~2014-04-10 19:01 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-10 18:30 [PATCH 0/4] Make update_refs more atomic V2 Ronnie Sahlberg
2014-04-10 18:30 ` [PATCH 1/4] refs.c: split writing and commiting a ref into two separate functions Ronnie Sahlberg
2014-04-10 18:30 ` [PATCH 2/4] refs.c: split delete_ref_loose() into a separate flag-for-deletion and commit phase Ronnie Sahlberg
2014-04-10 18:30 ` [PATCH 3/4] refs.c: change update_refs to run the commit loops once all work is finished Ronnie Sahlberg
2014-04-10 18:30 ` [PATCH 4/4] refs.c: sort the refs by new_sha1 and merge the two update/delete loops into one Ronnie Sahlberg
2014-04-10 19:01 ` Junio C Hamano [this message]
2014-04-10 19:40 ` Ronnie Sahlberg
2014-04-10 18:53 ` [PATCH 0/4] Make update_refs more atomic V2 Junio C Hamano
2014-04-10 20:08 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqy4zdvwjg.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=sahlberg@google$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox