Make WordPress Core

Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#17522 closed enhancement (fixed)

ms-blogs dynamic strings to static strings

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

Description

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 13 years ago.
static strings
string-test.php (1.6 KB) - added by niallkennedy 13 years ago.
single vs. double quoted string tests based on options use

Download all attachments as: .zip

Change History (11)

@niallkennedy
13 years ago

static strings

#1 @scribu
13 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.

#2 @scribu
13 years ago

  • Keywords reporter-feedback needs-testing added

#3 @scribu
13 years ago

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

Please provide some tests.

#4 @scribu
13 years ago

  • Keywords close added; needs-testing removed

@niallkennedy
13 years ago

single vs. double quoted string tests based on options use

#5 @niallkennedy
13 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.

#6 @scribu
13 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 #16474, since that is the oldest one.

Feel free to re-upload all the patches there.

Last edited 13 years ago by scribu (previous) (diff)

#7 @scribu
13 years ago

  • Milestone Awaiting Review deleted

#8 @nacin
13 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.

#9 @nacin
13 years ago

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