checkout.c: use fscache stat only in usable place#1443
checkout.c: use fscache stat only in usable place#1443atetubou wants to merge 1 commit intogit-for-windows:masterfrom
Conversation
|
|
||
| enable_delayed_checkout(&state); | ||
| enable_fscache(1); | ||
| enable_explicit_fscache(1); |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
c8c9e76 to
8e46ee3
Compare
builtin/checkout.c
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
entry.c
Outdated
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
2a76b6a to
c7d9fcf
Compare
This is fix for git-for-windows#1438, git-for-windows#1442 This patch introduces explicitly enabled fscache_lstat and fix the issue stat unintendedly returned from cache. I replace lstat with fscache_lstat in check_path called for every file in a repository. When we run `git checkout`, following two conditions are hold. * check_path is only called to get stat of file in working tree before checked out. * check_path is not called for the file after it is checked out. So cached stat when directory traversing does not affect returned value in check_path here. Signed-off-by: Takuto Ikuta <tikuta@chromium.org>
c7d9fcf to
ab641f4
Compare
|
👍. Thanks! |
|
Can't we introduce a |
|
I opted for the safe (if slow) way of reverting the original patch instead, with some information in the commit message and marked up as a |
The recently introduced speedup of `git checkout` sadly introduced a regression: after writing a file, the cached stat data is obviously incorrect, yet we still used it! This patch simply reverts the original speedup (as opposed to try to hotfix it as #1443 tries to do), and marks it as a fixup commit so that the next merging-rebase will drop the patch. We will most likely want to speed up this code path, though, but may find a better way, e.g. by invalidating the cached data cleverly. (This will be a *little* tricky, though, as readdir() uses the data, and we must ensure that we do not pull it under readdir()'s rag when invalidating it, and we also must avoid re-caching for every written file, as that would be *even* slower than fscache=false.) This fixes #1438 and #1442 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
|
As announced a couple of minutes ago, I decided to go for a revert instead. I am still very much in favor of pursuing a speedup via fscache in this code path, though! |
|
Thank you for revert. After breaking this largely used software, I'm afraid implicit usage of cached result. So I think explicit usage is safer and solid now. |
|
I didn't have this patch applied. Thanks. |
This is fix for #1438, #1442
This patch introduces explicitly enabled fscache_lstat and
fix the issue stat unintendedly returned from cache.
I replace lstat with fscache_lstat in check_path called for every file in a repository.
When we run
git checkout, following two conditions are hold.So cached stat when directory traversing does not affect returned value in check_path here.
Signed-off-by: Takuto Ikuta tikuta@chromium.org