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

Edit run_onchange_03-install-packages.tmpl: reduce number of times chezmoi data executes #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

zipperer
Copy link

Claims I believe (each of which could be false and thereby make this commit wrong):

  • each call to get_config returns the same information (i.e. for each call the return value is the same)
  • filter(lambda p: matches_features(p, get_config()), to_install), calls get_config() for each entry in to_install
  • get_packages computes data, and data contains what get_config returns

Based on those claims, I propose:

  • get_packages returns both packages and config
  • in filter(lambda p: matches_features(p, get_config()), to_install), replace get_config() with config, where config was computed once before and is reused for each package p

…hezmoi data` executes

Claims I believe (each of which could be false and thereby make this commit wrong):
- each call to `get_config` returns the same information (i.e. for each call the return value is the same)
- `filter(lambda p: matches_features(p, get_config()), to_install)`, calls get_config() for each entry in `to_install`
- `get_packages` computes `data`, and `data` contains what `get_config` returns

Based on those claims, I propose:
 - `get_packages` returns both packages and data
 - `filter(lambda p: matches_features(p, config), to_install)` where config was computed once before and reused for each package `p`
Copy link
Owner

@JoshMock JoshMock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. This only runs every few days when I run chezmoi update, but an optimization is always a win, especially for a repo that's all about having fun with it. 😎

I'd personally probably memoize it using a global instead. Haven't run this so I don't know if it's perfect, but something like:

chez_config = None

def chezmoi_data():
    if not chez_config:
        result = subprocess.run(["chezmoi", "data"], capture_output=True)
        chez_config = json.loads(result.stdout)
    return chez_config

def get_config():
    return chezmoi_data()["chezmoi"]["config"]["data"]

def get_packages():
    return chezmoi_data()["packages"]

That way it just fetches it once and stores it in memory for the remainder of the process lifetime, and I don't have to return a tuple, which almost always (for me) requires me to go look at the function and figure out what it's actually returning. 😆

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

Successfully merging this pull request may close these issues.

2 participants