SetLockableReadonly performance with no lockable files

[eps1lon]

Describe the issue

I noticed that our post-checkout was taking a fairly long time when doing a git-checkout. It was almost always taking roughly 3.5s¹. Once I ran git config lfs.SetLockableReadonly false that time got almost entirely eliminated (post-checkout only taking 0.08s).

What was confusing to me is the fact that lfs.locksverify was turned off and that the repository had no lockable files (empty git ls-files | git check-attr --stdin lockable | awk -F': ' '$3 ~ /set/ { print $1 }') to begin with.

Should this option then not have any performance impact? What are the implications of having SetLockableReadonly turned off when no files are lockable and lfs.locksverify is also turned off? Should git-lfs maybe warn if lfs.SetLockableReadonly is true when there are no lockable files?

¹ GIT_TRACE_PERFORMANCE=true git checkout master printing 14:45:49.833603 trace.c:411 performance: 3.583822000 s: git command: git lfs post-checkout 197348d8c4d497186cac6df28d52c80f7cdee2d8 e8868500b8d664ec8eb5159013fa3950404bca90 1

System environment

  System:
    OS: macOS 13.4.1
    CPU: (10) x64 Apple M1 Pro
    Memory: 793.52 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh

Output of git lfs env

git-lfs/3.3.0 (GitHub; darwin arm64; go 1.19.3)
git version 2.37.1 (Apple Git-137.1)

Endpoint=https://stash.int.klarna.net/klapp/klarna-app.git/info/lfs (auth=basic)
  [email protected]:/klapp/klarna-app.git
LocalWorkingDir=/Users/sebastian.silbermann/klapp3
LocalGitDir=/Users/sebastian.silbermann/klapp/.git/worktrees/klapp3
LocalGitStorageDir=/Users/sebastian.silbermann/klapp/.git
LocalMediaDir=/Users/sebastian.silbermann/klapp/.git/lfs/objects
LocalReferenceDirs=
TempDir=/Users/sebastian.silbermann/klapp/.git/lfs/tmp
ConcurrentTransfers=8
TusTransfers=false
BasicTransfersOnly=false
SkipDownloadErrors=false
FetchRecentAlways=false
FetchRecentRefsDays=7
FetchRecentCommitsDays=0
FetchRecentRefsIncludeRemotes=true
PruneOffsetDays=3
PruneVerifyRemoteAlways=false
PruneRemoteName=origin
LfsStorageDir=/Users/sebastian.silbermann/klapp/.git/lfs
AccessDownload=basic
AccessUpload=basic
DownloadTransfers=basic,lfs-standalone-file,ssh
UploadTransfers=basic,lfs-standalone-file,ssh
GIT_EXEC_PATH=/Applications/Xcode.app/Contents/Developer/usr/libexec/git-core
git config filter.lfs.process = "git-lfs filter-process"
git config filter.lfs.smudge = "git-lfs smudge -- %f"
git config filter.lfs.clean = "git-lfs clean -- %f"

Additional context

170k checked in files 24k checked into lfs

$ git ls-files | wc -l          
  172655
$ git lfs ls-files | wc -l
  24441	

.gitattributes:

*.pbxproj -text
clients/**/*.jpg filter=lfs diff=lfs merge=lfs -text
clients/**/*.jpeg filter=lfs diff=lfs merge=lfs -text
clients/**/*.png filter=lfs diff=lfs merge=lfs -text
clients/**/*.gif filter=lfs diff=lfs merge=lfs -text
clients/**/*.mov filter=lfs diff=lfs merge=lfs -text
clients/**/*.webp filter=lfs diff=lfs merge=lfs -text
deploy/*.png filter=lfs diff=lfs merge=lfs -text
deploy/teams/*.png filter=lfs diff=lfs merge=lfs -text
docs/favicon.ico filter=lfs diff=lfs merge=lfs -text
*.ttf filter=lfs diff=lfs merge=lfs -text
*-amd64 filter=lfs diff=lfs merge=lfs -text
*.mp4 filter=lfs diff=lfs merge=lfs -text
*.wav filter=lfs diff=lfs merge=lfs -text
*.pdf filter=lfs diff=lfs merge=lfs -text
*.sketch filter=lfs diff=lfs merge=lfs -text
*.tar.gz filter=lfs diff=lfs merge=lfs -text
*.jar filter=lfs diff=lfs merge=lfs -text
docker/cw-exporter/files/cloudwatch_exporter filter=lfs diff=lfs merge=lfs -text
bin/shellcheck_* filter=lfs diff=lfs merge=lfs -text
websites/kleco-docs/**/*.jp* filter=lfs diff=lfs merge=lfs -text
websites/kleco-docs/**/*.png filter=lfs diff=lfs merge=lfs -text
websites/kleco-docs/**/*.gif filter=lfs diff=lfs merge=lfs -text
websites/kleco-docs/**/*.mov filter=lfs diff=lfs merge=lfs -text
websites/kleco-docs/**/*.mp* filter=lfs diff=lfs merge=lfs -text
websites/kleco-docs/**/*.pdf* filter=lfs diff=lfs merge=lfs -text
services/consumer-banking-onboarding/**/*.webp filter=lfs diff=lfs merge=lfs -text

[chrisd8088]

Hey, thanks for the detailed report. It does seem like it would be preferable if we could avoid the performance penalty somehow. I'll mark this as a potential enhancement.

What are the implications of having SetLockableReadonly turned off when no files are lockable and lfs.locksverify is also turned off?

Off the top of my head, I don't think there should be any.

It's intriguing to me that the check for no lockable patterns doesn't have the same performance effect in your case as the check for the lfs.setlockablereadonly configuration. If you have a way to investigate that, that would be much appreciated!

[eps1lon]

Maybe lockClient.GetLockablePatterns() is the slow part? I suspect that this crawls all folders for a potential .gitattributes which grows with the size of the repository. But even git ls-files '*.gitattributes' only takes <0.2 so it wouldn't explain the 3.5s we see. Maybe git-lfs checks all potential locations not just the checked in filesß

[chrisd8088]

Indeed, I wondered something along the same lines, but so far I have not reproduced a noticeable lag even with a large repository (170k files) with a lot of Git history. I believe lockClient.GetLockablePatterns() is only looking for .gitattributes files in the working tree; it will then parse those that it finds. That should be relatively fast unless one's disk I/O is quite slow.

If you are by any chance able to further analyze what's going on with your situation, that would be very helpful, but I understand if that's not possible.

What I would do if I had your repository to hand is add some simple debugging output to the post-checkout code (e.g., just writing out temp files at different points with os.WriteFile() or something), run make build (assuming a recent Go is installed), and then edit my post-checkout Git hook so it points to the built git-lfs binary, like:

#!/bin/sh
command -v git-lfs >/dev/null 2>&1 || { echo >&2 "\nThis repository is configured for Git LFS but 'git-lfs' was not found on your path. If you no longer wish to use Git LFS, remove this hook by deleting .git/hooks/post-checkout.\n"; exit 2; }
## git lfs post-checkout "$@"
/path/to/bin/git-lfs post-checkout "$@"

But I appreciate if that's not feasible, and if we can get a reproduction case going ourselves, so much the better.

[eps1lon]

After manually adding some tracerx.PerformanceSince and drilling down, I noticed that it's simpler to leverage GIT_TRACE=1 git lfs post-checkout "$@":

22:16:38.112759 trace git-lfs: exec: git 'version'
22:16:38.131615 trace git-lfs: exec: git '-c' 'filter.lfs.smudge=' '-c' 'filter.lfs.clean=' '-c' 'filter.lfs.process=' '-c' 'filter.lfs.required=false' 'rev-parse' '--git-dir' '--show-toplevel'
22:16:38.148728 trace git-lfs: exec: git 'config' '--includes' '-l'
22:16:38.167329 trace git-lfs: exec: git 'rev-parse' '--is-bare-repository'
22:16:38.186345 trace git-lfs: exec: git 'config' '--includes' '-l' '-f' '/Users/sebastian.silbermann/klapp/.lfsconfig'
22:16:38.206134 trace git-lfs: exec: git '-c' 'filter.lfs.smudge=' '-c' 'filter.lfs.clean=' '-c' 'filter.lfs.process=' '-c' 'filter.lfs.required=false' 'rev-parse' 'HEAD' '--symbolic-full-name' 'HEAD'
22:16:38.230946 trace git-lfs: NewLsFiles: running in /Users/sebastian.silbermann/klapp git ls-files -z --cached --sparse --exclude-standard --others
22:16:38.231010 trace git-lfs: exec: git '-c' 'filter.lfs.smudge=' '-c' 'filter.lfs.clean=' '-c' 'filter.lfs.process=' '-c' 'filter.lfs.required=false' 'ls-files' '-z' '--cached' '--sparse' '--exclude-standard' '--others'
22:16:43.581444 trace git-lfs: findAttributeFiles: located .gitattributes
22:16:43.581481 trace git-lfs: findAttributeFiles: located clients/.gitattributes
22:16:43.581484 trace git-lfs: findAttributeFiles: located clients/packages/features/form-filling/.gitattributes
22:16:43.581489 trace git-lfs: findAttributeFiles: located clients/packages/features/web-coordinator-sdk/.gitattributes
22:16:43.581490 trace git-lfs: findAttributeFiles: located clients/packages/features/web-coordinator-sdk/_etc/.gitattributes
22:16:43.581496 trace git-lfs: findAttributeFiles: located services/deals-directory/.gitattributes
22:16:43.581500 trace git-lfs: findAttributeFiles: located services/enrichment-worker/.gitattributes
22:16:43.581501 trace git-lfs: findAttributeFiles: located services/faq/.gitattributes
22:16:43.581503 trace git-lfs: findAttributeFiles: located services/logistics-tracking-api/.gitattributes

Looks to me like git '-c' 'filter.lfs.smudge=' '-c' 'filter.lfs.clean=' '-c' 'filter.lfs.process=' '-c' 'filter.lfs.required=false' 'ls-files' '-z' '--cached' '--sparse' '--exclude-standard' '--others' is the bottleneck here.

Once I only consider tracked files when searching for .gitattributes files, we only spend .2s instead of 4s searching for .gitattributes.

[chrisd8088]

Interesting! That git ls-files is presumably coming from here, which implies to my mind that your git lfs post-checkout command is going past the GetLockablePatterns() step and proceeding to try to set file write permissions either here or here. Which would make sense, as that should be a much more intensive step than just looking for .gitattributes files, which is what the GetLockablePatterns() step does.

So I wonder what is different in your repo that's causing it to find (or think it's found) some relevant patterns? Do you have any other .gitattributes files besides the top-level one, for instance?

[chrisd8088]

Ah, no, I was wrong -- we also run that git ls-files command here, so it is run during the GetLockablePatterns() step.

So ignore what I said in my previous comment!

[chrisd8088]

And I do also see the same range of time difference with and without --others:

time git ls-files --cached --sparse --exclude-standard --others >/dev/null
real	0m2.791s
user	0m0.801s
sys	0m1.829s

$ time git ls-files --cached --sparse --exclude-standard  >/dev/null
real	0m0.085s
user	0m0.059s
sys	0m0.022s

So I guess the question is whether we need to look for untracked .gitattributes files, and specifically in the post-checkout stage. If not, we could take out that option.

[eps1lon]

I also filed a git but report to confirm it's expected that git ls-files --others --exclude-standard does not use core.untrackedCache. A repeated git status is fast but a git ls-files --others --exclude-standard is not, which does seem odd to me.