Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On Tue, 19 Feb 2013, Junio C Hamano wrote: Assuming that this says yes: D=/afs/athena.mit.edu/user/a/n/andersk/my/dir cd $D test $(/bin/pwd) = $D echo yes Correct. Perhaps existing of an empty element in the list would do? E.g. GIT_CEILING_DIRECTORIES=:/afs/athena.mit.edu/users/a/n/andesk or something like that. And in such a case, we do not run realpath on the elements on the list before comparing them with what we get from getcwd(3). That seems reasonable, and has the advantage of backwards compatibility with versions before 1.8.1.2, I guess. Anders -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On 10/29/2012 01:10 AM, Michael Haggerty wrote: How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a slowdown? Sorry to bring up this old thread again, but I just realized why my computer has been acting so slow when I’m not connected to the network. I put various network filesystem paths in $GIT_CEILING_DIRECTORIES, such as /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and /afs/athena.mit.edu/user/a/n which all live in different AFS volumes). Now when I’m not connected to the network, every invocation of Git, including the __git_ps1 in my shell prompt, waits for AFS to timeout. Obviously I’m going to stop using $GIT_CEILING_DIRECTORIES now that I know what the problem is, but I figured you might want to know why this feature is now useless for me. Anders -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
Anders Kaseorg ande...@mit.edu writes: On 10/29/2012 01:10 AM, Michael Haggerty wrote: How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a slowdown? Sorry to bring up this old thread again, but I just realized why my computer has been acting so slow when I’m not connected to the network. I put various network filesystem paths in $GIT_CEILING_DIRECTORIES, such as /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and /afs/athena.mit.edu/user/a/n which all live in different AFS volumes). Now when I’m not connected to the network, every invocation of Git, including the __git_ps1 in my shell prompt, waits for AFS to timeout. Thanks for a report. Assuming that this says yes: D=/afs/athena.mit.edu/user/a/n/andersk/my/dir cd $D test $(/bin/pwd) = $D echo yes iow, AFS mounting does not have funny symbolic link issues that make the logical and physical PWD to be different like some automounter implementation used to have, perhaps we can introduce a way for the user to say The element of this CEILING list do not have any alias due to funny symlinks to solve this. Perhaps existing of an empty element in the list would do? E.g. GIT_CEILING_DIRECTORIES=:/afs/athena.mit.edu/users/a/n/andesk or something like that. And in such a case, we do not run realpath on the elements on the list before comparing them with what we get from getcwd(3). Of course, you could condionally unset the environment while offline, but that feels like an ugly hack. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On 11/13/2012 09:50 PM, David Aguilar wrote: On Mon, Nov 12, 2012 at 9:47 AM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: The log message of the original commit (0454dd93bf) described the following scenario: a /home partition under which user home directories are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid hitting /home/.git, /home/.git/objects, and /home/objects (which would attempt to automount those directories). I believe that this scenario would not be slowed down by my patches. How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a slowdown? Yeah, I was also wondering about that. David? I double-checked our configuration and all the parent directories of those listed in GIT_CEILING_DIRECTORIES are local, so our particular configuration would not have a performance hit. We do have multiple directories listed there. Some of them share a parent directory. I'm assuming the implementation is simple and does not try and avoid repeating the check when the parent dir is the same across multiple entries. In any case, it won't be a problem in practice based on my reading of the current code. OK, so we're back to the following status: some people (including me) are nervous that this change could cause a performance regression, though it seems that the most sensible ways of using the GIT_CEILING_DIRECTORIES feature would not be affected. In favor: Currently, if a directory containing a symlink is added to GIT_CEILING_DIRECTORIES, then GIT_CEILING_DIRECTORIES will not work, git has no way of recognizing that there is a problem, and the only symptom observable by the user is that the hoped-for performance improvement from using GIT_CEILING_DIRECTORIES will not materialize (or will disappear after a filesystem reorg) [1]. Against: The change will cause a performance regression if a slow-to-stat directory is listed in GIT_CEILING_DIRECTORIES. The slowdown will occur whenever git is run outside of a true git-managed project, most nastily in the case of using __git_ps1 in a shell prompt. I don't have a preference either way about whether these patches should be merged. Michael [1] It is also conceivable that GIT_CEILING_DIRECTORIES is being used to *hide* an enclosing git project rather than to inform git that there are no enclosing projects, in which case the enclosing project would *not* be hidden. This is in fact the mechanism by which the problem causes failures in our test suite. But I don't expect that this is a common real-world scenario, and anyway such a failure would be obvious to the user and quickly fixed. -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On Mon, Nov 12, 2012 at 9:47 AM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: The log message of the original commit (0454dd93bf) described the following scenario: a /home partition under which user home directories are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid hitting /home/.git, /home/.git/objects, and /home/objects (which would attempt to automount those directories). I believe that this scenario would not be slowed down by my patches. How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a slowdown? Yeah, I was also wondering about that. David? I double-checked our configuration and all the parent directories of those listed in GIT_CEILING_DIRECTORIES are local, so our particular configuration would not have a performance hit. We do have multiple directories listed there. Some of them share a parent directory. I'm assuming the implementation is simple and does not try and avoid repeating the check when the parent dir is the same across multiple entries. In any case, it won't be a problem in practice based on my reading of the current code. Is there another way to accomplish this without the performance hit? Maybe something that can be solved with configuration? Without doing the symlink expansion there is no way for git to detect that GIT_CEILING_DIRECTORIES contains symlinks and is therefore ineffective. So the user has no warning about the misconfiguration (except that git runs slowly). On 10/29/2012 02:42 AM, Junio C Hamano wrote: Perhaps not canonicalize elements on the CEILING list ourselves? If we make it a user error to put symlinked alias in the variable, and document it clearly, wouldn't it suffice? There may be no other choice. (That, and fix the test suite in another way to tolerate a $PWD that involves symlinks.) -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
Michael Haggerty mhag...@alum.mit.edu writes: The log message of the original commit (0454dd93bf) described the following scenario: a /home partition under which user home directories are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid hitting /home/.git, /home/.git/objects, and /home/objects (which would attempt to automount those directories). I believe that this scenario would not be slowed down by my patches. How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a slowdown? Yeah, I was also wondering about that. David? Is there another way to accomplish this without the performance hit? Maybe something that can be solved with configuration? Without doing the symlink expansion there is no way for git to detect that GIT_CEILING_DIRECTORIES contains symlinks and is therefore ineffective. So the user has no warning about the misconfiguration (except that git runs slowly). On 10/29/2012 02:42 AM, Junio C Hamano wrote: Perhaps not canonicalize elements on the CEILING list ourselves? If we make it a user error to put symlinked alias in the variable, and document it clearly, wouldn't it suffice? There may be no other choice. (That, and fix the test suite in another way to tolerate a $PWD that involves symlinks.) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On Sat, Oct 20, 2012 at 11:51 PM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: This patch series has the side effect that all of the directories listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to resolve any symlinks that are present in their paths. It is admittedly odd that a feature intended to avoid accessing expensive directories would now *intentionally* access directories near the expensive ones. In the above scenario this shouldn't be a problem, because /home would be the directory listed in GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be expensive. Interesting observation. In the last sentence, accessing /home does not exactly mean accessing /home, but accessing / to learn about home in it, no? But there might be other scenarios for which this patch series causes a performance regression. Yeah, after merging this to 'next', we should ask people who care about CEILING to test it sufficiently. Thanks for rerolling. GIT_CEILING_DIRECTORIES was always about trying to avoid hitting them at all; they can be (busy) NFS volumes there. Here's the description from the 1.6.0 release notes: * A new environment variable GIT_CEILING_DIRECTORIES can be used to stop the discovery process of the toplevel of working tree; this may be useful when you are working in a slow network disk and are outside any working tree, as bash-completion and git help may still need to run in these places. In 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added GIT_ONE_FILESYSTEM to fix a related issue. Do you guys have GIT_CEILING_DIRECTORIES set too? We use GIT_CEILING_DIRECTORIES and I'm pretty sure we don't want every git command hitting them, so this would be a regression when seen from the POV of our current usage of this variable, which would be a bummer. Is there another way to accomplish this without the performance hit? Maybe something that can be solved with configuration? I'd be happy to lend a hand if you guys have some ideas on how we can help keep it fast. Thoughts? Original patches for those just joining us: http://thread.gmane.org/gmane.comp.version-control.git/208102 -- David -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
David Aguilar dav...@gmail.com wrote: Is there another way to accomplish this without the performance hit? Perhaps not canonicalize elements on the CEILING list ourselves? If we make it a user error to put symlinked alias in the variable, and document it clearly, wouldn't it suffice? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On 10/29/2012 01:15 AM, David Aguilar wrote: On Sat, Oct 20, 2012 at 11:51 PM, Junio C Hamano gits...@pobox.com wrote: Michael Haggerty mhag...@alum.mit.edu writes: This patch series has the side effect that all of the directories listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to resolve any symlinks that are present in their paths. It is admittedly odd that a feature intended to avoid accessing expensive directories would now *intentionally* access directories near the expensive ones. In the above scenario this shouldn't be a problem, because /home would be the directory listed in GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be expensive. Interesting observation. In the last sentence, accessing /home does not exactly mean accessing /home, but accessing / to learn about home in it, no? But there might be other scenarios for which this patch series causes a performance regression. Yeah, after merging this to 'next', we should ask people who care about CEILING to test it sufficiently. Thanks for rerolling. GIT_CEILING_DIRECTORIES was always about trying to avoid hitting them at all; they can be (busy) NFS volumes there. Here's the description from the 1.6.0 release notes: * A new environment variable GIT_CEILING_DIRECTORIES can be used to stop the discovery process of the toplevel of working tree; this may be useful when you are working in a slow network disk and are outside any working tree, as bash-completion and git help may still need to run in these places. In 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added GIT_ONE_FILESYSTEM to fix a related issue. Do you guys have GIT_CEILING_DIRECTORIES set too? We use GIT_CEILING_DIRECTORIES and I'm pretty sure we don't want every git command hitting them, so this would be a regression when seen from the POV of our current usage of this variable, which would be a bummer. I would certainly withdraw the patch series if it causes a performance hit. The log message of the original commit (0454dd93bf) described the following scenario: a /home partition under which user home directories are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid hitting /home/.git, /home/.git/objects, and /home/objects (which would attempt to automount those directories). I believe that this scenario would not be slowed down by my patches. How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a slowdown? Is there another way to accomplish this without the performance hit? Maybe something that can be solved with configuration? Without doing the symlink expansion there is no way for git to detect that GIT_CEILING_DIRECTORIES contains symlinks and is therefore ineffective. So the user has no warning about the misconfiguration (except that git runs slowly). On 10/29/2012 02:42 AM, Junio C Hamano wrote: Perhaps not canonicalize elements on the CEILING list ourselves? If we make it a user error to put symlinked alias in the variable, and document it clearly, wouldn't it suffice? There may be no other choice. (That, and fix the test suite in another way to tolerate a $PWD that involves symlinks.) Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
On 10/21/2012 08:51 AM, Junio C Hamano wrote:
Michael Haggerty mhag...@alum.mit.edu writes:
This patch series has the side effect that all of the directories
listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to
resolve any symlinks that are present in their paths. It is
admittedly odd that a feature intended to avoid accessing expensive
directories would now *intentionally* access directories near the
expensive ones. In the above scenario this shouldn't be a problem,
because /home would be the directory listed in
GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be
expensive.
Interesting observation. In the last sentence, accessing /home
does not exactly mean accessing /home, but accessing / to learn
about home in it, no?
This is the extra overhead on my system for using
GIT_CEILING_DIRECTORIES=/home:
stat(/home, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
getcwd(/home/mhagger, 1024) = 14
chdir(/home) = 0
getcwd(/home, 4096) = 6
lstat(/home, {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
chdir(/home/mhagger) = 0
If I use GIT_CEILING_DIRECTORIES=/dev/shm, which is a symlink to
/run/shm on my system, the overhead is comparable:
stat(/dev/shm, {st_mode=S_IFDIR|S_ISVTX|0777, st_size=200, ...}) = 0
getcwd(/home/mhagger, 1024) = 14
chdir(/dev/shm) = 0
getcwd(/run/shm, 4096)= 9
lstat(/run/shm, {st_mode=S_IFDIR|S_ISVTX|0777, st_size=200, ...}) = 0
chdir(/home/mhagger) = 0
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
Michael Haggerty mhag...@alum.mit.edu writes: This patch series has the side effect that all of the directories listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to resolve any symlinks that are present in their paths. It is admittedly odd that a feature intended to avoid accessing expensive directories would now *intentionally* access directories near the expensive ones. In the above scenario this shouldn't be a problem, because /home would be the directory listed in GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be expensive. Interesting observation. In the last sentence, accessing /home does not exactly mean accessing /home, but accessing / to learn about home in it, no? But there might be other scenarios for which this patch series causes a performance regression. Yeah, after merging this to 'next', we should ask people who care about CEILING to test it sufficiently. Thanks for rerolling. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
