For paginated listings, Drupal often executes a COUNT query before the data query.

When the data query returns fewer rows than the limit, the COUNT is unnecessary. The total is already known.

The proposed change is to run the data query first, and to only run the COUNT query if the number of results equals the limit, meaning there could be more pages.

I see quite a few callers of EntityQueryBase::initializePager():

  1. EntityListBuilder (base class for most entity admin pages, inherited by UserListBuilder, MediaListBuilder, BlockContentListBuilder, PathAliasListBuilder, ModeratedNodeListBuilder, etc.)
  2. VersionHistoryController
  3. CommentAdminOverview
  4. PrepareModulesEntityUninstallForm
  5. NodeController

Any optimization would benefit all these.

Issue fork drupal-3580381

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

dries created an issue. See original summary.

dries’s picture

dries commented
Issue tags: +Performance
Related issues: +#3580382: Skip unnecessary COUNT query in PagerSelectExtender::execute()
dries’s picture

dries commented
Issue summary: View changes
dries’s picture

dries commented

I know very little about the entity system's internals, but I created a draft MR to test an approach. It might be wrong. For example, I'm not sure yet if there are hooks or access checks that might interfere. However, I have to put this on hold for at least a week, focus on finishing my DriesNote, and take a flight to Chicago first thing in the morning.

Either way, the code is pretty simple. It overrides initializePager() to defer the COUNT query, then calls finalizePager() from result() after the data query returns. If results don't fill the page, we skip the COUNT. This leaves Config and KeyValueStore entity queries untouched since their "COUNT" just counts an in-memory array.

I believe this benefits all entity list builders: content types, users, media, blocks, path aliases, version history, comment admin, node controller, etc.

I don't believe the existing entity tests have tests for this. I don't see any code that tests the correctness of the pager.

I also looked at whether any of the performance tests would catch the improvement, but I don't believe they test any admin pages with entity list builders. So the query savings may not show up.

dries opened merge request !15150

dries’s picture

dries commented
Status: Active » Needs review

I'd love a review.

smustgrave’s picture

smustgrave commented

Left 1 small comment that may not be needed but rest seems fine to me

dries’s picture

dries commented
Status: Needs review » Reviewed & tested by the community

Thanks for the review, @smustgrave! I switched to descriptive variable names and added a block comment explaining the logic. I believe this is ready.

Btw, #3579381: Skip unnecessary COUNT query in Views Sql::execute() is the same optimization for Views. If you've reviewed this one, that one should be a quick review too. Just sayin' ...