This seems to be because I'm now on 'pu' as of a day or two ago in
order to test the abbrev logic, but lookie here:
time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
.. shows all the branches and tags ..
real 0m0.655s
user 0m0.011s
sys 0m0.004s
so the remote is fast to connect to, and with network connection
overhead and everything, it's just over half a second. But then:
time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
and it just sits there, and it's at 100% CPU the whole time, until it says:
Everything up-to-date
real 1m7.307s
user 1m2.761s
sys 0m0.475s
Whaa? It took a *minute* of CPU time to decide that everything was up-to-date?
That's just not right. The branch is entirely up-to-date:
git rev-parse HEAD
af79ad2b1f337a00aa150b993635b10bc68dc842
git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux master
af79ad2b1f337a00aa150b993635b10bc68dc842 refs/heads/master
so there should be no need for any history walking. But it sure is
doing *something*. A minute of CPU time on my machine is actually a
pretty damn big deal.
Looking at the trace, there's no IO - there's no back-and-forth about
"I have this, do you have it?" or anything like that. The system call
trace is just a lot of allocations, which I think means that "git
push" is walking a lot of objects but not doing anything useful.
I bisected it to commit 60cd66f "push: change submodule default to
check", which makes little sense since I have no submodules, but there
you go.. Apparently RECURSE_SUBMODULES_CHECK is just terminally
broken.
Linus
From: Stefan Beller <hidden> Date: 2016-10-03 21:17:54
On Mon, Oct 3, 2016 at 2:11 PM, Linus Torvalds
[off-list ref] wrote:
This seems to be because I'm now on 'pu' as of a day or two ago in
order to test the abbrev logic, but lookie here:
time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
.. shows all the branches and tags ..
real 0m0.655s
user 0m0.011s
sys 0m0.004s
so the remote is fast to connect to, and with network connection
overhead and everything, it's just over half a second. But then:
time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
and it just sits there, and it's at 100% CPU the whole time, until it says:
Everything up-to-date
real 1m7.307s
user 1m2.761s
sys 0m0.475s
Whaa? It took a *minute* of CPU time to decide that everything was up-to-date?
That's just not right. The branch is entirely up-to-date:
git rev-parse HEAD
af79ad2b1f337a00aa150b993635b10bc68dc842
git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux master
af79ad2b1f337a00aa150b993635b10bc68dc842 refs/heads/master
so there should be no need for any history walking. But it sure is
doing *something*. A minute of CPU time on my machine is actually a
pretty damn big deal.
Looking at the trace, there's no IO - there's no back-and-forth about
"I have this, do you have it?" or anything like that. The system call
trace is just a lot of allocations, which I think means that "git
push" is walking a lot of objects but not doing anything useful.
I bisected it to commit 60cd66f "push: change submodule default to
check", which makes little sense since I have no submodules, but there
you go.. Apparently RECURSE_SUBMODULES_CHECK is just terminally
broken.
Sorry for breaking you, too.
Junio complained about that when I was proposing the topic; but now
the strategy seems to just wait until Heiko fixed the
RECURSE_SUBMODULES_CHECK to be less broken.
* sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
- push: change submodule default to check
Turn the default of "push.recurseSubmodules" to "check".
Will hold to wait for hv/submodule-not-yet-pushed-fix
This reveals that the "check" mode is too inefficient to use in
real projects, even in ones as small as git itself.
cf. [off-list ref]
Which itself is
* hv/submodule-not-yet-pushed-fix (2016-09-14) 2 commits
- serialize collection of refs that contain submodule changes
- serialize collection of changed submodules
The code in "git push" to compute if any commit being pushed in the
superproject binds a commit in a submodule that hasn't been pushed
out was overly inefficient, making it unusable even for a small
project that does not have any submodule but have a reasonable
number of refs.
The last two in the original series seem to break a few tests when
queued to 'pu', and dropped for now.
Waiting for a reroll.
From: Stefan Beller <hidden> Date: 2016-10-03 21:23:30
+cc Heiko
On Mon, Oct 3, 2016 at 2:17 PM, Stefan Beller [off-list ref] wrote:
* sb/push-make-submodule-check-the-default (2016-08-24) 1 commit
- push: change submodule default to check
Turn the default of "push.recurseSubmodules" to "check".
Will hold to wait for hv/submodule-not-yet-pushed-fix
This reveals that the "check" mode is too inefficient to use in
real projects, even in ones as small as git itself.
cf. [off-list ref]
So maybe we should eject this series from pu as long as
hv/submodule-not-yet-pushed-fix is ejected to enable you
running pu happily.
Thanks,
Stefan
Hi,
On Mon, Oct 03, 2016 at 02:11:36PM -0700, Linus Torvalds wrote:
This seems to be because I'm now on 'pu' as of a day or two ago in
order to test the abbrev logic, but lookie here:
time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
.. shows all the branches and tags ..
real 0m0.655s
user 0m0.011s
sys 0m0.004s
so the remote is fast to connect to, and with network connection
overhead and everything, it's just over half a second. But then:
time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
The reason behind this is when pushing to an address we do not easily
have the remote refs to compare available. When pushing an existing ref
it would be easy and could get a shortcut but it gets more complicated
for new refs. Currently we fall back to walking the whole history since
that is "the most correct way" we have. But obviously it is not a
practical solution in any way.
I mentioned this fact when discussing the current state and my patches
to make this check less painful. So we still need to think about a
solution for this check when passing an address.
IMO: It's definitely not ready to be switched on as default, unless we
find something a lot cheaper for the above case.
My idea of a solution goes like this:
* collect all SHA1's of the remotes refs
* check if we have them locally
* if not we abort and tell the user to fetch them somehow into local
refs or disable the check
* when we have them locally we proceed passing those SHA1's as bases
instead of --remotes=<name>
Cheers Heiko
From: Jeff King <hidden> Date: 2016-10-04 11:44:36
On Tue, Oct 04, 2016 at 01:18:45PM +0200, Heiko Voigt wrote:
On Mon, Oct 03, 2016 at 02:11:36PM -0700, Linus Torvalds wrote:
quoted
This seems to be because I'm now on 'pu' as of a day or two ago in
order to test the abbrev logic, but lookie here:
time git ls-remote ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
.. shows all the branches and tags ..
real 0m0.655s
user 0m0.011s
sys 0m0.004s
so the remote is fast to connect to, and with network connection
overhead and everything, it's just over half a second. But then:
time git push ra.kernel.org:/pub/scm/linux/kernel/git/torvalds/linux
The reason behind this is when pushing to an address we do not easily
have the remote refs to compare available. When pushing an existing ref
it would be easy and could get a shortcut but it gets more complicated
for new refs. Currently we fall back to walking the whole history since
that is "the most correct way" we have. But obviously it is not a
practical solution in any way.
I mentioned this fact when discussing the current state and my patches
to make this check less painful. So we still need to think about a
solution for this check when passing an address.
IMO: It's definitely not ready to be switched on as default, unless we
find something a lot cheaper for the above case.
My idea of a solution goes like this:
* collect all SHA1's of the remotes refs
* check if we have them locally
* if not we abort and tell the user to fetch them somehow into local
refs or disable the check
* when we have them locally we proceed passing those SHA1's as bases
instead of --remotes=<name>
As I argued in [1], I think it's not just "this must be cheaper" but
"this must not be enabled if submodules are not in use at all". Most
repositories don't have submodules enabled at all, so anything that
cause any extra traversal, even of a portion of the history, is going to
be a net negative for a lot of people.
I think the only sane default is going to be some kind of heuristic that
says "submodules are probably in use". Something like "is there a
.gitmodules file" is not perfect (you can have gitlink entries without
it), but it's a really cheap constant-time check.
-Peff
[1] Quoted in
http://public-inbox.org/git/xmqqh9aaot49.fsf@gitster.mtv.corp.google.com/
On Tue, Oct 04, 2016 at 07:44:28AM -0400, Jeff King wrote:
quoted
My idea of a solution goes like this:
* collect all SHA1's of the remotes refs
* check if we have them locally
* if not we abort and tell the user to fetch them somehow into local
refs or disable the check
* when we have them locally we proceed passing those SHA1's as bases
instead of --remotes=<name>
As I argued in [1], I think it's not just "this must be cheaper" but
"this must not be enabled if submodules are not in use at all". Most
repositories don't have submodules enabled at all, so anything that
cause any extra traversal, even of a portion of the history, is going to
be a net negative for a lot of people.
I think the only sane default is going to be some kind of heuristic that
says "submodules are probably in use". Something like "is there a
.gitmodules file" is not perfect (you can have gitlink entries without
it), but it's a really cheap constant-time check.
I agree. We are adding convenience for submodules, so we can also say a
checked out ".gitmodules" file is a must to have convenience.
I am not sure if I agree on another layer of options for this as
suggested in your post. More options mean more implementation
complexity and more confusion on the users side.
How about we choose our defaults based on the existence of a checked out
.gitmodules file? So the default would only be --recurse-submodules=check
if there is a .gitmodules file in the worktree. All other users need to
either pass or explicitly configure it.
Cheers Heiko
From: Jeff King <hidden> Date: 2016-10-04 12:07:42
On Tue, Oct 04, 2016 at 02:04:21PM +0200, Heiko Voigt wrote:
quoted
I think the only sane default is going to be some kind of heuristic that
says "submodules are probably in use". Something like "is there a
.gitmodules file" is not perfect (you can have gitlink entries without
it), but it's a really cheap constant-time check.
I agree. We are adding convenience for submodules, so we can also say a
checked out ".gitmodules" file is a must to have convenience.
I am not sure if I agree on another layer of options for this as
suggested in your post. More options mean more implementation
complexity and more confusion on the users side.
How about we choose our defaults based on the existence of a checked out
.gitmodules file? So the default would only be --recurse-submodules=check
if there is a .gitmodules file in the worktree. All other users need to
either pass or explicitly configure it.
That's OK with me. Though you may end up in the long run wanting some
name for the default behavior (e.g., if people configure something else
and then want to override back to "auto" in some instances), but that
can probably come later.
-Peff