Edit run_onchange_03-install-packages.tmpl: reduce number of times chezmoi data executes#6
Open
zipperer wants to merge 1 commit intoJoshMock:mainfrom
Open
Edit run_onchange_03-install-packages.tmpl: reduce number of times chezmoi data executes#6zipperer wants to merge 1 commit intoJoshMock:mainfrom
chezmoi data executes#6zipperer wants to merge 1 commit intoJoshMock:mainfrom
Conversation
…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`
JoshMock
reviewed
Oct 19, 2023
Owner
JoshMock
left a comment
There was a problem hiding this comment.
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. 😆
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Claims I believe (each of which could be false and thereby make this commit wrong):
get_configreturns 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 into_installget_packagescomputesdata, anddatacontains whatget_configreturnsBased on those claims, I propose:
get_packagesreturns both packages and configfilter(lambda p: matches_features(p, get_config()), to_install), replaceget_config()withconfig, whereconfigwas computed once before and is reused for each packagep