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

Speed up yadm by caching results of git calls #505

Open
rasa opened this issue Nov 14, 2024 · 7 comments
Open

Speed up yadm by caching results of git calls #505

rasa opened this issue Nov 14, 2024 · 7 comments
Labels

Comments

@rasa
Copy link
Contributor

rasa commented Nov 14, 2024

Is your feature request related to a problem? Please describe.

yadm can be slow on slower systems.

Describe the solution you'd like

Cache expensive git calls.

Additional context

When I run yadm commit on my system, these are the commands that are run, and how often (found using GIT_TRACE=1):

  Count  Command
-------  ----------------------------------------- 
      2  git fetch --update-head-ok
      2  git maintenance run --auto --no-quiet
      2  git merge FETCH_HEAD
      2  git rev-parse --git-dir
      2  git rev-parse --git-path objects
      2  git rev-parse --show-prefix
      2  git rev-parse --show-toplevel
      2  git-submodule summary --cached --for-status --summary-limit -1 HEAD
      2  git-submodule summary --files --for-status --summary-limit -1
      4  git config --file=.config/yadm/config --bool yadm.alt-copy
      4  git config --file=.config/yadm/config --bool yadm.auto-alt
      4  git config --file=.config/yadm/config --bool yadm.auto-perms
      4  git config --file=.config/yadm/config --bool yadm.auto-private-dirs
      4  git config --file=.config/yadm/config --bool yadm.gpg-perms
      4  git config --file=.config/yadm/config --bool yadm.ssh-perms
      4  git config --file=.config/yadm/config yadm.git-program
      4  git config --get-all local.class
      4  git config core.worktree
      4  git config local.arch
      4  git config local.hostname
      4  git config local.os
      4  git config local.user
      4  git ls-files
      4  git-sh-i18n--envsubst 'usage: $dashless $USAGE'
      4  git-sh-i18n--envsubst --variables 'usage: $dashless $USAGE'

The results of at least the git config commands can be cached to speed things up, yes?

If you are open to this enhancement, I will draft a PR for your review.

@rasa rasa added the feature label Nov 14, 2024
@erijo
Copy link
Collaborator

erijo commented Nov 14, 2024

It would be interesting to see how much difference it makes if you replace the calls with hard coded values (i.e. the optimal case)?

@rasa
Copy link
Contributor Author

rasa commented Nov 15, 2024

In an iSH emulation environment, running a simple yadm command can take 45+ seconds with auto-alt and auto-perms enabled, and 7.5 seconds when they're disabled.

I will explore how this can be sped up.

#466 should help when auto-alt is enabled.

erijo added a commit that referenced this issue Nov 28, 2024
* Simplify score_file() by using case in instead of nested ifs with regexps.
* Merge record_score() and record_template().
* Alt condition processing no longer stops when a template condition is seen
  but continues processing to verify that all conditions are valid (as the
  documentation says it should). Fixes #478.
* Support alt dirs with deeply nested tracked files (fixes #495).
* Use git ls-files to filter out which tracked files to consider for alt
  processing. Should speed up auto-alt (#505).
@erijo
Copy link
Collaborator

erijo commented Nov 29, 2024

@rasa: If you have the time, please test with the deep-alt branch and let me know if that improved the performance.

erijo added a commit that referenced this issue Nov 29, 2024
* Simplify score_file() by using case in instead of nested ifs with regexps.
* Merge record_score() and record_template().
* Alt condition processing no longer stops when a template condition is seen
  but continues processing to verify that all conditions are valid (as the
  documentation says it should). Fixes #478.
* Support alt dirs with deeply nested tracked files (fixes #490).
* Use git ls-files to filter out which tracked files to consider for alt
  processing. Should speed up auto-alt (#505).
* Use nocasematch when comparing distro and distro_family. Fixed #455.
@rasa
Copy link
Contributor Author

rasa commented Nov 30, 2024

I ran yadm alt several times in an iSH VM, and consistently get:
master:

real    0m9.716s
user    0m4.294s
sys     0m0.000s

deep-alt:

real    0m4.896s
user    0m2.158s
sys     0m0.000s

so it's a 50% speed increase. Worth it!

@rasa
Copy link
Contributor Author

rasa commented Nov 30, 2024

No speed up on yadm perms, which always shows


real    0m0.693s
user    0m0.446s
sys     0m0.000s

erijo added a commit that referenced this issue Dec 5, 2024
* Simplify score_file() by using case in instead of nested ifs with regexps.
* Merge record_score() and record_template().
* Alt condition processing no longer stops when a template condition is seen
  but continues processing to verify that all conditions are valid (as the
  documentation says it should). Fixes #478.
* Support alt dirs with deeply nested tracked files (fixes #490).
* Use git ls-files to filter out which tracked files to consider for alt
  processing. Should speed up auto-alt (#505).
* Use nocasematch when comparing distro and distro_family. Fixed #455.
erijo added a commit that referenced this issue Dec 6, 2024
* Simplify score_file() by using case in instead of nested ifs with regexps.
* Merge record_score() and record_template().
* Alt condition processing no longer stops when a template condition is seen
  but continues processing to verify that all conditions are valid (as the
  documentation says it should). Fixes #478.
* Support alt dirs with deeply nested tracked files (fixes #490).
* Use git ls-files to filter out which tracked files to consider for alt
  processing. Should speed up auto-alt (#505).
* Use nocasematch when comparing distro and distro_family. Fixed #455.
@erijo
Copy link
Collaborator

erijo commented Dec 6, 2024

@rasa: Could you please test with the test-workflow branch? It now includes the fixes from the deep-alt branch + a change to how parse_encrypt is implemented which may affect the run time.

@rasa
Copy link
Contributor Author

rasa commented Dec 7, 2024

@erijo We lost about a second on iSH, using the test-workflow branch:

real    0m5.561s
user    0m2.191s
sys     0m0.000s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants