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

Performance: Optimize authorized students query for lower-access users #2574

Merged
merged 1 commit into from Aug 29, 2019

Conversation

@kevinrobinson
Copy link
Contributor

kevinrobinson commented Aug 29, 2019

Who is this PR for?

educators

What problem does this PR fix?

Optimizes authorized { Student.all } style queries. These are used in the home page feed and my students pages, among other places. It came up working on some unrelated features that performance regressed here for folks who fall through to the later lines in why_authorized_for_student?

What does this PR do?

Essentially removes n+1 across students, and moves the query in this method into something that can be eagerly included or cached on the Educator model or collection, and adds a short-circuit case too.

Numbers on PerfTest.authorized are pretty clear:

somerville
  before median: 7627ms    p95: 14195ms   (1% sample)
  after  median:  540ms    p95:  1197ms   (1% sample)
  revert median: 7294ms    p95:  8967ms   (1% sample)
  after  median:  720ms    p95:  6625ms  (10% sample)

new_bedford
  before median: 30805ms   p95: 43297ms   (1% sample)
  after  median:  1125ms   p95:  1762ms   (1% sample)
  after  median:  1527ms    p95: 2693ms  (10% sample)

bedford
  before median: 1719ms    p95: 2143ms   (1% sample)
  after  median:   99ms    p95:  172ms   (1% sample)
  after  median:  101ms     p95: 163ms  (50% sample)

Checklists

Which features or pages does this PR touch?

  • Home page
  • My Students
  • Core

Does this PR use tests to help verify we can deploy these changes quickly and confidently?

  • Included specs for changes
  • Manual testing made more sense here
@kevinrobinson

This comment has been minimized.

Copy link
Contributor Author

kevinrobinson commented Aug 29, 2019

selfie

@kevinrobinson kevinrobinson merged commit 6b36285 into master Aug 29, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@kevinrobinson kevinrobinson deleted the patch/educator-label-optimization branch Aug 29, 2019
@kevinrobinson kevinrobinson changed the title Authorized students: Optimize query for lower-access users Performance: Optimize authorized students query for lower-access users Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.