Remove N+1 queries

[tbcooney]

For example:

Started GET "/" for 127.0.0.1 at 2018-12-02 16:21:05 -0500
Processing by JobsController#index as HTML
  Job Load (4.4ms)  SELECT  "jobs".* FROM "jobs" WHERE "jobs"."published_at" IS NOT NULL ORDER BY "jobs"."published_at" DESC LIMIT $1 OFFSET $2  [["LIMIT", 30], ["OFFSET", 0]]
  ↳ app/controllers/jobs_controller.rb:22
  User Load (1.1ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT $2  [["id", 13], ["LIMIT", 1]]
  ↳ app/controllers/jobs_controller.rb:24
  Impression Exists (1.3ms)  SELECT  1 AS one FROM "impressions" WHERE "impressions"."impressionable_id" = $1 AND "impressions"."impressionable_type" = $2 AND "impressions"."session_hash" = $3 LIMIT $4  [["impressionable_id", 705], ["impressionable_type", "Job"], ["session_hash", "d80d52dd401011a626d600167140e49f"], ["LIMIT", 1]]
  ↳ app/controllers/jobs_controller.rb:24
  Impression Exists (0.6ms)  SELECT  1 AS one FROM "impressions" WHERE "impressions"."impressionable_id" = $1 AND "impressions"."impressionable_type" = $2 AND "impressions"."session_hash" = $3 LIMIT $4  [["impressionable_id", 704], ["impressionable_type", "Job"], ["session_hash", "d80d52dd401011a626d600167140e49f"], ["LIMIT", 1]]
  ↳ app/controllers/jobs_controller.rb:24
  Impression Exists (0.4ms)  SELECT  1 AS one FROM "impressions" WHERE "impressions"."impressionable_id" = $1 AND "impressions"."impressionable_type" = $2 AND "impressions"."session_hash" = $3 LIMIT $4  [["impressionable_id", 703], ["impressionable_type", "Job"], ["session_hash", "d80d52dd401011a626d600167140e49f"], ["LIMIT", 1]]
  ↳ app/controllers/jobs_controller.rb:24

I'm using the impressionist method directly, taking all of the Jobs on the page and logging an impression. The problem is because I'm only recording unique impressions, for records that already have an impression, these additional calls are redundant. I tried to use @jobs.includes(:impressions).each to preload the associated data hoping Rails was smart enough to figure out which records existed, but Rails still outputs numerous Impression Exists queries.

@jobs.each{|job| impressionist(job,'', :unique => [:session_hash])}

[johnmcaliley]

It must be related to the way we are using polymorphic associations:

impressionist/lib/impressionist/setup_association.rb

Line 16 in 41a209b

receiver.belongs_to(:impressionable, :polymorphic => true, :optional => true)

In the controller functions we are calling the impressions on the impressionable model instance, which I expect would take into consideration the includes statement, but I guess that is not the case.

impressionist/app/controllers/impressionist_controller.rb

Line 90 in 41a209b

return unique_opts.blank? || !impressionable.impressions.where(unique_query(unique_opts, impressionable)).exists?

I'm not sure if this is an ActiveRecord limitation or just the way we are using it. I'll have to do some more research. But you may be able to do this manually in your code to avoid the N+1 by using the includes that you mentioned and then when you iterate over your @jobs you could do a check against the impressions to make sure the session_hash is not in any of them before making the impressionist() call. Since the N+1 is happening somewhere in the impressionist() call, I'm hoping calling this before and excluding the unique parameter will avoid this. Something like this untested pseudo-code:

session_hash = request.session_options[:id]
@jobs.includes(:impressions).each do |job|
  unless job.impressions.any{|impression| impression.session_hash == session_hash}
    #remove the unique parameter
    impressionist(job,'')
  end
end

Can you try something like that and let me know if you are still seeing the N+1 calls to the impressions table? Also, when you were using the includes in your test, did it actually issue a secondary query to pull all impressions for the @jobs?

[tbcooney]

Good idea @johnmcaliley. I iterated on the impressionist() call and it slimed down the requests by a lot, this looks much better. Regarding my previous use of includes, it did issue a secondary query to pull all impressions for @jobs (on that page). Again though, this iteration looks much better.

It also recognizes new impressions and renders them (tested in another browser with a cleared cache and cookies).

Started GET "/" for 127.0.0.1 at 2018-12-06 23:23:01 -0500
Processing by JobsController#index as HTML
  Job Load (1.7ms)  SELECT  "jobs".* FROM "jobs" WHERE "jobs"."published_at" IS NOT NULL ORDER BY "jobs"."published_at" DESC LIMIT $1 OFFSET $2  [["LIMIT", 30], ["OFFSET", 0]]
  ↳ app/controllers/jobs_controller.rb:22
  NewsEntry Load (0.7ms)  SELECT  "news_entries".* FROM "news_entries" ORDER BY random() LIMIT $1  [["LIMIT", 3]]
  ↳ app/models/news_entry.rb:4
  CACHE Job Load (0.0ms)  SELECT  "jobs".* FROM "jobs" WHERE "jobs"."published_at" IS NOT NULL ORDER BY "jobs"."published_at" DESC LIMIT $1 OFFSET $2  [["LIMIT", 30], ["OFFSET", 0]]
  ↳ app/controllers/jobs_controller.rb:27
  Impression Load (6.6ms)  SELECT "impressions".* FROM "impressions" WHERE "impressions"."impressionable_type" = $1 AND "impressions"."impressionable_id" IN ($2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23, $24, $25, $26, $27, $28, $29, $30, $31)  [["impressionable_type", "Job"], ["impressionable_id", 705], ["impressionable_id", 704], ["impressionable_id", 702], ["impressionable_id", 701], ["impressionable_id", 700], ["impressionable_id", 699], ["impressionable_id", 698], ["impressionable_id", 697], ["impressionable_id", 696], ["impressionable_id", 695], ["impressionable_id", 694], ["impressionable_id", 693], ["impressionable_id", 692], ["impressionable_id", 691], ["impressionable_id", 690], ["impressionable_id", 689], ["impressionable_id", 688], ["impressionable_id", 687], ["impressionable_id", 686], ["impressionable_id", 685], ["impressionable_id", 684], ["impressionable_id", 683], ["impressionable_id", 682], ["impressionable_id", 681], ["impressionable_id", 680], ["impressionable_id", 679], ["impressionable_id", 678], ["impressionable_id", 677], ["impressionable_id", 676], ["impressionable_id", 675]]
  ↳ app/controllers/jobs_controller.rb:27
  Rendering jobs/index.html.haml within layouts/application
   (0.7ms)  SELECT COUNT(*) FROM "jobs" WHERE "jobs"."type" IN ('InternalJob')
  ↳ app/helpers/application_helper.rb:37
   (0.4ms)  SELECT COUNT(*) FROM "jobs" WHERE "jobs"."type" IN ('FetchedJob')
  ↳ app/helpers/application_helper.rb:37
   (0.7ms)  SELECT COUNT(*) FROM "jobs" WHERE "jobs"."published_at" IS NOT NULL
  ↳ app/views/jobs/index.html.haml:97
  Rendered jobs/index.html.haml within layouts/application (77.1ms)
  Rendered layouts/_meta_tags.html.haml (6.7ms)
  Rendered shared/_amplitude.html.haml (2.7ms)
  Rendered shared/_google_analytics.html.haml (1.7ms)
  Rendered shared/_footer.html.haml (7.4ms)
Completed 200 OK in 279ms (Views: 144.9ms | ActiveRecord: 10.8ms)