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

Run modify scripts in parallel #2814

Open
VorpalBlade opened this issue Mar 3, 2023 · 7 comments
Open

Run modify scripts in parallel #2814

VorpalBlade opened this issue Mar 3, 2023 · 7 comments
Labels
enhancement New feature or request v3 Planned for v3

Comments

@VorpalBlade
Copy link
Contributor

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

I have a few modify scripts (to deal with KDE):

$ find . -name 'modify_*' | wc -l                           
15

and I have noticed the performance of operations like chezmoi diff seem to degrade linearly with the number of scripts.

I used strace to figure out what was going on, and to me it looks like chezmoi waits for each script to run before starting the next one. There is no reason modify scripts could not be run in parallel (as they do not affect each other). Even if the scripts are completely CPU bound this would be a big speedup (8x - 16x in my case, depending on the computer). In practise my scripts perform a mix of IO and CPU, so the speedup could be even bigger (or smaller).

Describe the solution you'd like

I propose that modify scripts should be executed in parallel by chezmoi. That would interfer with the proposed solution of using /dev/tty in #2244 (which I have not yet implemented) but that is a non-standard thing to do and could be solved with the scripts themselves using locking between themselves when they rarely need to interact with the terminal.

Describe alternatives you've considered

I considered rewriting my python script in Rust. This would undoubtedly speed it up, and I might end up doing this in the end anyway. It is however harder to distribute (need to have x86, x86-64, ARM32 and ARM64 versions for just my own personal needs for example).

My script does not however allow much internal parallelisation, so it would still not be able to make efficient use of all cores. Running multiple modify scripts in parallel would still be beneficial regardless.

Additional context

It would be interesting to think about if there are any other areas of chezmoi that would benefit from parallelisation as well, but I feel that is the topic of another issue.

@VorpalBlade VorpalBlade added the enhancement New feature or request label Mar 3, 2023
@twpayne
Copy link
Owner

twpayne commented Mar 3, 2023

This is similar to #2740.

chezmoi runs scripts sequentially in a strict order for transparency and repeatability. Writing scripts that perform correctly when run concurrently is extremely difficult, and running them concurrently is not repeatable.

You can run things concurrently in a single script, for an example see #2740 (comment).

@VorpalBlade
Copy link
Contributor Author

This is similar to #2740.

chezmoi runs scripts sequentially in a strict order for transparency and repeatability. Writing scripts that perform correctly when run concurrently is extremely difficult, and running them concurrently is not repeatable.

This might be true in the general case, and for run_ scripts I tend to agree. However the intent of modify_ scripts is that should be purely functional, and idempotent. E.g. they should read their input provided by chezmoi and possibly some configuration from the source state. In this case they are trivially to parallelise.

However, there is of course that someone doesn't do something silly in their modification scripts! Maybe the feature could be opt-in? Still I understand you are probably thinking about getting annoying bug reports from users who blame the problems on chezmoi itself...

You can run things concurrently in a single script, for an example see #2740 (comment).

This is true in general, but not for modify_ scripts. These scripts operate on a file by file basis. Given my specific problem (merging ini files with custom merge logic) it is very difficult to opportunities to parallelise within a file. However multiple files are entirely independent. But modify_ scripts cannot operate on multiple files at once.

The only option to parallelise my use case would be convert to a single run_ script that basically did all the work of:

  • Finding files to work on in the chezmoi source directory.
  • Processing those files.
  • Have an option to show a coloured diff to the user for those files (as chezmoi diff would not be able to help with that any more).

At that point all chezmoi is doing is calling my program, I'm essentially not able to reuse/piggy back on any functionality from chezmoi.

@twpayne
Copy link
Owner

twpayne commented Mar 3, 2023

Ah, thanks for the extra detail. I'd somehow missed that this was specific to modify_ scripts and not scripts in general.

This is effectively concurrent generation of the target state. It's something that I'd like to implement eventually but it will require a significant refactor/rewrite of chezmoi's internals. Currently the source state is generated concurrently, but the target state generation and application to the destination directory are not. So, it's likely a chezmoi version 3 feature.

In the short term, is it possible to replace your Python modify_ scripts with a chezmoi modify_ template (see also #2526)? These run entirely in-process in chezmoi, which potentially makes them much faster than external scripts.

@twpayne twpayne added the v3 Planned for v3 label Mar 3, 2023
@VorpalBlade
Copy link
Contributor Author

Ah, thanks for the extra detail. I'd somehow missed that this was specific to modify_ scripts and not scripts in general.

This is effectively concurrent generation of the target state. It's something that I'd like to implement eventually but it will require a significant refactor/rewrite of chezmoi's internals. Currently the source state is generated concurrently, but the target state generation and application to the destination directory are not. So, it's likely a chezmoi version 3 feature.

In the short term, is it possible to replace your Python modify_ scripts with a chezmoi modify_ template (see also #2526)? These run entirely in-process in chezmoi, which potentially makes them much faster than external scripts.

I very much doubt it (or it will be exceedingly silly and painful). My script parses a source copy of the file as well as the target state of the file as INI, then merges those according to rules. See https://github.com/VorpalBlade/chezmoi_modify_manager

I think the best course of action for me would be to rewrite this in a compiled language (i.e. Rust, I'm on that bandwagon currently) and then make the modify scripts not be bash scripts that call the rust program but instead do something like this chezmoi template as the modify_ "script":

#!/{{ .chezmoi.sourceDir }}/my_rust_binary_{{ .chezmoi.arch }}

src-file {{ .chezmoi.sourceFile }}.src.ini

ignore-section MainWindow
ignore-key "Open-with settings" History
ignore-key-re "section" "key_prefix_.*"
transform unsorted_list Aliases AliasList '{"separator": ","}'

That is, pretend to the system/chezmoi that my rust program is an "interpreter" of this file which is actually just the config file that describes how and what to merge.

I would love to not need to rely on chezmoi templating here (since presumably that takes some time as well), but I really don't see a way around it, without hard coding in the path of my home directory (which may vary between systems!). Plus I need different binaries for different architectures anyway.

This will likely be faster than a bash script that calls python (which is what currently happens). But the future concurrency support would be really nice, there is only so much I can do on my end. Right now chezmoi is quite slow, taking almost a second on this laptop that is a few years old (with an i7-8550U, so not a low end laptop by any means):

$ hyperfine "chezmoi diff"
Benchmark 1: chezmoi diff
  Time (mean ± σ):     898.6 ms ±  12.6 ms    [User: 743.0 ms, System: 164.3 ms]
  Range (min … max):   888.9 ms … 932.0 ms    10 runs

@twpayne
Copy link
Owner

twpayne commented Dec 1, 2023

Note to self: some template functions are not threadsafe, for example glob temporarily changes the working directory of the process. As modify_ scripts can be templates, care will be need to make either all template functions threadsafe or serialize execution of the templates.

@VorpalBlade
Copy link
Contributor Author

Note to self: some template functions are not threadsafe, for example glob temporarily changes the working directory of the process. As modify_ scripts can be templates, care will be need to make either all template functions threadsafe or serialize execution of the templates.

That seems odd, surely it would be sense to be able to execute a glob relative any directory, not just the current working directory?
Being able to run templates in parallel would also be useful in general I assume (though they might be quick enough that the threading overhead isn't worth it.

That said, expanding the templates however it is done currently followed by running the actual processes async concurrently should work, right? I don't know Go but I believe it has async support (just like Rust does)? At the OS level it would probably be something like a posix_spawn call (so you don't have to deal with the mess that is fork+exec in a multi-threaded program).

@twpayne
Copy link
Owner

twpayne commented Dec 1, 2023

That seems odd, surely it would be sense to be able to execute a glob relative any directory, not just the current working directory?

The glob template function is a wrapper around github.com/bmatcuk/doublestar/v4.Glob which interprets relative patterns as being relative to the current working directory. There's no way to tell it to assume a different working directory, and prefixing the pattern passed to it with the desired working directory has a number of undesired side-effects.

That said, expanding the templates however it is done currently followed by running the actual processes async concurrently should work, right? I don't know Go but I believe it has async support (just like Rust does)? At the OS level it would probably be something like a posix_spawn call (so you don't have to deal with the mess that is fork+exec in a multi-threaded program).

It's not just glob though. Many password manager-related functions have caches that need to be protected from concurrent access, and the password manager CLIs themselves will often behave differently if you're not already logged in. If you invoke the password manager CLI twice concurrently you may end up with two password prompts fighting over the same terminal. The complexities here are beyond the implementation language and fork/exec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v3 Planned for v3
Projects
None yet
Development

No branches or pull requests

2 participants