Git development
 help / color / mirror / Atom feed
* Re: [PATCH v2 09/11] gitweb: git_is_head_detached() function
From: Junio C Hamano @ 2008-11-14  6:40 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Petr Baudis
In-Reply-To: <1226616555-24503-10-git-send-email-giuseppe.bilotta@gmail.com>

Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:

> The function checks if the HEAD for the current project is detached by
> checking if 'git branch' returns "* (no branch)"

This one looks more like "oops, the way detached HEAD is detected in 08 is
sucky, let's cover it up by introducing a function as an afterthought."

Have a patch that introduces git_is_head_detached() first, and then use
that function to implement the feature.  I personally think the user (that
is, 08/11) is small and isolated enough that these two can be a single
patch.

> +# check if current HEAD is detached
> +sub git_is_head_detached {
> +	my @x = (git_cmd(), 'branch');
> +	my @ret = split("\n", qx(@x));
> +	return 0 + grep { /^\* \(no branch\)$/ } @ret;
> +}

Do not read from Porcelain in scripts.

"git symbolic-ref HEAD" should error out when your HEAD is detached, and
will return refs/heads/frotz when you are on frotz branch.

But realistically speaking, what does it mean to have a detached HEAD in a
repository published via gitweb?  First of all these things are supposed
to be bare and there would be no checkout.

^ permalink raw reply

* Re: [RFC PATCH 0/4] deny push to current branch of non-bare repo
From: Jeff King @ 2008-11-14  6:37 UTC (permalink / raw)
  To: Kyle Moffett; +Cc: Junio C Hamano, git, Sam Vilain
In-Reply-To: <f73f7ab80811130558h34cb1220q76ef5536e853151c@mail.gmail.com>

On Thu, Nov 13, 2008 at 08:58:50AM -0500, Kyle Moffett wrote:

> I guess the issue comes down to a UI complication.  It would very easy
> for me to tell somebody how to check out and test their branch in my
> testbed if I'm not around, except for that little bit of arcane
> syntax.  Moreover, the consequences if they forget are really

If the problem is merely the syntax, then perhaps that argues for "git
checkout -d" to force detaching.

> frustrating and hard to figure out.  It's also very easy with a GUI to
> do the simple *rightclick branch, click "Checkout"*, but would be much
> harder to do the detached HEAD checkout correctly.

And again, perhaps this argues for a "Detach" option in the GUI.

But I have to admit, this is a pretty infrequently-used use-case. I
detach all the time when looking at non-branches, but I can't think of
the last time I used "ref^0" to detach intentionally.

> If it didn't involve reconfiguring a lot of other people's
> repositories, I might consider having them push to "refs/remotes/*".
> In theory that's actually much closer to what I'm doing anyways.  That
> would force any checkouts to be bare, but it would require lots of
> git-foo on the pushing side.  Perhaps some way to "git push" which
> asks the remote repository where it wants the stuff?

Or git-receive could even just silently munge the incoming refs when
writing them out (i.e., it exposes "refs/test/*" as "refs/heads/*", and
when you ask to write "refs/heads/foo" it writes "refs/test/foo"
instead).

Though that sort of lying feels a little wrong to me, since the pushing
side will incorrectly update its tracking branches. It wouldn't so bad
if the "fetch" side respected the munging, too.

But again, this seems uncommon enough that it is not worth trying to
implement something too clever.

> Alternatively, it might be possible to add ref attributes or a config
> option to force detached HEAD checkouts.

I think that is a more sensible solution. Your workflow is not about
"sometimes I want to detach the HEAD" but rather "in this particular
repo, we should _always_ detach the HEAD." Which a config option
represents very nicely.

-Peff

^ permalink raw reply

* Re: [PATCH] revision.c: use proper data type in call to sizeof() within xrealloc
From: Jeff King @ 2008-11-14  6:24 UTC (permalink / raw)
  To: Brandon Casey; +Cc: gitster, git
In-Reply-To: <2NXgsahhUCpkej6JukLNJjebzyoAQgZ4ptQMMrR2j_iWS5yEqKYn0kxHsUBpiY6bkpfbTjxlSn0@cipher.nrlssc.navy.mil>

On Thu, Nov 13, 2008 at 02:20:37PM -0600, Brandon Casey wrote:

>  	int num = ++revs->num_ignore_packed;
>  
>  	revs->ignore_packed = xrealloc(revs->ignore_packed,
> -				       sizeof(const char **) * (num + 1));
> +				       sizeof(const char *) * (num + 1));

Maybe it would be even nicer as "sizeof(*revs->ignore_packed)"?

-Peff

^ permalink raw reply

* Re: multiple-commit cherry-pick?
From: Chris Frey @ 2008-11-14  5:08 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Alex Riesen, Linus Torvalds, Junio C Hamano, Miles Bader, git
In-Reply-To: <alpine.DEB.1.00.0811102230330.30769@pacific.mpi-cbg.de>

On Mon, Nov 10, 2008 at 10:31:32PM +0100, Johannes Schindelin wrote:
> On Mon, 10 Nov 2008, Alex Riesen wrote:
> > for cs in HEAD^..HEAD HEAD~10; do
> >   case "$cs"; in
> >   *..*)
> >      git format-patch --stdout "$cs"
> >      ;;
> >   *)
> >      git show --pretty=email "$cs"
> >      ;;
> >   esac
> > done
> > 
> > At least, this is what I have in mind and how I expect it to work.
> 
> That is not the way git-show is implemented (it uses setup_revisions() to 
> check for validity and to parse the arguments), and I cannot think of any 
> way to make this work without ugly workarounds.

Would it be possible to add "range" support to a subset of commands by
using a git-range wrapper?

Hypothetical, pie-in-the-sky idea:

	git range HEAD^..HEAD HEAD~10 -- show --pretty=email
	git range HEAD^..HEAD HEAD~10 -- log
	git range HEAD^..HEAD HEAD~10 -- cherry-pick

Which would call the given command for each of the commits found in all
the specified ranges and lists.  git-range could have an internal list
of supported git subcommands that it would massage the parameter lists for.

I find this both elegant and ugly at the same time. :-)

- Chris

^ permalink raw reply

* Re: hosting git on a nfs
From: Linus Torvalds @ 2008-11-14  5:01 UTC (permalink / raw)
  To: James Pickens
  Cc: Brandon Casey, Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <885649360811131933webae91w134dce4c5c0ccf89@mail.gmail.com>



On Thu, 13 Nov 2008, James Pickens wrote:
> 
> I wonder if there are other completely different parts of git that could
> benefit from multi threading when the work tree is on nfs?

I'm sure there are. That said, threading things is usually really quite 
painful. The only reason this preloading was easy to do was that we really 
had all the data structures laid out beautifully for this, and I had spent 
a lot of effort earlier on a whole series of "avoid duplicate lstat()" 
changes, which gave us that whole ce_uptodate() thing, and all normal 
cases already taking advantage of it, and the "uptodate" bit being 
percolated along all the paths.

If it hadn't been for that, it would have been much nastier to do.

As it was, there was literally just a simple little extra phase to fill in 
all teh data structures that we already had set up in parallel.

> I'm thinking specifically of 'git checkout', since while testing this 
> patch I happened to do a 'git pull' that resulted in several thousand 
> new files being created, and the "Checking out files" part took 
> *forever* to run.

Now, the good news is that the actual work-tree part of checking things 
out is probably pretty amenable to the same kind of parallelization, for 
largely the same reasons: the whole checking out thing is already done in 
multiple phases with all error handling done before-hand. So we will have 
built up all our data structures earlier, and set the CE_UPDATE bit, and 
then there's just a final "push it all out" phase.

So CE_UPTODATE and CE_UPDATE are really very similar in that sense - 
except at opposite ends of the pipeline. The CE_UPTODATE bit marks a name 
entry as matching the filesystem data (and allows all later phases to 
avoid doing the expensive lstat()s), while the CE_UPDATE (and CE_REMOVE) 
bits allow us to do all our complex work in-memory without committing it 
to disk, and then we push it out in one go.

So if you want to multi-thread checkout, you literally need to just thread 
the last for-loop in unpack-trees.c:check_updates() (the CE_UPDATE loop 
that does "checkout_entry()" over the whole index). 

> And FWIW, I timed 50 iterations of 'git diff', and the average runtime
> dropped from 11.7s to 2.8s after this patch.  A nice improvement.

Very impressive. That said, I suspect you get a "superlinear" improvement 
because once it gets faster, the kernel cache also works better, since you 
can do more loops without having the NFS attributes time out.

Whether that kind of effect happens much in actual practice is debatable, 
although it's quite possible that it will work the same way in some 
scripting schenarios.

			Linus

^ permalink raw reply

* Re: hosting git on a nfs
From: James Pickens @ 2008-11-14  3:33 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Brandon Casey, Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811131707090.3468@nehalem.linux-foundation.org>

On Thu, Nov 13, 2008 at 6:15 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>  - maybe there are other users of "read_cache()" that want to do the
>   preloading. I just did "git diff" and "git status" (where the stuff
>   that "git commit" does falls out of the status changes)

I wonder if there are other completely different parts of git that could
benefit from multi threading when the work tree is on nfs?  I'm thinking
specifically of 'git checkout', since while testing this patch I
happened to do a 'git pull' that resulted in several thousand new files
being created, and the "Checking out files" part took *forever* to run.

And FWIW, I timed 50 iterations of 'git diff', and the average runtime
dropped from 11.7s to 2.8s after this patch.  A nice improvement.

James

^ permalink raw reply

* Re: [PATCH v2] Edit recipient addresses with the --compose flag
From: Junio C Hamano @ 2008-11-14  3:31 UTC (permalink / raw)
  To: Ian Hilt; +Cc: Git Mailing List, Pierre Habouzit
In-Reply-To: <alpine.LFD.2.00.0811132013530.6125@sys-0.hiltweb.site>

Ian Hilt <ian.hilt@gmx.com> writes:

> On Wed, 12 Nov 2008, Junio C Hamano wrote:
>> Ian Hilt <ian.hilt@gmx.com> writes:
>> 
>> > Sometimes specifying the recipient addresses can be tedious on the
>> > command-line.  This commit allows the user to edit the recipient
>> > addresses in their editor of choice.
>> >
>> > Signed-off-by: Ian Hilt <ian.hilt@gmx.com>
>> > ---
>> > Here's an updated commit with improved regex's from Junio and Francis.
>> 
>> This heavily depends on Pierre's patch, so I am CC'ing him for comments.
>> Until his series settles down, I cannot apply this anyway.
>
> I didn't realize this was such a bad time to submit this patch.

It is not a bad time.  I just won't be able to apply it right away, but
people (like Pierre) who are interested in send-email enhancement can help
improving your patch by reviewing.

>> Why does the user must keep "Cc:" in order for this new code to pick up
>> the list of recipients?  ...
>> 
>>     cc              =       "Cc:" address-list CRLF
>>     bcc             =       "Bcc:" (address-list / [CFWS]) CRLF
>>     address-list    =       (address *("," address)) / obs-addr-list
>
> I think you're mistaken here.  It is entirely possible to delete the Cc
> and Bcc lines with no ill effect.

You have this piece of code

>> > +	if ($c_file =~ /^To:\s*(\S.+?)\s*\nCc:/ism) {
>> > +		@tmp_to = get_recipients($1);
>> > +	}

to pick up the "To: " addressees.  If your user deletes Cc: line, would
that regexp still capture them in @tmp_to?  How?

> determine if $cc is equal to ''.  If it's not, then it will use it.

Ah, somehow I thought C2 you are writing into (message.final) was used as
the final payload, but you are right.  The foreach () loop at the toplevel
reads them and interprets them.

>> I think the parsing code you introduced simply suck.  Why isn't it done as
>> a part of the main loop to read the same file that already exists?
>
> Multiline recipient fields.

So you were trying to handle folded headers after all, I see.

But if you were to go that route, I think you are much better off doing so
by enabling the header folding for all the header lines in the while (<C>)
loop that currently reads one line at a time.

I however hove to wonder why we are not using any canned e-mail header
parser for this part of the code.  Surely there must be a widely used one
that everybody who writes Perl uses???

^ permalink raw reply

* Re: [RFC PATCH] repack: make repack -a equivalent to repack -A and drop previous -a behavior
From: Theodore Tso @ 2008-11-14  2:22 UTC (permalink / raw)
  To: Brandon Casey; +Cc: Björn Steinbrink, git
In-Reply-To: <jKWdt94ZxgNW0UAgUUW-qjTtpWohpQXMfvw-AUmOXND8SD5yFw0N8w@cipher.nrlssc.navy.mil>

On Thu, Nov 13, 2008 at 06:53:29PM -0600, Brandon Casey wrote:
> > -Ad is nice regarding it's safety-net value, but eg. after a large
> > filter-branch run, when refs/original and the reflogs have been cleaned,
> > you just want to get rid of all those old unreachable objects,
> > immediately. For example after importing and massaging some large
> > history from SVN, the -Ad behaviour is definitely _not_ what I want
> > there. Writing a few thousand loose objects just to prune them is just a
> > waste of time.
> 
> hmm. That's a good point. Even though I think it is likely that the thousand
> loose objects that are written will be small commit objects and not blobs,
> this use case may be enough to trump the safety benefit provided by the
> proposed change.

The problem is even small commit objects take a full 4k (or whatever
your filesystem block size is) when they are ejected as loose objects.
As a result, the current "git gc" defaults can end up requiring far
*more* disk space than before, certainly while it is running, and
sometimes even after the "git gc" completes.  (I then end up running
"git prune" to complete deletion of the ejected objects.)

Sometimes this gets so annoying that I'll run the individual commands
run by git-gc by hand, except I use git repack -ad instead of git
repack -A.  If we are going to get rid of the distinction between git
repack -a and git repack -A, perhaps there can be a config option to
force the immediate ejection of the unreachable objects, instead of
creating loose objects?  

If the goal is safety, it would be nice if git repack could create a
separate pack that only contained unreachable objects, and then have
git prune be able to remove a pack if it only contains unreachable
objects.

						- Ted

^ permalink raw reply

* Re: [PATCH v2] Edit recipient addresses with the --compose flag
From: Ian Hilt @ 2008-11-14  2:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git Mailing List, Pierre Habouzit
In-Reply-To: <7vskpwia91.fsf@gitster.siamese.dyndns.org>

On Wed, 12 Nov 2008, Junio C Hamano wrote:
> Ian Hilt <ian.hilt@gmx.com> writes:
> 
> > Sometimes specifying the recipient addresses can be tedious on the
> > command-line.  This commit allows the user to edit the recipient
> > addresses in their editor of choice.
> >
> > Signed-off-by: Ian Hilt <ian.hilt@gmx.com>
> > ---
> > Here's an updated commit with improved regex's from Junio and Francis.
> 
> This heavily depends on Pierre's patch, so I am CC'ing him for comments.
> Until his series settles down, I cannot apply this anyway.

I didn't realize this was such a bad time to submit this patch.

> > @@ -489,6 +492,9 @@ GIT: for the patch you are writing.
> >  GIT:
> >  GIT: Clear the body content if you don't wish to send a summary.
> >  From: $tpl_sender
> > +To: $tpl_to
> > +Cc: $tpl_cc
> > +Bcc: $tpl_bcc
> >  Subject: $tpl_subject
> >  In-Reply-To: $tpl_reply_to
> >  
> > @@ -512,9 +518,31 @@ EOT
> >  	open(C,"<",$compose_filename)
> >  		or die "Failed to open $compose_filename : " . $!;
> >  
> > +	local $/;
> > +	my $c_file = <C>;
> > +	$/ = "\n";
> > +	close(C);
> > +
> > +	my (@tmp_to, @tmp_cc, @tmp_bcc);
> > +
> > +	if ($c_file =~ /^To:\s*(\S.+?)\s*\nCc:/ism) {
> > +		@tmp_to = get_recipients($1);
> > +	}
> 
> Why "\S.+?" and not "\S.*?"?  A local user whose login name is 'q' is
> disallowed?

True.  Thanks for pointing this out.

> Why does the user must keep "Cc:" in order for this new code to pick up
> the list of recipients?  In other words, you are forbidding the user from
> removing the entire "Cc:" line, even when the message should not be Cc'ed
> to anywhere.  Instead there has to remain an empty Cc: line.  Worse yet,
> such an empty "Cc:" line is printed to C2 with your patch and eventually
> fed to sendmail.  I think it is a violation of 2822 to have Cc: that is
> empty, as the format is specified as:
> 
>     cc              =       "Cc:" address-list CRLF
>     bcc             =       "Bcc:" (address-list / [CFWS]) CRLF
>     address-list    =       (address *("," address)) / obs-addr-list

I think you're mistaken here.  It is entirely possible to delete the Cc
and Bcc lines with no ill effect.  It is also possible to leave them in
and not add any addresses and the code won't feed these empty lines to
sendmail.  In the subroutine send_message, I believe a check is made to
determine if $cc is equal to ''.  If it's not, then it will use it.  The
Bcc list is even simpler.  It is gathered into @recipients via
unique_email_list().  If it's empty, nothing happens.

> > +	if ($c_file =~ /^Cc:\s*(\S.+?)\s*\nBcc:/ism) {
> > +		@tmp_cc = get_recipients($1);
> > +	}
> > +	if ($c_file =~ /^Bcc:\s*(\S.+?)\s*\nSubject:/ism) {
> > +		@tmp_bcc = get_recipients($1);
> > +	}
> 
> Exactly the same comment applies to Bcc and Subject part of the parsing.
> 
> I think the parsing code you introduced simply suck.  Why isn't it done as
> a part of the main loop to read the same file that already exists?

Multiline recipient fields.

I know rfc 2822 that you cited above specifies that the address list not
contain a CRLF but as a terminator.  However, I thought that for
readability's sake it would be good to enable the user to introduce a
line break into the recipient field.  This is why I used the regex's the
way I did and slurp'd the file rather than worked on it line-by-line.

> Unlike your additional code above that reads the whole file into a scalar
> only to discard, the existing main loop processes one line at a file
> (which should be more memory efficient), and you are not handling the
> header continuation line anyway, processing one line at a time would make
> your code much simpler (for one thing, you do not have to do /sm at all).
> Also it won't be confused as your version would if the message happens to
> have "To:" or "Cc:" in the message part, thanks to $in_body variable check
> that is already in the code.

I definitely agree.  I started out coding exactly this way but then
thought about Pierre's comment about bloated To and Cc fields.  This is
why I wanted to allow the user to introduce line breaks into the
recipient fields.  It's a lot easier to read a nicely formatted email
list if there are a lot of addresses.

As a second thought, I probably should have put RFC into the subject of
this patch.

And as far as my code's recipient regex matching the body of the
message, I'll try to find a solution.  It may not be possible to do
multiline recipient fields without this problem.  Thanks for reviewing
this.

^ permalink raw reply

* Re: [RFC PATCH] repack: make repack -a equivalent to repack -A and drop previous -a behavior
From: Björn Steinbrink @ 2008-11-14  1:48 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git
In-Reply-To: <Dei99j9xFoGAh0A3LCO0zx3LqCb4lViSZpGvv_KGn38@cipher.nrlssc.navy.mil>

On 2008.11.13 19:36:45 -0600, Brandon Casey wrote:
> Björn Steinbrink wrote:
> > On 2008.11.13 18:53:29 -0600, Brandon Casey wrote:
> >> Björn Steinbrink wrote:
> >>> We have:
> >>> 	-a	Create a new pack, containing all reachable objects
> >>> 	-A	Same as -a
> >>> 	-ad	Same as -a, and drop all old packs and loose objects
> >> by loose objects, I assume you mean packed unreachable objects.
> > 
> > No, actually I just totally ignored the fact that -a of course already
> > deletes the loose objects.
> 
> Actually, I had forgotten that repack deletes any loose objects at all.
> It does call prune-packed, but only when -d is used.

Ugh, right. -a does not delete loose objects without -d. So, ignoring
the .keep stuff, my initial description was even right and I just
confused myself afterwards :-/

Thanks,
Björn

^ permalink raw reply

* Re: [RFC PATCH] repack: make repack -a equivalent to repack -A and drop previous -a behavior
From: Brandon Casey @ 2008-11-14  1:36 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: git
In-Reply-To: <20081114012502.GC5285@atjola.homenet>

Björn Steinbrink wrote:
> On 2008.11.13 18:53:29 -0600, Brandon Casey wrote:
>> Björn Steinbrink wrote:
>>> I didn't check all the (proposed) commits for that branch, so just let
>>> me know if I'm missing anything, but doesn't this change mean that you
>>> just lose what "-ad" did?
>> yes.
>>
>>> We have:
>>> 	-a	Create a new pack, containing all reachable objects
>>> 	-A	Same as -a
>>> 	-ad	Same as -a, and drop all old packs and loose objects
>> by loose objects, I assume you mean packed unreachable objects.
> 
> No, actually I just totally ignored the fact that -a of course already
> deletes the loose objects.

Actually, I had forgotten that repack deletes any loose objects at all.
It does call prune-packed, but only when -d is used.

> IMHO, "git gc" already provides enough safety. I tend to see "gc" as the
> regular "just use it" tool, while repack gives me more control over how
> I want things to be done, without forcing me to use the real plumbing or
> to fumble around with the configuration for gc. And when I want control,
> I'm generally prepared to shoot myself in the foot.

I think you're right. Thanks for providing an example of a real use case.

-brandon

^ permalink raw reply

* Re: [RFC PATCH] repack: make repack -a equivalent to repack -A and drop previous -a behavior
From: Björn Steinbrink @ 2008-11-14  1:25 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git
In-Reply-To: <jKWdt94ZxgNW0UAgUUW-qjTtpWohpQXMfvw-AUmOXND8SD5yFw0N8w@cipher.nrlssc.navy.mil>

On 2008.11.13 18:53:29 -0600, Brandon Casey wrote:
> Björn Steinbrink wrote:
> > I didn't check all the (proposed) commits for that branch, so just let
> > me know if I'm missing anything, but doesn't this change mean that you
> > just lose what "-ad" did?
> 
> yes.
> 
> > We have:
> > 	-a	Create a new pack, containing all reachable objects
> > 	-A	Same as -a
> > 	-ad	Same as -a, and drop all old packs and loose objects
> 
> by loose objects, I assume you mean packed unreachable objects.

No, actually I just totally ignored the fact that -a of course already
deletes the loose objects. The packed unreachable objects are in the old
packs, so they're already included in the first half of my sentence ;-)

> > 	-Ad	Sama as -ad, but keep unreachable objects loose
> > 
> > -Ad is nice regarding it's safety-net value, but eg. after a large
> > filter-branch run, when refs/original and the reflogs have been cleaned,
> > you just want to get rid of all those old unreachable objects,
> > immediately. For example after importing and massaging some large
> > history from SVN, the -Ad behaviour is definitely _not_ what I want
> > there. Writing a few thousand loose objects just to prune them is just a
> > waste of time.
> 
> hmm. That's a good point. Even though I think it is likely that the thousand
> loose objects that are written will be small commit objects and not blobs,

When you only fix up merge commits, author information and such things,
then yes, most objects will be commits. And then it's not even that bad.

But a more interesting case is when in your old SCM you had multiple
projects in one repo, and you can't sanely separate them before the
import. So you might end up using the subdirectory filter a few times,
or even just drop a bunch of branches in each copy of your import.

And another one is when you had accidently commited some huge, useless
files, and as you're switching to git now anyway, you want to get rid of
them, so you use an index-filter to drop them.

For those two cases, -Ad vs -ad can make a huge difference. I remember
someone on #git using a subdirectory filter on some project and trying
to get the repo to a sane size afterwards. -Ad took basically forever,
while -ad finished in 5 seconds or so.

> this use case may be enough to trump the safety benefit provided by the
> proposed change.

IMHO, "git gc" already provides enough safety. I tend to see "gc" as the
regular "just use it" tool, while repack gives me more control over how
I want things to be done, without forcing me to use the real plumbing or
to fumble around with the configuration for gc. And when I want control,
I'm generally prepared to shoot myself in the foot.

Björn

^ permalink raw reply

* Re: hosting git on a nfs
From: Linus Torvalds @ 2008-11-14  1:15 UTC (permalink / raw)
  To: Brandon Casey
  Cc: James Pickens, Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811131630470.3468@nehalem.linux-foundation.org>



On Thu, 13 Nov 2008, Linus Torvalds wrote:
> 
> I'll clean it up a bit and make a less hacky version. And I'll try to make 
> it work for "git status" and friends too.

Ok, this is a no-longer-totally-hacky thing, which also adds support for 
doing the same for "git status". I haven't actually done any timings, but 
the preload algorithm is all the same. The interface is just a much more 
natural one.

NOTE NOTE NOTE! It may not be totally hacky, but it's still not 
"finished". There's a few more things to look at:

 - maybe there are other users of "read_cache()" that want to do the 
   preloading. I just did "git diff" and "git status" (where the stuff 
   that "git commit" does falls out of the status changes)

 - I do think the thing should be more configurable. The "ten threads" 
   approach makes no sense for people who have single-core CPU's and tend 
   to have their trees cached - it will just slow things down for that 
   case. So there should probably be some config option to turn this on 
   and off.

 - It would be really cool to find some way to automatically notice when 
   the tree is hot-cached and we might as well just be linear. I don't 
   know exactly what it might be, but one of the nice things is that the 
   preloading is _entirely_ optimistic, and we could just stop it in the 
   middle if we notice that there's no upside.

 - Somebody else should take a look at it in general. It may work, but 
   maybe there's something stupid I'm doing, or maybe somebody else can 
   come up with a clever thing.

So I think this is probably my last version for now, and let's see if 
others can find improvements. I'm not ashamed of it any more. There's 
still a "Hacky hack start" comment, it's about the whole config thing. 

			Linus

---
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu, 13 Nov 2008 16:36:30 -0800
Subject: [PATCH] Add cache preload facility

This can do the lstat() storm in parallel, giving potentially much
improved performance for cold-cache cases or things like NFS that have
weak metadata caching.

Just use "read_cache_preload()" instead of "read_cache()" to force an
optimistic preload of the index stat data.  The function takes a
pathspec as its argument, allowing us to preload only the relevant
portion of the index.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 Makefile             |    1 +
 builtin-commit.c     |    8 ++--
 builtin-diff-files.c |    4 +-
 builtin-diff.c       |    8 ++--
 cache.h              |    2 +
 preload-index.c      |   83 ++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 96 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 920cb42..8387005 100644
--- a/Makefile
+++ b/Makefile
@@ -495,6 +495,7 @@ LIB_OBJS += write_or_die.o
 LIB_OBJS += ws.o
 LIB_OBJS += wt-status.o
 LIB_OBJS += xdiff-interface.o
+LIB_OBJS += preload-index.o
 
 BUILTIN_OBJS += builtin-add.o
 BUILTIN_OBJS += builtin-annotate.o
diff --git a/builtin-commit.c b/builtin-commit.c
index 93ca496..90b976e 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -225,18 +225,18 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
 
 	if (interactive) {
 		interactive_add(argc, argv, prefix);
-		if (read_cache() < 0)
+		if (read_cache_preload(NULL) < 0)
 			die("index file corrupt");
 		commit_style = COMMIT_AS_IS;
 		return get_index_file();
 	}
 
-	if (read_cache() < 0)
-		die("index file corrupt");
-
 	if (*argv)
 		pathspec = get_pathspec(prefix, argv);
 
+	if (read_cache_preload(pathspec) < 0)
+		die("index file corrupt");
+
 	/*
 	 * Non partial, non as-is commit.
 	 *
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 2b578c7..5b64011 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -59,8 +59,8 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	    (rev.diffopt.output_format & DIFF_FORMAT_PATCH))
 		rev.combine_merges = rev.dense_combined_merges = 1;
 
-	if (read_cache() < 0) {
-		perror("read_cache");
+	if (read_cache_preload(rev.diffopt.paths) < 0) {
+		perror("read_cache_preload");
 		return -1;
 	}
 	result = run_diff_files(&rev, options);
diff --git a/builtin-diff.c b/builtin-diff.c
index 82d4dda..b9a2b37 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -134,8 +134,8 @@ static int builtin_diff_index(struct rev_info *revs,
 	    revs->max_count != -1 || revs->min_age != -1 ||
 	    revs->max_age != -1)
 		usage(builtin_diff_usage);
-	if (read_cache() < 0) {
-		perror("read_cache");
+	if (read_cache_preload(revs->diffopt.paths) < 0) {
+		perror("read_cache_preload");
 		return -1;
 	}
 	return run_diff_index(revs, cached);
@@ -234,8 +234,8 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
 		revs->combine_merges = revs->dense_combined_merges = 1;
 
 	setup_work_tree();
-	if (read_cache() < 0) {
-		perror("read_cache");
+	if (read_cache_preload(revs->diffopt.paths) < 0) {
+		perror("read_cache_preload");
 		return -1;
 	}
 	result = run_diff_files(revs, options);
diff --git a/cache.h b/cache.h
index 6be60ea..c7b69e8 100644
--- a/cache.h
+++ b/cache.h
@@ -262,6 +262,7 @@ static inline void remove_name_hash(struct cache_entry *ce)
 
 #define read_cache() read_index(&the_index)
 #define read_cache_from(path) read_index_from(&the_index, (path))
+#define read_cache_preload(pathspec) read_index_preload(&the_index, (pathspec))
 #define is_cache_unborn() is_index_unborn(&the_index)
 #define read_cache_unmerged() read_index_unmerged(&the_index)
 #define write_cache(newfd, cache, entries) write_index(&the_index, (newfd))
@@ -368,6 +369,7 @@ extern int init_db(const char *template_dir, unsigned int flags);
 
 /* Initialize and use the cache information */
 extern int read_index(struct index_state *);
+extern int read_index_preload(struct index_state *, const char **pathspec);
 extern int read_index_from(struct index_state *, const char *path);
 extern int is_index_unborn(struct index_state *);
 extern int read_index_unmerged(struct index_state *);
diff --git a/preload-index.c b/preload-index.c
new file mode 100644
index 0000000..e322c27
--- /dev/null
+++ b/preload-index.c
@@ -0,0 +1,83 @@
+/*
+ * Copyright (C) 2008 Linus Torvalds
+ */
+#include <pthread.h>
+#include "cache.h"
+
+/* Hacky hack-hack start */
+#define MAX_PARALLEL (100)
+int parallel_lstats = 1;
+
+struct thread_data {
+	pthread_t pthread;
+	struct index_state *index;
+	const char **pathspec;
+	int offset, nr;
+};
+
+static void *preload_thread(void *_data)
+{
+	int nr;
+	struct thread_data *p = _data;
+	struct index_state *index = p->index;
+	struct cache_entry **cep = index->cache + p->offset;
+
+	nr = p->nr;
+	if (nr + p->offset > index->cache_nr)
+		nr = index->cache_nr - p->offset;
+
+	do {
+		struct cache_entry *ce = *cep++;
+		struct stat st;
+
+		if (ce_stage(ce))
+			continue;
+		if (ce_uptodate(ce))
+			continue;
+		if (!ce_path_match(ce, p->pathspec))
+			continue;
+		if (lstat(ce->name, &st))
+			continue;
+		if (ie_match_stat(index, ce, &st, 0))
+			continue;
+		ce_mark_uptodate(ce);
+	} while (--nr > 0);
+	return NULL;
+}
+
+static void preload_index(struct index_state *index, const char **pathspec)
+{
+	int i, work, offset;
+	int threads = index->cache_nr / 100;
+	struct thread_data data[MAX_PARALLEL];
+
+	if (threads < 2)
+		return;
+	if (threads > MAX_PARALLEL)
+		threads = MAX_PARALLEL;
+	offset = 0;
+	work = (index->cache_nr + threads - 1) / threads;
+	for (i = 0; i < threads; i++) {
+		struct thread_data *p = data+i;
+		p->index = index;
+		p->pathspec = pathspec;
+		p->offset = offset;
+		p->nr = work;
+		offset += work;
+		if (pthread_create(&p->pthread, NULL, preload_thread, p))
+			die("unable to create threaded lstat");
+	}
+	for (i = 0; i < threads; i++) {
+		struct thread_data *p = data+i;
+		if (pthread_join(p->pthread, NULL))
+			die("unable to join threaded lstat");
+	}
+}
+
+int read_index_preload(struct index_state *index, const char **pathspec)
+{
+	int retval = read_index(index);
+
+	preload_index(index, pathspec);
+	return retval;
+}

^ permalink raw reply related

* Re: hosting git on a nfs
From: Pieter de Bie @ 2008-11-14  0:59 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Brandon Casey, James Pickens, Bruce Fields, Junio C Hamano,
	Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811131630470.3468@nehalem.linux-foundation.org>


On Nov 14, 2008, at 1:38 AM, Linus Torvalds wrote:

> Ok, both you and Julian do seem to be getting a nice speedup from  
> this.
>
> I'll clean it up a bit and make a less hacky version. And I'll try  
> to make
> it work for "git status" and friends too.

I have two more datapoints.

The first is OS X 10.5 on a local HFS repository. git.git goes down  
from ~70 to ~63ms and mozilla's repository (30000) files goes from  
~350ms to ~230ms (yeah, HFS sucks).

Using AFP for the same repositories, the first goes down from ~80 to  
~67ms, the second from ~70 seconds to ~50 seconds, which is a nice  
speedup :)

- Pieter

^ permalink raw reply

* Re: [RFC PATCH] repack: make repack -a equivalent to repack -A and drop previous -a behavior
From: Brandon Casey @ 2008-11-14  0:53 UTC (permalink / raw)
  To: Björn Steinbrink; +Cc: git
In-Reply-To: <20081114000219.GB5285@atjola.homenet>

Björn Steinbrink wrote:
> On 2008.11.13 17:22:36 -0600, Brandon Casey wrote:
>> Once upon a time, repack had only a single option which began with the first
>> letter of the alphabet.  Then, a second was created which would repack
>> unreachable objects into the newly created pack so that git-gc --auto could
>> be invented.  But, the -a option was still necessary so that it could be
>> called every now and then to discard the unreachable objects that were being
>> repacked over and over and over into newly generated packs. Later, -A was
>> changed so that instead of repacking the unreachable objects, it ejected
>> them from the pack so that they resided in the object store in loose form,
>> to be garbage collected by prune-packed according to normal expiry rules.
>>
>> And so, -a lost its raison d'etre.
>>
>> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
>> ---
>>
>>
>> This is on top of bc/maint-keep-pack
> 
> I didn't check all the (proposed) commits for that branch, so just let
> me know if I'm missing anything, but doesn't this change mean that you
> just lose what "-ad" did?

yes.

> We have:
> 	-a	Create a new pack, containing all reachable objects
> 	-A	Same as -a
> 	-ad	Same as -a, and drop all old packs and loose objects

by loose objects, I assume you mean packed unreachable objects.

> 	-Ad	Sama as -ad, but keep unreachable objects loose
> 
> -Ad is nice regarding it's safety-net value, but eg. after a large
> filter-branch run, when refs/original and the reflogs have been cleaned,
> you just want to get rid of all those old unreachable objects,
> immediately. For example after importing and massaging some large
> history from SVN, the -Ad behaviour is definitely _not_ what I want
> there. Writing a few thousand loose objects just to prune them is just a
> waste of time.

hmm. That's a good point. Even though I think it is likely that the thousand
loose objects that are written will be small commit objects and not blobs,
this use case may be enough to trump the safety benefit provided by the
proposed change.

-brandon

^ permalink raw reply

* Re: [EGIT PATCH 6/7] Add tags to the graphical history display.
From: Robin Rosenberg @ 2008-11-14  0:49 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081111182816.GQ2932@spearce.org>

tisdag 11 november 2008 19:28:16 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > @@ -92,7 +104,64 @@ protected void drawText(final String msg, final int x, final int y) {
> >  		g.drawString(msg, cellX + x, cellY + texty, true);
> >  	}
> 
> My SWTPlotRenderer doesn't have this context line.  Am I missing
> a patch in the series?

>From the "Respect background when drawing history " patch sent to you after the first version
of this series.

-- robin

^ permalink raw reply

* Re: [EGIT PATCH 4/7] Handle peeling of loose refs.
From: Robin Rosenberg @ 2008-11-14  0:38 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081111182357.GO2932@spearce.org>

tisdag 11 november 2008 19:23:58 skrev Shawn O. Pearce:
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > @@ -238,10 +235,19 @@ public ObjectId getObjectId() {
> >  	 *         refer to an annotated tag.
> >  	 */
> >  	public ObjectId getPeeledObjectId() {
> > +		if (peeledObjectId == ObjectId.zeroId())
> > +			return null;
> >  		return peeledObjectId;
> >  	}
> >  
> >  	/**
> > +	 * @return whether the Ref represents a peeled tag
> > +	 */
> > +	public boolean isPeeled() {
> > +		return peeledObjectId != null && peeledObjectId != ObjectId.zeroId();
> > +	}
> 
> Huh.  So peeledObjectId == ObjectId.zeroId() means what, exactly?
> That we tried to peel this but we cannot because its not a tag?
Means we haven't tried to peel it yet. I could use an extra flags instead. We may
have thousands of refs but perhaps not zillions.

> 
> What does a packed-refs record which has no peel data but which
> is in a packed-refs with-peeled file have for peeledObjectId?
> null or ObjectId.zeroId()?
> 
> > @@ -288,6 +289,28 @@ private void readOneLooseRef(final Map<String, Ref> avail,
> >  		}
> >  	}
> >  
> > +	Ref peel(final Ref ref) {
> > +		if (ref.isPeeled())
> > +			return ref;
> 
> I think you mean something more like:
> 
> 	if (ref.peeledObjectId != null)
> 		return ref;
> 
> as the ref is already peeled, or at least attempted.
No, if it is a loose ref we did not yet attempt to peel it. null means we have tried
zeroId means we haven't tried (see above).

> 
> > +		try {
> > +			Object tt = db.mapObject(ref.getObjectId(), ref.getName());
> > +			if (tt != null && tt instanceof Tag) {
> > +				Tag t = (Tag)tt;
> > +				while (t != null && t.getType().equals(Constants.TYPE_TAG))
> > +					t = db.mapTag(t.getTag(), t.getObjId());
> > +				if (t != null)
> > +					ref.setPeeledObjectId(t.getObjId());
> > +				else
> > +					ref.setPeeledObjectId(null);
> > +			} else
> > +				ref.setPeeledObjectId(ref.getObjectId());
> > +		} catch (IOException e) {
> > +			// Serious error. Caller knows a ref should never be null
> > +			ref.setPeeledObjectId(null);
> > +		}
> > +		return ref;
> 
> This whole block is simpler to write as:
ok..
>         try {
>                 Object target = db.mapObject(ref.getObjectId(), ref.getName());
>                 while (target instanceof Tag) {
>                         final Tag tag = (Tag)target;
>                         ref.setPeeledObjectId(tag.getObjId());
>                         if (Constants.TYPE_TAG.equals(tag.getType()))
>                                 target = db.mapObject(tag.getObjId(), ref.getName());
>                         else
>                                 break;
>                 }
>         } catch (IOException e) {
>                 // Ignore a read error.  Callers will also get the same error
>                 // if they try to use the result of getPeeledObjectId.
>         }
> 
That gives a different behavior, but which is also ok...

> >  	/**
> > +	 * Peel a possibly unpeeled ref and updates it. If the ref cannot be peeled
> > +	 * the peeled id is set to {@link ObjectId#zeroId()}
> 
> But applications never see ObjectId.zeroId() because you earlier defined
> getPeeledObjectId() to return null in that case.  So why is this part of
> the documentation relevant? 
...it's wrong, and thus irrelevant.

> > +	 * 
> > +	 * @param ref
> > +	 *            The ref to peel
> > +	 * @return The same, an updated ref with peeled info or a new instance with
> > +	 *         more information
> > +	 */
> > +	public Ref peel(final Ref ref) {
> > +		return refs.peel(ref);
> 
> Can we tighten this contract?  Are we always going to update the
> current ref in-place, or are we going to return the caller a new
> Ref instance?  I'd like it to be consistent one way or the other.
Ack. 
> 
> If we are updating in-place then the caller needs to realize there
> may be some cache synchronization issues when using multiple threads
> to access the same Ref instances and peeling them.  I.e. they may
> need to go through their own synchornization barrier to ensure the
> processor caches are coherent.
> 
> Given that almost everywhere else we treat the Ref as immutable
> I'm tempted to say this should always return a new Ref object if we
> peeled; but return the parameter if the parameter is already peeled
> (or is known to be unpeelable, e.g. a branch head).

I pick the copy.

-- robin

^ permalink raw reply

* Re: hosting git on a nfs
From: Linus Torvalds @ 2008-11-14  0:38 UTC (permalink / raw)
  To: Brandon Casey
  Cc: James Pickens, Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <371xaQfxsMMQ-9LK24q-nhcS4loEggn8Cj3J1IzfMbzzYDGE6HKbQQ@cipher.nrlssc.navy.mil>



On Thu, 13 Nov 2008, Brandon Casey wrote:
>
> Cold cache* (over NFS):
> 
> Before:
> 
> 0.01user 0.31system 0:04.40elapsed 7%CPU
> 0.01user 0.31system 0:06.47elapsed 5%CPU
> 0.01user 0.26system 0:04.19elapsed 6%CPU
> 0.01user 0.30system 0:04.99elapsed 6%CPU
> 
> After:
> 
> 0.01user 0.46system 0:01.39elapsed 34%CPU
> 0.01user 0.41system 0:00.88elapsed 47%CPU
> 0.01user 0.45system 0:01.16elapsed 40%CPU
> 0.01user 0.45system 0:00.99elapsed 47%CPU
> 0.01user 0.45system 0:01.02elapsed 45%CPU

Ok, both you and Julian do seem to be getting a nice speedup from this.

I'll clean it up a bit and make a less hacky version. And I'll try to make 
it work for "git status" and friends too.

		Linus

^ permalink raw reply

* Re: [BUG] fatal error during merge
From: Junio C Hamano @ 2008-11-14  0:29 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Anders Melchiorsen, SZEDER Gábor, Samuel Tardieu, git,
	Johannes Schindelin
In-Reply-To: <48332.N1gUGH5fRhE=.1226619243.squirrel@webmail.hotelhot.dk>

"Anders Melchiorsen" <mail@cup.kalibalik.dk> writes:

> Your patch got rid of the errors and it commits the merged tree. But the
> working tree is not updated correctly, so the moved file disappears.

Isn't this a long known breakage of D/F conflict check logic in
merge-recursive backend?

^ permalink raw reply

* Re: hosting git on a nfs
From: Brandon Casey @ 2008-11-14  0:14 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: James Pickens, Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811131518070.3468@nehalem.linux-foundation.org>



Second version.


Hot cache (over NFS):

Before:

0.01user 0.11system 0:00.13elapsed 95%CPU
0.01user 0.11system 0:00.13elapsed 96%CPU
0.01user 0.12system 0:00.13elapsed 97%CPU

After:

0.01user 0.09system 0:00.11elapsed 95%CPU
0.01user 0.09system 0:00.11elapsed 95%CPU
0.01user 0.09system 0:00.11elapsed 95%CPU


Cold cache* (over NFS):

Before:

0.01user 0.31system 0:04.40elapsed 7%CPU
0.01user 0.31system 0:06.47elapsed 5%CPU
0.01user 0.26system 0:04.19elapsed 6%CPU
0.01user 0.30system 0:04.99elapsed 6%CPU

After:

0.01user 0.46system 0:01.39elapsed 34%CPU
0.01user 0.41system 0:00.88elapsed 47%CPU
0.01user 0.45system 0:01.16elapsed 40%CPU
0.01user 0.45system 0:00.99elapsed 47%CPU
0.01user 0.45system 0:01.02elapsed 45%CPU


* Note: I can't do 'echo 3 > /proc/sys/vm/drop_caches', so cold
  cache means 'sleep 60' between git diff (30 wasn't long enough).

-brandon

^ permalink raw reply

* Re: hosting git on a nfs
From: Julian Phillips @ 2008-11-14  0:04 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: James Pickens, Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <alpine.LFD.2.00.0811131518070.3468@nehalem.linux-foundation.org>

On Thu, 13 Nov 2008, Linus Torvalds wrote:

> The good news is that I seem to actualluy see a bit of a win from this
> even on a disk, now that the kernel doesn't serialize things. So it may
> be worth it. So I have some hope that it actually helps on NFS too.
> The numbers for five runs (with clearing of the caches in between, of
> course) are:
>
> Before:
>
> 	0.01user 0.23system 0:10.87elapsed 2%CPU
> 	0.04user 0.19system 0:10.86elapsed 2%CPU
> 	0.03user 0.26system 0:10.82elapsed 2%CPU
> 	0.02user 0.27system 0:12.67elapsed 2%CPU
> 	0.01user 0.22system 0:10.86elapsed 2%CPU
>
> After:
>
> 	0.03user 0.26system 0:07.88elapsed 3%CPU
> 	0.02user 0.25system 0:07.63elapsed 3%CPU
> 	0.01user 0.26system 0:08.62elapsed 3%CPU
> 	0.01user 0.26system 0:07.27elapsed 3%CPU
> 	0.05user 0.28system 0:08.61elapsed 3%CPU
>
> so it really does seem like it has possibly given a 30% improvement in
> cold-cache performance even on a disk.

On an NFS kernel checkout I get the following elapsed times:

master:

0:02.78
0:02.70
0:02.43
0:02.28
0:02.71
0:02.80
0:02.60
0:02.06
0:02.00

master + new patch:

0:00.77
0:00.83
0:01.02
0:00.77
0:00.91
0:00.78
0:00.78
0:01.09
0:01.00

-- 
Julian

  ---
There are no accidents whatsoever in the universe.
 		-- Baba Ram Dass

^ permalink raw reply

* Re: [RFC PATCH] repack: make repack -a equivalent to repack -A and drop previous -a behavior
From: Björn Steinbrink @ 2008-11-14  0:02 UTC (permalink / raw)
  To: Brandon Casey; +Cc: git
In-Reply-To: <7xh1_tFsJkHTjg4Hjm-R4mGSRfYyGFmLI62OMmaNg32f86EbVIYvYrGiYpEGkvooY4pUM1e7CHk@cipher.nrlssc.navy.mil>

On 2008.11.13 17:22:36 -0600, Brandon Casey wrote:
> Once upon a time, repack had only a single option which began with the first
> letter of the alphabet.  Then, a second was created which would repack
> unreachable objects into the newly created pack so that git-gc --auto could
> be invented.  But, the -a option was still necessary so that it could be
> called every now and then to discard the unreachable objects that were being
> repacked over and over and over into newly generated packs. Later, -A was
> changed so that instead of repacking the unreachable objects, it ejected
> them from the pack so that they resided in the object store in loose form,
> to be garbage collected by prune-packed according to normal expiry rules.
> 
> And so, -a lost its raison d'etre.
> 
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
> 
> 
> This is on top of bc/maint-keep-pack

I didn't check all the (proposed) commits for that branch, so just let
me know if I'm missing anything, but doesn't this change mean that you
just lose what "-ad" did?

We have:
	-a	Create a new pack, containing all reachable objects
	-A	Same as -a
	-ad	Same as -a, and drop all old packs and loose objects
	-Ad	Sama as -ad, but keep unreachable objects loose

-Ad is nice regarding it's safety-net value, but eg. after a large
filter-branch run, when refs/original and the reflogs have been cleaned,
you just want to get rid of all those old unreachable objects,
immediately. For example after importing and massaging some large
history from SVN, the -Ad behaviour is definitely _not_ what I want
there. Writing a few thousand loose objects just to prune them is just a
waste of time.

Björn

^ permalink raw reply

* [PATCH v2 11/11] gitweb: CSS style and refs mark for detached HEAD
From: Giuseppe Bilotta @ 2008-11-13 23:54 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1226620461-25168-1-git-send-email-giuseppe.bilotta@gmail.com>

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 gitweb/gitweb.css  |    5 +++++
 gitweb/gitweb.perl |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 751749f..c0c4540 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -523,6 +523,11 @@ span.refs span.head {
 	border-color: #ccffcc #00cc33 #00cc33 #ccffcc;
 }
 
+span.refs span.detached {
+	background-color: #ffaaaa;
+	border-color: #ccffcc #00cc33 #00cc33 #ccffcc;
+}
+
 span.atnight {
 	color: #cc0000;
 }
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 256c962..51e133d 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2298,7 +2298,7 @@ sub git_get_references {
 	my %refs;
 	if (git_is_head_detached()) {
 		my $hash = git_get_head_hash($project);
-		$refs{$hash} = [ 'HEAD' ];
+		$refs{$hash} = [ 'detached/HEAD' ];
 	}
 	# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11
 	# c39ae07f393806ccf406ef966e9a15afc43cc36a refs/tags/v2.6.11^{}
-- 
1.5.6.5

^ permalink raw reply related

* [PATCH v2 10/11] gitweb: add HEAD to list of shortlog refs if detached
From: Giuseppe Bilotta @ 2008-11-13 23:54 UTC (permalink / raw)
  To: git; +Cc: Jakub Narebski, Petr Baudis, Junio C Hamano, Giuseppe Bilotta
In-Reply-To: <1226616555-24503-10-git-send-email-giuseppe.bilotta@gmail.com>

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
 gitweb/gitweb.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index ceb0271..256c962 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -2296,6 +2296,10 @@ sub git_get_last_activity {
 sub git_get_references {
 	my $type = shift || "";
 	my %refs;
+	if (git_is_head_detached()) {
+		my $hash = git_get_head_hash($project);
+		$refs{$hash} = [ 'HEAD' ];
+	}
 	# 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c refs/tags/v2.6.11
 	# c39ae07f393806ccf406ef966e9a15afc43cc36a refs/tags/v2.6.11^{}
 	open my $fd, "-|", git_cmd(), "show-ref", "--dereference",
-- 
1.5.6.5

^ permalink raw reply related

* Re: hosting git on a nfs
From: Linus Torvalds @ 2008-11-13 23:48 UTC (permalink / raw)
  To: James Pickens; +Cc: Bruce Fields, Junio C Hamano, Git Mailing List
In-Reply-To: <885649360811131523h2e10dc44x8603c9793dae03b8@mail.gmail.com>



On Thu, 13 Nov 2008, James Pickens wrote:
> 
> I'm trying to test this and so far I'm not seeing any effect.

Ok, try my second version. The first version would get _some_ parallelism, 
but very little due to all the threads often trying to just look up in the 
same directory. The second version should hopefully have less of that 
effect.

Also, under Linux, try it with caches cleared in between runs, so that you 
can avoid any issues of the Linux client caching the directory lookup data 
(which linux _will_ do - I think the default timeout is 30 seconds or 
something like that):

	echo 3 > /proc/sys/vm/drop_caches ; time git diff

which should hopefully get you more reliable timings.

			Linus

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox