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 support for preloading related data for missing collection entries #43

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

Conversation

Nerian
Copy link

@Nerian Nerian commented Apr 20, 2022

Hi,

This PR adds a preloads method to add a hook so we can preload relations in collections so that we can avoid n+1 issues

@adamcrown
Copy link
Member

Thanks @Nerian. I pushed up some changes to address a failing spec. It looks like it caused some merge conflicts. Sorry about that.

About the PR, I like the idea of this. However, I'm a bit concerned that it could have a negative impact on performance since there is so much logic being added, especially in the case where there is a cache hit. Would you be willing to do some performance comparisons, with and without this change?

@Nerian
Copy link
Author

Nerian commented Jun 7, 2022

We have been using this for a few months in production and it has worked fine. But I added more changes in some other branch so maybe I will rebase this branch with all these changes and submit again.

I like the idea of measuring performance difference between versions of cache-crispies. Is there a rake task I can use for that, or should I write one? Should I perhaps use this? https://github.com/codenoble/cache-crispies-performance-comparison

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