Improve performance of the dashboard and user page sidebar items.

Review Request #13365 — Created Oct. 22, 2023 and submitted

Information

Review Board
release-5.0.x

Reviewers

When computing state for the dashboard, we fetch a list of all review
groups and then we fetch all starred review groups (excluding any IDs
we already fetched). On the user page, we fetch the user's list of
accesible review groups.

In each case, we unconditionally factor in Local Sites, whether or not
they're used on the server. We fetch all group state, even though we
don't need it all. We sort in the database. And on the dashboard, we
unconditionally check for starred review groups, even though we have
modern state for tracking whether they even have any.

This change optimizes the sidebar item computation to do the following:

  1. Use LocalSite.objects.build_q() to only filter by Local Site if
    Local Sites are used on the server.

  2. We now fetch the minimum amount of fields we need from the groups
    for each use.

  3. We sort in Python rather than in the database, freeing up the
    database to give us the results in any order that's needed. This is
    going to be a small list, so it's cheap to do.

  4. The dashboard sidebar now skips the starred review groups if we have
    computed state showing the user doesn't have any.

This results in simpler queries every time the dashboard or user page
loads, and reduces the queries for most dashboard users.

The existing dashboard unit tests have been updated to clear out any
starred review groups, to keep tests normalized and to avoid any
optional state in the computations. The exception is the sidebar test,
which does include joined groups.

Unit tests pass.

Tested viewing the dashboard and user pages.

Summary ID
Improve performance of the dashboard and user page sidebar items.
When computing state for the dashboard, we fetch a list of all review groups and then we fetch all starred review groups (excluding any IDs we already fetched). On the user page, we fetch the user's list of accesible review groups. In each case, we unconditionally factor in Local Sites, whether or not they're used on the server. We fetch all group state, even though we don't need it all. We sort in the database. And on the dashboard, we unconditionally check for starred review groups, even though we have modern state for tracking whether they even have any. This change optimizes the sidebar item computation to do the following: 1. Use `LocalSite.objects.build_q()` to only filter by Local Site if Local Sites are used on the server. 2. We now fetch the minimum amount of fields we need from the groups for each use. 3. We sort in Python rather than in the database, freeing up the database to give us the results in any order that's needed. This is going to be a small list, so it's cheap to do. 4. The dashboard sidebar now skips the starred review groups if we have computed state showing the user doesn't have any. This results in simpler queries every time the dashboard or user page loads, and reduces the queries for most dashboard users.
bc34e37a3f9f986a7eb199b8b66691a180ef405d
Description From Last Updated

Could you add a Yields: and typing while you're here?

maubinmaubin

Can this fit on one line? Same comment for all the other self._prefetch_cached calls.

maubinmaubin
david
  1. Ship It!
  2. 
      
maubin
  1. 
      
  2. reviewboard/datagrids/builtin_items.py (Diff revision 1)
     
     
     
    Show all issues

    Could you add a Yields: and typing while you're here?

    1. Looks like I had this in my branch but not posted.

  3. Show all issues

    Can this fit on one line? Same comment for all the other self._prefetch_cached calls.

    1. I have it one-per-line to ease future addition, as there's going to be more state passed in over time as more things are made cacheable. Sort of like keeping a dictionary item per line.

  4. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (fc9fd96)
Loading...