Repository navigation
Conversation
Since 090a308 (t/helper/test-chmtime: update mingw to support chmtime on directories, 2022-03-02), mingw_utime() opens the file with CreateFileW() and only FILE_WRITE_ATTRIBUTES access. Windows grants that access on read-only files; FILE_ATTRIBUTE_READONLY only denies writing the file's data and deleting it. Clearing the attribute first, and restoring it afterwards, dates from 852f098 (Windows: fix utime() for read-only files, 2010-03-30), when the file was opened O_RDWR, and is no longer needed. It is not free. Loose objects are read-only, and Git freshens an existing loose object with utime() whenever it is about to write an object it already has (for example `git add` of content that was added before, `git stash`, or a rebase that recreates blobs). Each call made two extra opens of the object file through SetFileAttributesW(), which on Windows means two more passes through the file system filter stack (on-access antivirus scanning in particular). Open the file first, and fall back to clearing the attribute only if the open is refused with ERROR_ACCESS_DENIED, so that a file system that does enforce the attribute for FILE_WRITE_ATTRIBUTES keeps working as before. Signed-off-by: Alexandre Zollinger Chohfi <alzollin@microsoft.com>
Author
|
The one failing job, It did the same on the re-run this morning. The same job passed on other pull requests a little later (the |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When Git is about to write an object that it already has, it freshens the existing file with
utime()instead. On Windows that happens a lot:git addof content that was added before,git stash,rebases that recreate blobs, and so on. Loose objects are read-only, and
mingw_utime()clearsFILE_ATTRIBUTE_READONLYbefore every call and restores it afterwards. Each freshen therefore opensthe object file three times instead of once.
The toggle comes from 852f098 (Windows: fix utime() for read-only files, 2010), when
utime()opened the file
O_RDWR. Since 090a308 (2022) it opens the file withCreateFileW()and onlyFILE_WRITE_ATTRIBUTESaccess, which Windows grants on read-only files. This PR tries the open first.It clears the attribute only if the open is refused with
ERROR_ACCESS_DENIED, so a file system thatenforces it keeps working as before.
Effect. Measured with
git add -Aafter rewriting 200 tracked files whose new content was alreadyin the object store (a 40,000-file repository, Git for Windows v2.55.0.windows.5 with and without this
patch, same toolchain). 20 interleaved runs, median, paired 95% CI:
Per freshened object, measured with the same Win32 calls outside Git, the cost drops from 159 µs to 63 µs
on the Dev Drive and from 460 µs to 206 µs on NTFS. Each freshen now makes one
NtCreateFile, and the twoNtOpenFile/NtSetInformationFilepairs are gone.Tests.
t6501-freshen-objects,t5304-prune,t3700-addandt7063-status-untracked-cachepass. Thefreshened objects keep
FILE_ATTRIBUTE_READONLY, and their mtime is updated.The same code is in upstream
compat/mingw.c(without long-path support). I can send it to themailing list too if you prefer it to go upstream first.