Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#17522 closed enhancement (fixed)

ms-blogs dynamic strings to static strings

Reported by: niallkennedy Owned by: nacin
Milestone: 3.2 Priority: normal
Severity: normal Version:
Component: Multisite Keywords: has-patch
Focuses: Cc:


wp-includes/ms-blogs.php builds some option key strings with dynamic string components that should instead be static strings. Speed.

Attachments (2)

ms-blogs.diff (1.5 KB) - added by niallkennedy 4 years ago.
static strings
string-test.php (1.6 KB) - added by niallkennedy 4 years ago.
single vs. double quoted string tests based on options use

Download all attachments as: .zip

Change History (11)

@niallkennedy4 years ago

static strings

comment:1 @scribu4 years ago

When you cite speed as a reason, you'd better have some benchmarks handy.

Please don't open any more tickets of this type until you can prove that they provide a non-trivial performance boost.

comment:2 @scribu4 years ago

  • Keywords reporter-feedback needs-testing added

comment:3 @scribu4 years ago

Sorry for the rather aggressive tone, but I get annoyed when time is spent on useless optimizations.

Please provide some tests.

comment:4 @scribu4 years ago

  • Keywords close added; needs-testing removed

@niallkennedy4 years ago

single vs. double quoted string tests based on options use

comment:5 @niallkennedy4 years ago

Initial patch corrects for WordPress Coding Standards for strings:

If you're not evaluating anything in the string, use single quotes.

Variables and escape sequences for special characters are not expanded when they occur in single-quoted strings. So you would think not having to look for extra things within a string declaration would process more quickly. We can run some tests and confirm the performance angle. string-test.php added to ticket. In my PHP 5.3 CLI tests the single quoted strings were faster. "Non-trivial" up for debate.

Patch is admittedly trivial compared to rewriting Media or something similar. Yet if I'm already inside a file and notice something not to Coding Standards or capable of making a few million websites run nanoseconds faster I figure why not flag the diff and improve the software.

comment:6 @scribu4 years ago

  • Resolution set to duplicate
  • Status changed from new to closed

A couple of things:

1) Patches that just fix coding standards are discouraged. They can invalidate other patches that actually deal with code logic, or even with inline docs, while providing a fraction of the benefit.

2) If patches are very similar, they should be uploaded to a single ticket.

Going to close all such tickets as duplicates of #17522, since that is the oldest one.

Feel free to re-upload all the patches there.

Version 1, edited 4 years ago by scribu (previous) (next) (diff)

comment:7 @scribu4 years ago

  • Milestone Awaiting Review deleted

comment:8 @nacin4 years ago

  • Owner set to nacin
  • Resolution changed from duplicate to fixed

In [17988]:

Single quotes and spaces in ms-blogs.php. props niallkennedy, fixes #17522.

comment:9 @nacin4 years ago

  • Keywords reporter-feedback close removed
  • Milestone set to 3.2
Note: See TracTickets for help on using tickets.