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.

Sign up for GitHub

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

Jump to bottom

[#2329] categorisation game performance #2362

Closed
wants to merge 5 commits into from
Closed

[#2329] categorisation game performance #2362

wants to merge 5 commits into from

Conversation

garethrees
Copy link
Member

Initial pass at #2329

Hotfix in to master and wdtk so that we can profile on newrelic

@mysociety-pusher mysociety-pusher force-pushed the 2329-categorisation-game-performance branch from 70d0bf0 to 39c938d Compare April 22, 2015 11:34
@garethrees
Copy link
Member Author

Build failing until pboling/rack-insight#42 is merged released to rubygems.

@garethrees
Copy link
Member Author

Updated with some more commits.

|              | Original (7d68d91) | New (9f752d6) |
|--------------|--------------------|---------------|
| Full Request | 11080ms            | 4270ms        |
| Templates    | 9150ms             | 3160ms        |

@crowbot
Copy link
Member

crowbot commented Apr 23, 2015

Looks like bullet is incompatible with ruby 1.8.7 https://travis-ci.org/mysociety/alaveteli/jobs/59692871

@garethrees
Copy link
Member Author

Looks like bullet is incompatible with ruby 1.8.7 https://travis-ci.org/mysociety/alaveteli/jobs/59692871

Ah, yeah I had to change that. I've got it noted down to submit a PR to revert flyerhzm/bullet@a484c0c

Failing that we can just install it for 1.9 given that 1.8 is soon to be dropped

@garethrees
Minor tidy up
4837af5
@garethrees
Reduce N+1 queries in RequestGameController#play
594318e 
Eliminates the following Bullet warnings:

2015-04-21 11:42:26[WARN] /categorise/play
N+1 Query detected
user: vagrant
InfoRequest => [:public_body]
Add to your finder: :includes => [:public_body]
N+1 Query method call stack
  /home/vagrant/alaveteli/app/views/request/_request_listing_single.html.erb:16:in `_app_views_request__request_listing_single_html_erb__949797960_70091194573720'
  /home/vagrant/alaveteli/app/views/request_game/play.html.erb:46:in `_app_views_request_game_play_html_erb___279307748_70091194720080'
  /home/vagrant/alaveteli/app/views/request_game/play.html.erb:45:in `each'
  /home/vagrant/alaveteli/app/views/request_game/play.html.erb:45:in `_app_views_request_game_play_html_erb___279307748_70091194720080'
  /home/vagrant/alaveteli/app/controllers/application_controller.rb:111:in `record_memory'
  /home/vagrant/alaveteli/lib/whatdotheyknow/strip_empty_sessions.rb:14:in `call'

2015-04-21 11:42:26[WARN] /categorise/play
N+1 Query detected
user: vagrant
InfoRequest => [:user]
Add to your finder: :includes => [:user]
N+1 Query method call stack
  /home/vagrant/alaveteli/app/views/request/_request_listing_single.html.erb:17:in `_app_views_request__request_listing_single_html_erb__949797960_70091194573720'
  /home/vagrant/alaveteli/app/views/request_game/play.html.erb:46:in `_app_views_request_game_play_html_erb___279307748_70091194720080'
  /home/vagrant/alaveteli/app/views/request_game/play.html.erb:45:in `each'
  /home/vagrant/alaveteli/app/views/request_game/play.html.erb:45:in `_app_views_request_game_play_html_erb___279307748_70091194720080'
  /home/vagrant/alaveteli/app/controllers/application_controller.rb:111:in `record_memory'
  /home/vagrant/alaveteli/lib/whatdotheyknow/strip_empty_sessions.rb:14:in `call'
@garethrees
Improve performance initializing OutgoingMessage
b10f2a0 
Reduces the amount of queries to associated objects by directly checking the
database before setting the default body content.

Before

> info_request.outgoing_messages
  OutgoingMessage Load (0.4ms)  SELECT "outgoing_messages".* FROM "outgoing_messages" WHERE "outgoing_messages"."info_request_id" = 137 ORDER BY created_at
  InfoRequest Load (0.5ms)  SELECT "info_requests".* FROM "info_requests" WHERE "info_requests"."id" = 137 LIMIT 1
  PublicBody Load (0.5ms)  SELECT "public_bodies".* FROM "public_bodies" WHERE "public_bodies"."id" = 16 LIMIT 1
  HasTagString::HasTagStringTag Load (0.4ms)  SELECT "has_tag_string_tags".* FROM "has_tag_string_tags" WHERE "has_tag_string_tags"."model_id" = 16 AND (model = 'PublicBody')
  CensorRule Load (0.4ms)  SELECT "censor_rules".* FROM "censor_rules" WHERE "censor_rules"."info_request_id" IS NULL AND "censor_rules"."public_body_id" IS NULL AND "censor_rules"."user_id" IS NULL
  User Load (0.4ms)  SELECT "users".* FROM "users" WHERE "users"."id" = 13 LIMIT 1
   (0.5ms)  SELECT COUNT(*) FROM "censor_rules" WHERE "censor_rules"."user_id" = 13
  CensorRule Load (0.5ms)  SELECT "censor_rules".* FROM "censor_rules" WHERE "censor_rules"."info_request_id" = 137 ORDER BY created_at desc
  CensorRule Load (0.5ms)  SELECT "censor_rules".* FROM "censor_rules" WHERE "censor_rules"."public_body_id" = 16 ORDER BY created_at desc
  InfoRequest Load (0.5ms)  SELECT "info_requests".* FROM "info_requests" WHERE "info_requests"."id" = 137 LIMIT 1
  PublicBody Load (0.5ms)  SELECT "public_bodies".* FROM "public_bodies" WHERE "public_bodies"."id" = 16 LIMIT 1
  HasTagString::HasTagStringTag Load (0.6ms)  SELECT "has_tag_string_tags".* FROM "has_tag_string_tags" WHERE "has_tag_string_tags"."model_id" = 16 AND (model = 'PublicBody')
  CensorRule Load (0.5ms)  SELECT "censor_rules".* FROM "censor_rules" WHERE "censor_rules"."info_request_id" IS NULL AND "censor_rules"."public_body_id" IS NULL AND "censor_rules"."user_id" IS NULL
  User Load (0.6ms)  SELECT "users".* FROM "users" WHERE "users"."id" = 13 LIMIT 1
   (0.5ms)  SELECT COUNT(*) FROM "censor_rules" WHERE "censor_rules"."user_id" = 13
  CensorRule Load (0.4ms)  SELECT "censor_rules".* FROM "censor_rules" WHERE "censor_rules"."info_request_id" = 137 ORDER BY created_at desc
  CensorRule Load (0.5ms)  SELECT "censor_rules".* FROM "censor_rules" WHERE "censor_rules"."public_body_id" = 16 ORDER BY created_at desc
=> [#<OutgoingMessage id: 36, info_request_id: 137, body: "Some information please", status: "sent", message_type: "initial_request", created_at: "2015-04-17 10:50:21", updated_at: "2015-04-17 10:50:21", last_sent_at: "2015-04-17 10:50:21", incoming_message_followup_id: nil, what_doing: "normal_sort", prominence: "normal", prominence_reason: nil>, #<OutgoingMessage id: 37, info_request_id: 137, body: "Dear Example Public Body 6,\r\n\r\nc'mooooooonnnnnn\r\n\r\n...", status: "sent", message_type: "followup", created_at: "2015-04-20 12:18:30", updated_at: "2015-04-20 12:18:30", last_sent_at: "2015-04-20 12:18:30", incoming_message_followup_id: 30, what_doing: "normal_sort", prominence: "normal", prominence_reason: nil>]

After

> info_request.outgoing_messages
  OutgoingMessage Load (0.8ms)  SELECT "outgoing_messages".* FROM "outgoing_messages" WHERE "outgoing_messages"."info_request_id" = 137 ORDER BY created_at
=> [#<OutgoingMessage id: 36, info_request_id: 137, body: "Some information please", status: "sent", message_type: "initial_request", created_at: "2015-04-17 10:50:21", updated_at: "2015-04-17 10:50:21", last_sent_at: "2015-04-17 10:50:21", incoming_message_followup_id: nil, what_doing: "normal_sort", prominence: "normal", prominence_reason: nil>, #<OutgoingMessage id: 37, info_request_id: 137, body: "Dear Example Public Body 6,\r\n\r\nc'mooooooonnnnnn\r\n\r\n...", status: "sent", message_type: "followup", created_at: "2015-04-20 12:18:30", updated_at: "2015-04-20 12:18:30", last_sent_at: "2015-04-20 12:18:30", incoming_message_followup_id: 30, what_doing: "normal_sort", prominence: "normal", prominence_reason: nil>]
@garethrees
Only load the outgoing messages that will be used
bfe3724 
Prevents loading of all outgoing messages related to the info request.

Before

OutgoingMessage Load (1.5ms)  SELECT "outgoing_messages".* FROM
"outgoing_messages" WHERE "outgoing_messages"."info_request_id" = 137
ORDER BY created_at

After

OutgoingMessage Load (0.8ms)  SELECT "outgoing_messages".* FROM
"outgoing_messages" WHERE "outgoing_messages"."info_request_id" = 137
ORDER BY created_at LIMIT 1
@garethrees
Tidy InfoRequest#initial_request_text
7c11248
@mysociety-pusher mysociety-pusher force-pushed the 2329-categorisation-game-performance branch from 9f752d6 to 7c11248 Compare April 27, 2015 11:32
@garethrees
Copy link
Member Author

Extracted out the dev tools and some of the other PRs

@garethrees
Copy link
Member Author

This PR is now all 👍'd commits – others have been extracted

@coveralls
Copy link

coveralls commented Apr 27, 2015 •
edited
Loading

Coverage Status

Coverage decreased (-0.003%) to 87.926% when pulling 7c11248 on 2329-categorisation-game-performance into 0b91fc3 on master.

@crowbot
Copy link
Member

crowbot commented Apr 27, 2015

👍

@garethrees
Copy link
Member Author

First spike is pre-deploy; second bump is post-deploy ⚡

screen shot 2015-04-27 at 15 23 57

@garethrees
Copy link
Member Author

Still on ~2500ms, but much better than 86k 👍

screen shot 2015-04-27 at 15 26 39

@garethrees
Copy link
Member Author

Think we're good to merge this to master?

@crowbot
Copy link
Member

crowbot commented Apr 27, 2015

👍

@garethrees
Copy link
Member Author

Merged in 2f2415d

@garethrees garethrees closed this Apr 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@garethrees @crowbot @coveralls