Junio C Hamano <gitster@xxxxxxxxx> writes: > Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes: <snip> >> 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) Ok, I admit I was not aware of this Windows fact. >> 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? > > Hmm, write_entry() is actually called once per one path we write out, and > the fstat() is added to the common case (no --tempfile, no --prefix=<dir> > checkout), Yes, I had to make sure that the path string and ce->name was the exact same string, so therefore I had to add the test "&& !to_tempfile && !state->base_dir_len" to the if-test. > which would mean that if there were any performance gain from > this change, it was obtained by trading correctness away. Sad. Sorry about this. Yes, I agree that we should drop this patch. 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 performance gain from this patch. Junio, is it OK to ask that you drop this patch if/when you update the pu branch, such that I do not have to resend the patch series almost unchanged to the mailinglist (except for one missing patch)? Ok, maybe wait one day, just in case there will be more comments. And, thanks for the review! -- kjetil -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
