WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#23855 closed enhancement (fixed)

Leave Shortcode functions early, if there's no Shortcode delimiter

Reported by: TobiasBg Owned by: nacin
Milestone: 3.9 Priority: normal
Severity: normal Version:
Component: Shortcodes Keywords: has-patch
Focuses: performance Cc:

Description

The main Shortcode processing functions (like do_shortcode()) all make several function calls to build the Shortcode RegEx, with a lot of array handling and string concatenation, before finally using a comparably expensive preg_replace() - and that's for every post that is shown on a page, even if there's no Shortcode in the post.

With a quick check for the opening bracket [, that is required for every Shortcode, we can skip that extra work and leave those functions early to save some processing time.

The attached patch adds that to do_shortcode(), strip_shortcodes(), and has_shortcode().

(Hat tip to bobbingwhite, who suggested this in this comment before, but it didn't gain traction as the ticket dealt with a different aspect of Shortcodes.)

Attachments (5)

23855-shortcodes.patch (1.1 KB) - added by TobiasBg 7 years ago.
Leave Shortcode functions early if there can not be a Shortcode in the content
23855.test.php (3.5 KB) - added by TobiasBg 6 years ago.
Measure execution time of shortcode functions
23855.2.patch (997 bytes) - added by TobiasBg 6 years ago.
Refreshed patch
23855.3.patch (1.1 KB) - added by TobiasBg 6 years ago.
Fixed patch
23855.test.2.php (4.1 KB) - added by TobiasBg 6 years ago.
Extended test/measuring file

Download all attachments as: .zip

Change History (17)

@TobiasBg
7 years ago

Leave Shortcode functions early if there can not be a Shortcode in the content

#1 @nacin
7 years ago

I wonder how much processing this saves.

#2 @TobiasBg
7 years ago

Good question. I'm not really sure what the best strategy is to reliably test this for a real-world scenario. bobbingwhite seems to have done some simple testing in this comment, with the functions being 4 to 5 times faster.

Given how many themes add several Shortcodes nowadays, just the many function evaluations in get_shortcode_regex() (especially the array_map(...)) might already have potential for savings. The PREG function is probably even more expensive than that, and saving that might also have potential.
This is increased by the fact, that get_shortcode_regex() is called for every single post/page, so usually at least ten times on most blog pages - another factor of 10 for savings.

#3 @kovshenin
7 years ago

  • Cc kovshenin added

#4 @jdgrimes
7 years ago

  • Cc jdg@… added

@TobiasBg
6 years ago

Measure execution time of shortcode functions

@TobiasBg
6 years ago

Refreshed patch

#5 @TobiasBg
6 years ago

  • Focuses performance added

I finally had some time to refresh the patch (for develop.svn., and with if-{}) and also to write a quick script to measure the execution time of the suggested change: 23855.test.php. Just run this on the patched and unpatched versions of shortcodes.php (by simply putting 23855.test.php in src/).
It runs the three functions in question on a dummy text with two Shortcodes that appear twice, and on a dummy text that does not have Shortcodes.

Text with ShortcodesText without Shortcodes
has_shortcode()0.298s — unpatched0.297s — unpatched
0.409s — 23855.2.patch0.259s — 23855.2.patch
do_shortcode()13.379s — unpatched4.497s — unpatched
13.309s — 23855.2.patch0.253s — 23855.2.patch
strip_shortcode()7.524s — unpatched4.431s — unpatched
7.521s — 23855.2.patch0.244s — 23855.2.patch

The patched version appears to be much faster for do_shortcode() and strip_shortcode() (factor 15 to 20) and not so much with has_shortcode(), in texts without Shortcodes. For texts with Shortcodes, the execution time remains pretty much constant.
So, adding this check to has_shortcode() might not be necessary.

#6 @TobiasBg
6 years ago

  • Milestone changed from Awaiting Review to 3.9

#7 @SergeyBiryukov
6 years ago

My results are similar, but the patched has_shortcode() appears to be slower both with and without shortcodes:

Text with ShortcodesText without Shortcodes
has_shortcode()1.121s — unpatched1.133s — unpatched
1.725s — 23855.2.patch2.374s — 23855.2.patch
do_shortcode()41.693s — unpatched13.464s — unpatched
43.025s — 23855.2.patch2.353s — 23855.2.patch
strip_shortcodes()21.834s — unpatched13.405s — unpatched
22.650s — 23855.2.patch2.345s — 23855.2.patch

So, yeah, the check should only be added to do_shortcode() and strip_shortcodes().

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

#8 @nacin
6 years ago

It would be good to test these patches, they have a *slight* problem.

#9 @SergeyBiryukov
6 years ago

do_shortcode() and strip_shortcodes() should obviously return $content rather than false. That doesn't affect the tests in 23855.test.php though, so the results are still relevant.

Last edited 6 years ago by SergeyBiryukov (previous) (diff)

@TobiasBg
6 years ago

Fixed patch

@TobiasBg
6 years ago

Extended test/measuring file

#10 follow-up: @TobiasBg
6 years ago

Ok, that's somewhat embarrassing now... I tested the functionality of this change on the (correct) original patch 23855-shortcodes.patch, but failed to repeat that after refreshing coding standards in 23855.2.patch. My bad, sorry. Won't happen again.

So, 23855.3.patch is the fixed patch.
Additionally, I fixed/extended the test file 23855.test.2.php a little bit, as it was actually testing the uncommon scenario of a non-existing Shortcode in its has_shortcode() test.

Here are updated measurements (100,000 instead of 500,000 runs though) on a different server than my measurements from above:

Text with ShortcodesText without Shortcodes
has_shortcode() 2.824s — unpatched1.745s — unpatched
+($tag exists) 2.896s — 23855.3.patch0.131s — 23855.3.patch
has_shortcode() 0.119s — unpatched0.123s — unpatched
+($tag exists) 0.178s — 23855.3.patch0.132s — 23855.3.patch
do_shortcode() 6.411s — unpatched1.924s — unpatched
6.491s — 23855.3.patch0.130s — 23855.3.patch
strip_shortcode()3.184s — unpatched1.928s — unpatched
3.254s — 23855.3.patch0.130s — 23855.3.patch

As a consistency check: The patched functions, for text without Shortcodes, run 0.13(0|1|2)s, which makes sense as they execute the same code in this scenario.

Adding the check to has_shortcode() (and not just do_shortcode() and strip_shortcodes()) does also make sense, we were just measuring the wrong scenario.

#11 in reply to: ↑ 10 @nacin
6 years ago

Replying to TobiasBg:

Ok, that's somewhat embarrassing now... I tested the functionality of this change on the (correct) original patch 23855-shortcodes.patch, but failed to repeat that after refreshing coding standards in 23855.2.patch. My bad, sorry. Won't happen again.

Ha, no worries. I do worse regularly. :-)

#12 @nacin
6 years ago

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

In 27394:

Bail early from shortcode functions if no delimiter is present.

This is a significant performance improvement for processing content without shortcodes, and only the slightest hit when content contains shortcodes (which must then undergo processing anyway). Performance results on the ticket.

props TobiasBg.
fixes #23855.

Note: See TracTickets for help on using tickets.