Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mingw: improve performance of mingw_unlink() #2589

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

jeffhostetler
Copy link

@jeffhostetler jeffhostetler commented Apr 17, 2020

Update mingw_unlink() to first try to delete the file with existing
permissions before trying to force it.

Windows throws an error when trying to delete a read-only file. The
mingw_unlink() compatibility wrapper always tries to _wchmod(666) the
file before calling _wunlink() to avoid that error. However, since
most files in the worktree are already writable, this is usually
wasted effort.

Update mingw_unlink() to just call DeleteFileW() directly and if that
succeeds return. If that fails, fall back into the existing code path
to update the permissions and use _wunlink() to get the existing
error code mapping.

Signed-off-by: Jeff Hostetler jeffhost@microsoft.com

Update mingw_unlink() to first try to delete the file with existing
permissions before trying to force it.

Windows throws an error when trying to delete a read-only file.  The
mingw_unlink() compatibility wrapper always tries to _wchmod(666) the
file before calling _wunlink() to avoid that error.  However, since
most files in the worktree are already writable, this is usually
wasted effort.

Update mingw_unlink() to just call DeleteFileW() directly and if that
succeeds return.  If that fails, fall back into the existing code path
to update the permissions and use _wunlink() to get the existing
error code mapping.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it! :shipit:

@dscho
Copy link
Member

dscho commented Apr 18, 2020

I'd like to submit this directly upstream via GGG after we've had a
chance to review it and avoid the technical debt.

@jeffhostetler I think we should do both. Are you totally opposed to having it in Git for Windows early? It's such a trivial patch, it will be simply skipped from the rebase once it arrives in any official Git version.

@jeffhostetler
Copy link
Author

@dscho I’m ok with including in both. i was just trying to not cause you more work later. i’ll update and merge this and send one to GGG and the list on monday. thanks.

@dscho
Copy link
Member

dscho commented Apr 18, 2020

Excellent!

@derrickstolee
Copy link

@jeffhostetler any reason you didn't create a PR against vfs-2.26.1 in microsoft/git? I could generate an installer and run it through some perf tests that way.

@jeffhostetler jeffhostetler changed the title [DO NOT MERGE] mingw: improve performance of mingw_unlink() mingw: improve performance of mingw_unlink() Apr 20, 2020
@jeffhostetler
Copy link
Author

@derrickstolee I'll merge this into GFW as is. Then cherry-pick it into vfs so we can test it there. And then send a patch to the mailing list via GGG.

@jeffhostetler jeffhostetler merged commit 06995d4 into git-for-windows:master Apr 20, 2020
dscho added a commit to git-for-windows/build-extra that referenced this pull request Apr 20, 2020
Worktree updates (e.g. `git checkout`, `git
reset --hard`) [got a performance boost in sparse
checkouts](git-for-windows/git#2589).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
git-for-windows-ci pushed a commit that referenced this pull request Apr 20, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Apr 20, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Apr 20, 2020
mingw: improve performance of mingw_unlink()
derrickstolee pushed a commit to microsoft/git that referenced this pull request Apr 20, 2020
…f-gfw

mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Apr 21, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Apr 21, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 1, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 4, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 4, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 10, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 10, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 10, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 11, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 11, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 11, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 11, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 11, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 11, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 12, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 12, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 12, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 12, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 13, 2020
mingw: improve performance of mingw_unlink()
git-for-windows-ci pushed a commit that referenced this pull request Aug 13, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 17, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 18, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Aug 19, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Sep 17, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Sep 21, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Sep 23, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Sep 23, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Sep 24, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Sep 29, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit that referenced this pull request Oct 2, 2020
mingw: improve performance of mingw_unlink()
dscho pushed a commit to dscho/git that referenced this pull request Oct 5, 2020
…f-gfw

mingw: improve performance of mingw_unlink()
@jeffhostetler jeffhostetler deleted the unlink-perf-gfw branch October 20, 2020 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants