Reduce review request detail queries for diff and comment queries.

Review Request #14315 — Created Jan. 30, 2025 and updated

Information

Review Board
release-7.1.x

Reviewers

When rendering content for diffs backed by hosting services, we ended up
needing access to the parent DiffSet, its Repository, and that
repository's HostingServiceAccount. We ended up doing
HostingServiceAccount queries far too often, since we couldn't reuse
any fetched state.

To address this, we're now re-attaching the repository on each DiffSet
we load, so long as it's the same repository ID. We also attach the
DiffSet back to each diff comment, completing that chain. We had
processing code like this for file attachments and screenshots, so we
now have similar code for diff comments.

To reduce overhead, the per-comment processing code has moved into
local functions, which can just be provided to our prepared data for the
comment processing loop. This avoids an isinstance check per-comment.

While there, work was done to limit how many comments we have to
process. We no longer check screenshot comments if we don't have
screenshots, file attachment comments if we don't have file attachments,
or diff comments if we don't have a repository.

And finally, some code operating on DiffSet IDs has been simplified to
reuse existing state in one place and to avoid an unnecessary query in
another.

Unit tests pass.

Checked queries using Django Debug Toolbar. Verified that these
redundant queries were eliminated.

Summary ID
Reduce review request detail queries for diff and comment queries.
When rendering content for diffs backed by hosting services, we ended up needing access to the parent `DiffSet`, its `Repository`, and that repository's `HostingServiceAccount`. We ended up doing `HostingServiceAccount` queries far too often, since we couldn't reuse any fetched state. To address this, we're now re-attaching the repository on each `DiffSet` we load, so long as it's the same repository ID. We also attach the `DiffSet` back to each diff comment, completing that chain. We had processing code like this for file attachments and screenshots, so we now have similar code for diff comments. To reduce overhead, the per-comment processing code has moved into local functions, which can just be provided to our prepared data for the comment processing loop. This avoids an `isinstance` check per-comment. While there, work was done to limit how many comments we have to process. We no longer check screenshot comments if we don't have screenshots, file attachment comments if we don't have file attachments, or diff comments if we don't have a repository. And finally, some code operating on `DiffSet` IDs has been simplified to reuse existing state in one place and to avoid an unnecessary query in another.
8a6388da3506c5e470538edeeb3f1dc0769b049c
Checks run (2 succeeded)
flake8 passed.
JSHint passed.
david
  1. Ship It!
  2. 
      
maubin
  1. Ship It!
  2.