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

Add more options to memoization cache #3296

Merged
merged 6 commits into from
Dec 20, 2023
Merged

Add more options to memoization cache #3296

merged 6 commits into from
Dec 20, 2023

Conversation

serras
Copy link
Member

@serras serras commented Nov 10, 2023

Instead of rolling our own caches, this PR opens the door to using any library out there. Given our focus in Multiplatform, I've added a small bridge to cache4k, which seems to have all the interesting options (size, eviction policies, and so on).

Fixes #3133

@serras serras requested review from nomisRev, a team and raulraja November 10, 2023 10:28
@serras
Copy link
Member Author

serras commented Nov 10, 2023

@greyhairredbear do you think this PR would satisfy your needs from #3133?

Copy link
Contributor

github-actions bot commented Nov 10, 2023

@greyhairredbear
Copy link

@serras I suppose it does, at least cache4k does check the box for my use case. Many thanks for the implementation 👏 🙌

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

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

This is really great work @serras! 🙌
Sorry for the unreasonable delay on the reviews 🙈

arrow-libs/core/arrow-cache4k/build.gradle.kts Outdated Show resolved Hide resolved
Co-authored-by: Simon Vergauwen <nomisRev@users.noreply.github.com>
@serras serras merged commit 563a848 into main Dec 20, 2023
11 checks passed
@serras serras deleted the serras/memoization-cache branch December 20, 2023 10:48
serras added a commit to arrow-kt/arrow-website that referenced this pull request Feb 27, 2024
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.

["Request"] Add basic cache size configuration option to memoize()
4 participants