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

Bullet`s N+1 warning #681

Open
sintro opened this issue Sep 1, 2015 · 7 comments
Open

Bullet`s N+1 warning #681

sintro opened this issue Sep 1, 2015 · 7 comments

Comments

@sintro
Copy link

sintro commented Sep 1, 2015

Just began to work with this gem, and noticed the warning form Bullet gem

  # N+1 Query detected
  # Forem::Category => [:forums]
  # Add to your finder: :includes => [:forums]
  # N+1 Query method call stack

This warning was right on categories index (main page of forem) and I am not sure on other actions there will not be same warnings. As forum is not main feature of my project, I am leaving my investigations on this problem for later, but still post this issue here. Did anybody face this problem, or it is just Bullet`s paranoia?

Update: Bullet was correct.

@radar
Copy link
Collaborator

radar commented Sep 1, 2015

Patches welcome to fix this and any other issues like it.

On 1 Sep 2015, at 19:46, sintro notifications@github.com wrote:

Just began to work with this gem, and noticed the warning form Bullet gem

N+1 Query detected

Forem::Category => [:forums]

Add to your finder: :includes => [:forums]

N+1 Query method call stack

This warning was right on categories index (main page of forem), and I am not sure on other actions there will not be same warnings. As forum is not main feature of my project, I am leaving my investigations on this problem for later, but still post this issue here. Did anybody face this problem, or it is just Bullet`s paranoia?


Reply to this email directly or view it on GitHub.

@sintro
Copy link
Author

sintro commented Sep 2, 2015

Just quick-checked how the main forum page rendered, and looks like Bullet is absolutely right here. What I noticed is calling "render category.forums" there, while the view got from controller only @categories collection without included associations. So for every category it will make additional query to get its' forums (and may be then some additional queries for every forum to render something associated with them, didn't check that). It can be fixed by adding something like .includes(:forums) to controller's actions, like here (not sure that it will be exactly this way) @categories = Forem::Category.by_position.includes(:forums)

I don't have time now to make all checking and fixing, may be someone who has working forum and who has wish to fix this eager loading problem will do it? We need to check all controllers and queries for N+1 query, it can be done with https://github.com/flyerhzm/bullet gem or manually, then we need to fix ActiveRecord queries with correct methods (.includes() or .joins())

@radar
Copy link
Collaborator

radar commented Sep 2, 2015

Patches welcome to fix this and any other issues like it.

@krainboltgreene
Copy link

Hey @sintro I'll be looking into this when I can. Is there any chance that you'll want to work on this or pair on this together?

Also, editing your comment for clarity.

@phillyslick
Copy link

@krainboltgreene I can help out. I did some preliminary fixes, but much much more to do.

@krainboltgreene
Copy link

Awesome!
On Nov 16, 2015 3:34 PM, "phillyslick" notifications@github.com wrote:

@krainboltgreene https://github.com/krainboltgreene I can help out. I
did some preliminary fixes, but much much more to do.


Reply to this email directly or view it on GitHub
#681 (comment).

@c2169000
Copy link

How's this looking? And, could it be related to #697?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants