From: Heiko Voigt <hvoigt@hvoigt.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, Jeff King <peff@peff.net>,
Drew Northup <drew.northup@maine.edu>,
Jakub Narebski <jnareb@gmail.com>
Subject: Re: Re: [PATCH 6/6] Retain caches of submodule refs
Date: Mon, 10 Oct 2011 21:53:26 +0200 [thread overview]
Message-ID: <20111010195325.GA5981@sandbox-rc> (raw)
In-Reply-To: <4E918194.5060102@alum.mit.edu>
Hi Michael,
On Sun, Oct 09, 2011 at 01:12:20PM +0200, Michael Haggerty wrote:
> On 08/17/2011 12:45 AM, Junio C Hamano wrote:
> > All the changes except for this one made sense to me, but I am not sure
> > about this one. How often do we look into different submodule refs in the
> > same process over and over again?
>
> I am having pangs of uncertainty about this patch.
Currently when doing a 3 way merge of submodule hashes and we find a
conflict. We then use this api to look into the submodule to search for
commit suggestions which contain both sides.
If there are multiple submodules that conflict in this way we look into
those as well.
Since the setup_revision() api can currently not be used to safely
iterate twice over the same submodule my patch
allow multiple calls to submodule merge search for the same path
rewrites the search into using a child process. AFAIK the submodule ref
iteration api would then even be unused.
> Previous to this patch, the submodule reference cache was only used for
> the duration of one call to do_for_each_ref(). (It was not *discarded*
> until later, but the old cache was never reused.) Therefore, the
> submodule reference cache was implicitly invalidated between successive
> uses.
The implicit discarding was just done because it was the quickest way to
get a handle on submodule refs from the main process. There was no need
that they get reloaded every time.
> After this change, submodule ref caches are invalidated whenever
> invalidate_cached_refs() is called. But this function is static, and it
> is only called when main-module refs are changed.
>
> AFAIK there is no way within refs.c to add, modify, or delete a
> submodule reference. But if other code modifies submodule references
> directly, then the submodule ref cache in refs.c would become stale.
> Moreover, there is currently no API for invalidating the cache.
>
> So I think I need help from a submodule guru (Heiko?) who can tell me
> what is done with submodule references and whether they might be
> modified while a git process is executing in the main module. If so,
> then either this patch has to be withdrawn, or more work has to be put
> in to make such code invalidate the submodule reference cache.
At least in my code there is no place where a submodule ref is changed.
I only used it for merging submodule which only modifies the main
module. So I would say its currently safe to assume that submodule refs
do not get modified. If we do need that later on we can still add
invalidation for submodule refs.
Cheers Heiko
next prev parent reply other threads:[~2011-10-10 19:53 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-12 22:36 [PATCH 0/6] Retain caches of submodule refs Michael Haggerty
2011-08-12 22:36 ` [PATCH 1/6] Extract a function clear_cached_refs() Michael Haggerty
2011-08-12 22:36 ` [PATCH 2/6] Access reference caches only through new function get_cached_refs() Michael Haggerty
2011-08-14 22:12 ` Junio C Hamano
2011-08-23 4:21 ` Michael Haggerty
2011-08-12 22:36 ` [PATCH 3/6] Change the signature of read_packed_refs() Michael Haggerty
2011-08-12 22:36 ` [PATCH 4/6] Allocate cached_refs objects dynamically Michael Haggerty
2011-08-14 22:21 ` Junio C Hamano
2011-08-12 22:36 ` [PATCH 5/6] Store the submodule name in struct cached_refs Michael Haggerty
2011-08-12 22:36 ` [PATCH 6/6] Retain caches of submodule refs Michael Haggerty
2011-08-13 12:54 ` Heiko Voigt
2011-08-24 8:17 ` Michael Haggerty
2011-08-24 20:05 ` Heiko Voigt
2011-08-16 22:45 ` Junio C Hamano
2011-08-24 11:33 ` Michael Haggerty
2011-10-09 11:12 ` Michael Haggerty
2011-10-09 20:10 ` Junio C Hamano
2011-10-10 5:46 ` [PATCH 0/2] Provide API to invalidate refs cache Michael Haggerty
2011-10-10 5:46 ` [PATCH 1/2] invalidate_cached_refs(): take the submodule as parameter Michael Haggerty
2011-10-10 5:46 ` [PATCH 2/2] invalidate_cached_refs(): expose this function in refs API Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 0/7] Provide API to invalidate refs cache Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs() Michael Haggerty
2011-10-11 0:00 ` Junio C Hamano
2011-10-11 5:53 ` Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 2/7] invalidate_ref_cache(): take the submodule as parameter Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 3/7] invalidate_ref_cache(): expose this function in refs API Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 4/7] clear_cached_refs(): rename parameter Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 5/7] clear_cached_refs(): extract two new functions Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 6/7] write_ref_sha1(): only invalidate the loose ref cache Michael Haggerty
2011-10-10 8:24 ` [PATCH v2 7/7] clear_cached_refs(): inline function Michael Haggerty
2011-10-11 0:02 ` [PATCH v2 0/7] Provide API to invalidate refs cache Junio C Hamano
2011-10-11 5:50 ` Michael Haggerty
2011-10-11 8:09 ` Julian Phillips
2011-10-11 17:26 ` Junio C Hamano
2011-10-12 18:44 ` [PATCH v3 " Michael Haggerty
2011-10-12 18:44 ` [PATCH v3 1/7] invalidate_ref_cache(): rename function from invalidate_cached_refs() Michael Haggerty
2011-10-12 19:14 ` Junio C Hamano
2011-10-12 22:12 ` Michael Haggerty
2011-10-12 18:44 ` [PATCH v3 2/7] invalidate_ref_cache(): take the submodule as parameter Michael Haggerty
2011-10-12 19:19 ` Junio C Hamano
2011-10-12 22:07 ` Michael Haggerty
2011-10-17 18:00 ` Junio C Hamano
2011-11-03 10:23 ` Michael Haggerty
2011-11-03 18:57 ` Junio C Hamano
2011-10-12 18:44 ` [PATCH v3 3/7] invalidate_ref_cache(): expose this function in refs API Michael Haggerty
2011-10-12 18:44 ` [PATCH v3 4/7] clear_cached_refs(): rename parameter Michael Haggerty
2011-10-12 18:44 ` [PATCH v3 5/7] clear_cached_refs(): extract two new functions Michael Haggerty
2011-10-12 18:44 ` [PATCH v3 6/7] write_ref_sha1(): only invalidate the loose ref cache Michael Haggerty
2011-10-12 18:44 ` [PATCH v3 7/7] clear_cached_refs(): inline function Michael Haggerty
2011-10-12 19:28 ` [PATCH v3 0/7] Provide API to invalidate refs cache Junio C Hamano
2011-10-10 19:53 ` Heiko Voigt [this message]
2011-10-11 4:12 ` [PATCH 6/6] Retain caches of submodule refs Michael Haggerty
2011-10-11 17:41 ` Heiko Voigt
2011-08-13 12:34 ` [PATCH 0/6] " Heiko Voigt
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=20111010195325.GA5981@sandbox-rc \
--to=hvoigt@hvoigt.net \
--cc=drew.northup@maine.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jnareb@gmail.com \
--cc=mhagger@alum.mit.edu \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).