6 messages, 3 authors, 2016-06-15 · open the first message on its own page
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:49
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?
From: Michael Haggerty <hidden>
Date: 2016-06-15 22:51:52
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?
As I've mentioned, I am not very familiar with submodules and I don't
know what actions in submodules can be triggered from the top-level
project. Here is the only code path that I can find in the current git
code that causes submodule refs to be read at all:
setup_revisions() with opt->submodule is set,
which can only happen when called via merge_submodule(),
which is called from merge_file() in merge-recursive.c,
which is called by process_renames() and merge_content(),
which are both ultimately called from merge_trees().
I don't know how often this can happen during the lifetime of one process.
The cost of retaining the submodule ref caches is roughly 100 bytes per
reference of memory. Another side effect is that the submodule caches
never have to be cleaned up; this saves a list walk and one free() per
cached reference if the refs for more than one submodule are accessed.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
From: Michael Haggerty <hidden>
Date: 2016-06-15 22:52:13
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. 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. 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. Sorry for the oversight, but I forgot that not all code necessarily uses the refs.c API when dealing with references (a regrettable situation, BTW). Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/
From: Heiko Voigt <hidden>
Date: 2016-06-15 22:52:13
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:quoted
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
From: Michael Haggerty <hidden>
Date: 2016-06-15 22:52:13
On 10/10/2011 09:53 PM, Heiko Voigt wrote:
On Sun, Oct 09, 2011 at 01:12:20PM +0200, Michael Haggerty wrote: 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.
If your patch is accepted, then we should check whether anything should be ripped out.
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.
OK, thanks! Michael -- Michael Haggerty mhagger@alum.mit.edu http://softwareswirl.blogspot.com/
From: Heiko Voigt <hidden>
Date: 2016-06-15 22:52:13
Hi, On Tue, Oct 11, 2011 at 06:12:34AM +0200, Michael Haggerty wrote:
On 10/10/2011 09:53 PM, Heiko Voigt wrote:quoted
On Sun, Oct 09, 2011 at 01:12:20PM +0200, Michael Haggerty wrote: 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.If your patch is accepted, then we should check whether anything should be ripped out.
I would rather like to extend the setup_revision() api so that we can iterate multiple times. Additionally I have a feeling that this API might be useful for further extensions of the recursive-push support of submodules which is currently under development. So in summary: I would like to wait with ripping anything out until we get to a final state with submodule support. Cheers Heiko