From: Johannes Sixt <j.sixt@viscovery.net>
To: Kjetil Barvik <barvik@broadpark.no>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH/RFC v3 7/9] write_entry(): use fstat() instead of lstat() when file is open
Date: Wed, 04 Feb 2009 15:01:08 +0100 [thread overview]
Message-ID: <49899FA4.2020003@viscovery.net> (raw)
In-Reply-To: <21073c1f3f6c2c81b26a632f495325f5e7a7de5a.1233751281.git.barvik@broadpark.no>
Kjetil Barvik schrieb:
> Currently inside write_entry() we do an lstat(path, &st) call on a
> file which have just been opened inside the exact same function. It
> should be better to call fstat(fd, &st) on the file while it is open,
> and it should be at least as fast as the lstat() method.
...
> @@ -145,6 +146,11 @@ static int write_entry(struct cache_entry *ce, char *path, const struct checkout
> }
>
> wrote = write_in_full(fd, new, size);
> + /* use fstat() only when path == ce->name */
> + if (state->refresh_cache && !to_tempfile && !state->base_dir_len) {
> + fstat(fd, &st);
> + fstat_done = 1;
> + }
> close(fd);
I've a bad gut feeling about this: It may not work as expected on Windows
because there is this statement in the documentation:
"The only guarantee about a file timestamp is that the file time is
correctly reflected when the handle that makes the change is closed."
(http://msdn.microsoft.com/en-us/library/ms724290(VS.85).aspx)
We are operating on a temporary file. It could happen that the fstat()
returns the time when the file was created, as opposed to when the
write_in_full() was completed successfully. The fstat()ed time ends up in
the index, but it can be different from what later lstat() calls report
(and the file would be regarded as modified).
I have the suspicion that the gain from this patch is minimal. Would you
mind playing it safe and drop this patch?
-- Hannes
next prev parent reply other threads:[~2009-02-04 14:51 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-02-04 12:52 [PATCH/RFC v3 0/9] git checkout: more cleanups, optimisation, less lstat() calls Kjetil Barvik
2009-02-04 12:52 ` [PATCH/RFC v3 1/9] lstat_cache(): small cleanup and optimisation Kjetil Barvik
2009-02-04 12:52 ` [PATCH/RFC v3 2/9] lstat_cache(): generalise longest_match_lstat_cache() Kjetil Barvik
2009-02-04 12:52 ` [PATCH/RFC v3 3/9] lstat_cache(): swap func(length, string) into func(string, length) Kjetil Barvik
2009-02-04 12:52 ` [PATCH/RFC v3 4/9] unlink_entry(): introduce schedule_dir_for_removal() Kjetil Barvik
2009-02-04 20:54 ` Junio C Hamano
2009-02-04 20:55 ` Junio C Hamano
2009-02-04 21:32 ` Kjetil Barvik
2009-02-04 12:52 ` [PATCH/RFC v3 5/9] create_directories(): remove some memcpy() and strchr() calls Kjetil Barvik
2009-02-04 12:52 ` [PATCH/RFC v3 6/9] write_entry(): cleanup of some duplicated code 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 [this message]
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
2009-02-04 12:53 ` [PATCH/RFC v3 8/9] show_patch_diff(): remove a call to fstat() Kjetil Barvik
2009-02-04 12:53 ` [PATCH/RFC v3 9/9] lstat_cache(): print a warning if doing ping-pong between cache types Kjetil Barvik
-- strict thread matches above, loose matches on Subject: below --
2009-02-05 10:46 [PATCH/RFC v3 7/9] write_entry(): use fstat() instead of lstat() when file is open 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=49899FA4.2020003@viscovery.net \
--to=j.sixt@viscovery.net \
--cc=barvik@broadpark.no \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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.