search

rubysherpas / forem

The best Rails 3 and Rails 4 forum engine. Ever.
http://forem.herokuapp.com
MIT License
1.55k stars 422 forks source link

Bullet`s N+1 warning #681

Open sintro opened 9 years ago

sintro commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

krainboltgreene commented 8 years ago

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 commented 8 years ago

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

krainboltgreene commented 8 years ago

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 https://github.com/rubysherpas/forem/issues/681#issuecomment-157209186.

c2169000 commented 8 years ago

How's this looking? And, could it be related to https://github.com/rubysherpas/forem/issues/697?