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

Incorporate lru-cache library for basis of async cache logic #34

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

cmdcolin
Copy link
Contributor

@cmdcolin cmdcolin commented Sep 6, 2024

This is a proposal to swap out the core async cache logic for lru-cache

lru-cache is a battle-tested library and would be beneficial to incorporate as the basis for our system, as it is hard to reason about the exact nature of async caching

This PR swaps out our logic for lru-cache by making "class AbortablePromiseCache extend LRUCache" (the class from lru-cache's library)

The hope would be to achieve less memory leaks and control memory usage better in jbrowse.

Because we extend the LRUCache class, this does change the API interface. "Wrapping" the class and exposing our own interface could allow us to keep our old interface

Note: I'm only acting on a hunch, not out of a specific memory leak that I observed....

@cmdcolin
Copy link
Contributor Author

cmdcolin commented Sep 6, 2024

one important note is that we retain almost full test compatibility with current master, and any test changes i made seemed to be essentially unneeded concepts to preserve

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.

1 participant