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

cache mock-up #43

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

cache mock-up #43

wants to merge 1 commit into from

Conversation

fluvf
Copy link
Contributor

@fluvf fluvf commented Dec 12, 2023

Accidentally based on #42
One monolithic commit as this is a mock-up
Notable changes needed to make this work:

  • Fallback IDs are now searched for in reverse
    Simplifies logic and allows subsequent non cached find_entry_paths to overwrite any set aliases (Not actually strictly needed?)
    It is, if cached find finds a duplicate entry that's of lower priority than one found during a full search, the higher priority entry is skipped No it's not, that entry alias would be unset
    Nevertheless, I will create a separate pull request with only this change later
  • find_entry_paths call and $FALLBACK_ENTRY_IDS was moved inside find_entry
    The function needs to see the masked find function set inside find_cached_entry, and the variable needs to be reset between calls
    There's actually no need for it to be a global any more

I don't know if I like sourceing the cache file, might change that to read loop when cleaning up, if you're happy with the implementation

@fluvf
Copy link
Contributor Author

fluvf commented Dec 15, 2023

Thoughts? Would you be happy with this implementation? I'd have time to clean this up today if you are

@Vladimir-csp
Copy link
Owner

Thank you. I will dig deeper in it a bit later. What I can see right away is that entry read is still needed.

With current cache implementation, exec line is read directly from the cache.

@fluvf
Copy link
Contributor Author

fluvf commented Dec 15, 2023

Yes, this isn't trying to be faster than the current implementation. Some quick tests prove that it's not.
But it is less code to maintain, and the cache should be much harder to invalidate

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