All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: Jeff King <peff@peff.net>
Cc: Johan Herland <johan@herland.net>, git@vger.kernel.org
Subject: Re: another packed-refs race
Date: Mon, 06 May 2013 14:12:29 +0200	[thread overview]
Message-ID: <51879E2D.8060506@alum.mit.edu> (raw)
In-Reply-To: <20130503172853.GB21715@sigill.intra.peff.net>

On 05/03/2013 07:28 PM, Jeff King wrote:
> On Fri, May 03, 2013 at 11:26:11AM +0200, Johan Herland wrote:
> 
>> You don't really need to be sure that packed-refs is up-to-date. You
>> only need to make sure that don't rely on lazily loading loose refs
>> _after_ you have loaded packed-refs.
> 
> True. As long as you load them both together, and always make sure you
> do loose first, you'd be fine. But I think there will be corner cases
> where you have loaded _part_ of the loose ref namespace. I think part of
> the point of Michael's ref work is that if you call "for_each_tag_ref",
> we would not waste time loading refs/remotes/ at all. If you then follow
> that with a call to "for_each_ref", you would want to re-use the cached
> work from traversing refs/tags/, and then traverse refs/remotes/. You
> know that your cached packed-refs is good with respect to refs/tags/,
> but you don't with respect to refs/remotes.

Correct.

[I wonder if there really are a lot of iterations over overlapping parts
of the reference namespace within a single git process...]

>> The following solution might work in both the resolve-a-single-ref and
>> enumerating-refs case:
>>
>> 0. Look for ref already cached in memory. If found, OK.
>>
>> 1. Look for loose ref. If found, OK.
>>
>> 2. If not found, load all loose refs and packed-refs from disk (in
>> that order), and store in memory for remainder of this process. Never
>> reload packed-refs from disk (unless you also reload all loose refs
>> first).
> 
> I think that would be correct (modulo that step 1 cannot happen for
> enumeration). But we would like to avoid loading all loose refs if we
> can. Especially on a cold cache, it can be quite slow, and you may not
> even care about those refs for the current operation (I do not recall
> the exact original motivation for the lazy loading, but it was something
> along those lines).

Lazy loading was first inspired by the observation that effectively
every git invocation was loading *all* loose references to do an
iteration over refs/replace/ (which I've never even used!)  This was
absolutely killing the performance of filter-branch, which creates a lot
of loose references and invokes git many times--even though the cache
was warm.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

  parent reply	other threads:[~2013-05-06 12:12 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-03  8:38 another packed-refs race Jeff King
2013-05-03  9:26 ` Johan Herland
2013-05-03 17:28   ` Jeff King
2013-05-03 18:26     ` Jeff King
2013-05-03 21:02       ` Johan Herland
2013-05-06 12:12     ` Michael Haggerty [this message]
2013-05-06 18:44       ` Jeff King
2013-05-03 21:21 ` Jeff King
2013-05-06 12:03 ` Michael Haggerty
2013-05-06 18:41   ` Jeff King
2013-05-06 22:18     ` Jeff King
2013-05-07  4:32     ` Michael Haggerty
2013-05-07  4:44       ` Jeff King
2013-05-07  8:03         ` Michael Haggerty
2013-05-07  2:36 ` [PATCH 0/4] fix packed-refs races Jeff King
2013-05-07  2:38   ` [PATCH 1/4] resolve_ref: close race condition for packed refs Jeff King
2013-05-12 22:56     ` Michael Haggerty
2013-05-16  3:47       ` Jeff King
2013-05-16  5:50         ` Michael Haggerty
2013-05-12 23:26     ` Michael Haggerty
2013-06-11 14:26     ` [PATCH 0/4] Fix a race condition when reading loose refs Michael Haggerty
2013-06-11 14:26       ` [PATCH 1/4] resolve_ref_unsafe(): extract function handle_missing_loose_ref() Michael Haggerty
2013-06-11 14:26       ` [PATCH 2/4] resolve_ref_unsafe(): handle the case of an SHA-1 within loop Michael Haggerty
2013-06-11 14:26       ` [PATCH 3/4] resolve_ref_unsafe(): nest reference-reading code in an infinite loop Michael Haggerty
2013-06-11 14:26       ` [PATCH 4/4] resolve_ref_unsafe(): close race condition reading loose refs Michael Haggerty
2013-06-12  8:04         ` Jeff King
2013-06-13  8:22         ` Thomas Rast
2013-06-14  7:17           ` Michael Haggerty
2013-06-11 20:57       ` [PATCH 0/4] Fix a race condition when " Junio C Hamano
2013-05-07  2:39   ` [PATCH 2/4] add a stat_validity struct Jeff King
2013-05-13  2:29     ` Michael Haggerty
2013-05-13  3:00       ` [RFC 0/2] Separate stat_data from cache_entry Michael Haggerty
2013-05-13  3:00         ` [RFC 1/2] Extract a struct " Michael Haggerty
2013-05-13  3:00         ` [RFC 2/2] add a stat_validity struct Michael Haggerty
2013-05-13  5:10         ` [RFC 0/2] Separate stat_data from cache_entry Junio C Hamano
2013-05-16  3:51       ` [PATCH 2/4] add a stat_validity struct Jeff King
2013-05-07  2:43   ` [PATCH 3/4] get_packed_refs: reload packed-refs file when it changes Jeff King
2013-05-07  2:54     ` [PATCH 0/2] peel_ref cleanups changes Jeff King
2013-05-07  2:56       ` [PATCH 1/2] peel_ref: rename "sha1" argument to "peeled" Jeff King
2013-05-07  3:06       ` [PATCH 2/2] peel_ref: refactor for safety with simultaneous update Jeff King
2013-05-09 19:18     ` [PATCH 3/4] get_packed_refs: reload packed-refs file when it changes Eric Sunshine
2013-05-13  2:43     ` Michael Haggerty
2013-05-07  2:51   ` [PATCH 4/4] for_each_ref: load all loose refs before packed refs Jeff King
2013-05-07  6:40   ` [PATCH 0/4] fix packed-refs races Junio C Hamano
2013-05-07 14:19     ` Jeff King

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=51879E2D.8060506@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=git@vger.kernel.org \
    --cc=johan@herland.net \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.