Re: [PATCH v4] read-cache: do not trust a size change when conversion is active

["Torsten Bögershausen" <tboegi@web.de>]

On Tue, Oct 13, 2026 at 12:58:28AM -0600, Curtis Allen Smith wrote:

Thanks for the fast and good answers everybody.
As a comment: no, I didn't intend to tak over the series -
just want to move things forward.
And the Helped-By is of course ok.

I don't know if we need more explanations why this change
is needed (after so long time), but here is my undestanding:

When we run


git init t && cd t
printf 'one\ntwo\nthree\n' >file.txt
git add . && git commit -m init

followed by

printf 'two\ntwo\nthree\n' >file.txt

then Git will notice that the actual  mtime as a result of lstat()
is different from what is recorded in the index.
As a result, git needs to re-hash the content to conclude if the
content has changed.
However, in order to avoid the expensive hashing,
Git concludes that when the size of a file has changed
then the content -must- be different - files having a different size
are never identical.
This did work well until the .gitattributes feature was introduced.
Now, in the example below, a file having a different size -can-
result in the same blob once it is commited.
Therefore this patch does need to check if attributes
(for input conversions) are defined.
If not, take the shortcut and conclude that the file is modified.
But if, take the long way, which is what happens when only mtime
(and possibly other elements from struct st are changed.

And now the new size, mtime (and the rest)
will be recorded in the index. So it is a one-time cost.

Does this make sense ?


> "git status" can report a file as modified while "git diff" and
> "git add" agree that its contents are unchanged:
> 
> 	git init t && cd t
> 	printf '* text eol=lf\n' >.gitattributes
> 	printf 'one\ntwo\nthree\n' >file.txt
> 	git add . && git commit -m init
> 
> 	printf 'one\r\ntwo\r\nthree\r\n' >file.txt
> 
> 	git status --short      # ->  M file.txt
> 	git diff                # -> empty
> 	git add file.txt        # -> nothing to commit
> 
> ie_modified() takes a change in the file's size since the index
> entry was last refreshed as proof that the file is modified, without
> looking at the contents.  A size change proves only that the bytes in
> the working tree changed.  When the path is subject to conversion
> (the "text" and "eol" attributes, core.autocrlf, a clean filter),
> different bytes can convert to the same blob, as the CRLF rewrite
> above does.
> 
> For such a path, fall through to ce_modified_check_fs(), as is
> already done when only the mtime changed.  Paths without conversion
> return early as before.
> 
> Helped-by: Torsten Bögershausen <tboegi@web.de>
> Signed-off-by: Curtis Allen Smith <curtis.allen.smith@gmail.com>
> ---
> Changes since v3:
>  - The fix is inlined into the condition in ie_modified(), with no
>    helper function.
>  - Shorter log message, using Junio's wording; the "real life"
>    examples are gone.
> 
> Cost, i9-14900K under WSL2, warm cache, fastest of 5 runs of
> "git status -uno", stock 6de20f6 against this patch:
> 
> 	10000 files x 2.6 KB, no conversion       stock      v4
> 	  clean tree                               5 ms     5 ms
> 	  10000 files with a size change          26 ms    24 ms
> 
> 	10000 files x 2.6 KB, "* text=auto eol=lf"
> 	  clean tree                               5 ms     5 ms
> 	  10000 genuinely modified                25 ms   137 ms
> 	  10000 CRLF-rewritten, 1st run           21 ms   143 ms
> 	  the same, every run after               21 ms     5 ms
> 
> 	200 files x 1 MB, "* text=auto eol=lf"
> 	  clean tree                               3 ms     3 ms
> 	  200 genuinely modified                   3 ms   498 ms
> 	  200 CRLF-rewritten, 1st run              3 ms   584 ms
> 	  the same, every run after                3 ms     3 ms
> 
> The attribute lookup happens only for a path whose size changed, so a
> clean tree pays nothing, and a tree without conversion pays one
> lookup per size-changed path (the first block).  A converted path
> whose size changed is now read, converted and hashed, which is what
> already happens when only its mtime changes.  Stock git keeps
> reporting the CRLF-rewritten files as modified on every run.
> 
> A follow-up that skips the conversion and the hash when the length
> already rules out a match is ready if wanted.  It takes the
> "genuinely modified" rows to 57 ms and 25 ms.

[snip]