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

Loading circular relationships #20

Closed
sheldond opened this issue May 2, 2019 · 5 comments
Closed

Loading circular relationships #20

sheldond opened this issue May 2, 2019 · 5 comments

Comments

@sheldond
Copy link

sheldond commented May 2, 2019

Hi there! Thank you for publishing this gem, it's solving a big issue!

I've run into a performance issue when loading lazy relationships that are circular. I suspected that maybe you've run into, or thought about, this issue yourself. So I thought I would reach out and see!

When serializers define circular relationships it appears like the loader will follow the circle and visit the same records multiple times (via load_all_lazy_relationships). The evaluator has a guard against more than 3 nested levels which prevents this circular loading from becoming an infinite loop. Feel free to let me know if my understanding is correct or not!

The recursive circular loading has an exponential effect, each loop requires more objects to be instantiated at an exponential rate, so given enough records it becomes slower than the original n+1 queries it's eliminating.

I had some ideas of what might be done to improve the situation, but I was super curious if you had run into this and if so what your thoughts were on how to approach it. I know that some APIs don't use any circular relationships, which is of course one way to solve the issue, but it might not work in my situation.

Thanks for taking the time!

@Bajena
Copy link
Owner

Bajena commented May 5, 2019

Hey, thanks for posting this issue. You found a nice case! I didn't yet have time to check it but I'll try writing a few test cases this week and will let you know:)

@Bajena
Copy link
Owner

Bajena commented May 16, 2019

@sheldond I spent some time trying to reproduce your issue.
I think there can be two things I may have done wrong:

  1. In the default Association loader I wrote an "optimization" to skip loading association if a record's association was already loaded and instantly return the association records. This will cause lower level's associations to start preloading.
    It seems that ActiveRecord::Associations::Preloader is smart enough not to load the data if not necessary.

  2. I've used records as BatchLoader's keys in lazy loaders instead. This will cache data by object's reference. This means that if you'll load e.g. post -> comments -> post -> comments the comments will be loaded twice.
    Fixing this should be simple for SimpleHasMany and SimpleHasOne loaders - just use record.id or record.foreign_key_id as BatchLoader's cache key, but will require some more thinking for Association loader.

I drafted quickly improvements for issue no.1 and for SimpleHasMany/SimpleBelongsTo in this branch
https:/Bajena/ams_lazy_relationships/compare/circular?expand=1

Check out if performance is better for you and let me know! Also it'd be cool to see how your serializers look like.

@sheldond
Copy link
Author

@Bajena Thank you so much for your reply! This is still an open issue on my end, when I circle back to that item I'll look into your changes in detail and get back to you!

@sheldond
Copy link
Author

@Bajena Hello again!

Great news, I believe your changes in 1.3.0 have fully resolved the issue!

I wrote a test to measure the performance difference as the amount of relationship data increases when rendering the circular relationship. Previously the difference was exponential and now it's very linear. 👍

Thanks again very much for the help on this! It should be going out to production next week. 😃

@Bajena
Copy link
Owner

Bajena commented May 25, 2019

Yess!💪

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

No branches or pull requests

2 participants