Eliminate redundant token joining and const_get calls #2655
Conversation
This file contains 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
When searching for loaded facts that match the user query, we were joining the query tokens once per loaded fact. For example, if the user query was `networking.interfaces.eth0`, then we split the query into tokens ['networking', 'interfaces', 'eth0']. Then for each loaded fact definition, we joined the query tokens back to 'networking.interfaces.eth0`. If none matched, then we tried again using the parent prefix ['networking', 'interfaces'], again joining for each loaded fact. Now only join the query tokens once outside of the method.
Rename loaded_facts_hash and loaded_fact to loaded_facts
joshcooper
force-pushed
the
perf_improvements
branch
from
8c90493 to
1fbfe5c
Compare
mhashizume
reviewed
Dec 8, 2023
|
Nit, but the wording of the commit message for ba58f16 seems like it has some editing artifacts in it |
joshcooper
force-pushed
the
perf_improvements
branch
from
1fbfe5c to
fab8c39
Compare
I updated this, hopefully it's clearer? |
|
Oh wait I see, I’ll fix that |
It's wasn't super obvious, but if `fact_with_wildcard` was true, then we returned true or false depending on whether query_fact didn't match the fact name exactly or not. Refactor the code to make it explicit. Some more context, some legacy facts are dynamically generated and their names contain wildcards, such as the `blockdevice_.*_model` legacy fact. If the user queries for `blockdevice_sda_model` specifically, then we need to return the legacy fact. In that case `fact_with_wildcard` will be true and the fact and query names match exactly, so `found_fact?` will return true. However, if you query for the similarly named structured fact `blockdevice.sda.model`, then `fact_with_wildcard` will be false, since we want to return the structured fact, not the legacy one.
joshcooper
force-pushed
the
perf_improvements
branch
from
fab8c39 to
a6cfedc
Compare
mhashizume
approved these changes
Dec 11, 2023
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.
While working on a spike to fix dotted notation, I noticed the following:
https://github.com/puppetlabs/facter/blob/1fbfe5c5dc8ca69b2c3e53823be0af9f2f6a5a4d/docs/data-flow.md