WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 3 years ago

#25735 closed defect (bug) (fixed)

paginate_links runs number_format_i18n for the entire range of pages

Reported by: johnpbloch Owned by: johnbillion
Milestone: 3.9 Priority: normal
Severity: normal Version: 2.7
Component: General Keywords: has-patch
Focuses: template, performance Cc:

Description

It does this regardless of whether they are actually used.

On large sites this adds significant overhead when paginate_links() is used on the front page.

Attachments (4)

25735.test.patch (877 bytes) - added by johnpbloch 4 years ago.
Unit Test
25735.patch (1.4 KB) - added by johnpbloch 4 years ago.
Patch
25735.test.2.patch (990 bytes) - added by johnpbloch 4 years ago.
Slightly better test
25735.2.patch (2.3 KB) - added by johnpbloch 4 years ago.
Unified patch

Download all attachments as: .zip

Change History (11)

@johnpbloch
4 years ago

Unit Test

@johnpbloch
4 years ago

Patch

#1 @johnpbloch
4 years ago

This is a bad ticket.

To reproduce:

Use paginate_links in such a way that it will not use the whole range of pages passed to it.

Expected behavior: it will run number_format_i18n() only when the number is actually used.

Actual behavior: it runs number_format_i18n() for all numbers regardless of whether they're used.

@johnpbloch
4 years ago

Slightly better test

#2 @johnpbloch
4 years ago

  • Keywords has-patch added

I've added a failing test and a potential solution.

@johnpbloch
4 years ago

Unified patch

#3 @johnpbloch
4 years ago

As a matter of curiosity, Nacin had wondered what performance looks like before and after applying the patch. Here are two cachegrind profiles, one from before and one from after:

Before

After

This was on an installation with just north of 22,000 posts in the database. At that size, this defect added .1 second to the request and number_format_i18n() had a total self cost of 6.67% and a total inclusive cost of 11.69%. This is entirely in keeping with the data I saw when I first discovered the issue on a database with 110,000+ posts in the database.

#4 @nacin
3 years ago

  • Component changed from General to Performance
  • Milestone changed from Awaiting Review to 3.9

#5 @nacin
3 years ago

  • Component changed from Performance to Template
  • Focuses performance added

#6 @nacin
3 years ago

  • Component changed from Template to General
  • Focuses template added

#7 @johnbillion
3 years ago

  • Owner set to johnbillion
  • Resolution set to fixed
  • Status changed from new to closed

In 27523:

Improve paginate_links() performance by not calling number_format_i18n() unnecessarily. Fixes #25735 with tests. Props johnpbloch.

Note: See TracTickets for help on using tickets.