All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.