git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kjetil Barvik <barvik@broadpark.no>
To: Johannes Sixt <j.sixt@viscovery.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH/RFC v3 7/9] write_entry(): use fstat() instead of lstat() when file is open
Date: Thu, 05 Feb 2009 11:46:24 +0100	[thread overview]
Message-ID: <86fxit9n5r.fsf@broadpark.no> (raw)

* Kjetil Barvik schrieb:
| And, yes, since each lstat() call cost approximately 44 microseconds
| compared to 12-16 for each lstat() on my Linux box, there was a little
                             ^^^^^^^ fstat()?

  Yes, is is the fstat() call which takes 12-16 microseconds on my Linux
  box.

| performance gain from this patch.

* Johannes Sixt <j.sixt@viscovery.net>
| This does look like a good gain. But do you have hard numbers that back
| the claim?
|
| If you can squeeze out a 10% improvement on Linux with this patch, we
| should take it, and if it turns out that it doesn't work on Windows, we
| could trivially throw in an #ifdef MINGW (or even #ifdef WIN32 if Cygwin
| is affected, too) that skips the fstat() optimization.
| 
| But we really should have numbers that justify this effort.

  I have been working on a long running test script latly, but then I
  started to play with the 'git repack' command, so it was not top
  priority.  But, I can finish the script today, and run it while I am
  sleeping tonight.

| BTW, the statement from the Windows documentation was:
|
|  "The only guarantee about a file timestamp is that the file time is
|   correctly reflected when the handle that makes the change is closed."
|
| In the case of this patch, the timestamp is queried via the handle
| that made the change, and in this case special case the timestamp
| could be correct nevertheless. The guarantee doesn't cover this case,
| but it would be natural, and perhaps it Just Works?

  Yes it could perhaps "just works".  But, then I guess that when it
  does not work, the user would not notice it _except_ for more time
  used.  Since I can to this:

kjetil@localhost ~/git/git $ git status 
# On branch lstat_fstat_v6
nothing to commit (working directory clean)
kjetil@localhost ~/git/git $ touch var.c
kjetil@localhost ~/git/git $ git status 
# On branch lstat_fstat_v6
nothing to commit (working directory clean)
kjetil@localhost ~/git/git $ 

  ... I think that git will have to check the content and read each byte
  and do a SHA1 of the file var.c in this case (since the timestamps do
  not match), which is a more time and CPU hungry opperation, to decide
  if there is a difference inside the file or not.

  And, maybe some other platform also have problems with this trick?

  OK, I do the time tests and let the numbers talk.

  -- kjetil

             reply	other threads:[~2009-02-05 10:48 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-05 10:46 Kjetil Barvik [this message]
  -- strict thread matches above, loose matches on Subject: below --
2009-02-04 12:52 [PATCH/RFC v3 0/9] git checkout: more cleanups, optimisation, less lstat() calls Kjetil Barvik
2009-02-04 12:53 ` [PATCH/RFC v3 7/9] write_entry(): use fstat() instead of lstat() when file is open Kjetil Barvik
2009-02-04 14:01   ` Johannes Sixt
2009-02-04 18:41     ` Junio C Hamano
2009-02-04 19:53       ` Kjetil Barvik
2009-02-04 20:30         ` Junio C Hamano
2009-02-05  8:14         ` Johannes Sixt
2009-02-05 11:03           ` Johannes Schindelin
2009-02-05 17:45             ` Junio C Hamano
2009-02-06 11:06             ` Kjetil Barvik
2009-02-06 11:26               ` Johannes Schindelin
2009-02-14 17:50           ` Kjetil Barvik

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=86fxit9n5r.fsf@broadpark.no \
    --to=barvik@broadpark.no \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j.sixt@viscovery.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).