Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 6 years ago

#36767 closed defect (bug) (fixed)

oEmbed performance optimisation

Reported by: bactisme's profile bactisme Owned by: swissspidy's profile swissspidy
Milestone: 4.5.3 Priority: normal
Severity: normal Version: 4.4
Component: Embeds Keywords: has-patch fixed-major
Focuses: performance Cc:

Description

If you add one of your own post as oembed, It will, at the post page (or feed) request, still trigger oembed discovery mechanism (calling the page, parsing content to find oembed link), making a request from the server on itself.

(wp-include/class-oembed.php get_provider/discover)

If you don't have page cache (I don't on /feed) and make a post like "weekly summary" using a lot of oembed, its seems to trigger lot of requests.

Since we know it's one of our url, could we just remove the overhead of making a external request?

Attachments (1)

36767.diff​ (533 bytes) - added by kraftbj 8 years ago.
Return out early if there's no post ID found.

Download all attachments as: .zip

Change History (17)

#1 @swissspidy
8 years ago

  • Component changed from General to Embeds
  • Owner set to swissspidy
  • Status changed from new to assigned
  • Version changed from trunk to 4.4

#2 @swissspidy
8 years ago

  • Milestone changed from Awaiting Review to 4.6

#3 @swissspidy
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 37708:

Embeds: Improve performance when embedding a post from the current site.

When the post being embedded is from the same site, there's no reason to do an HTTP request for it. The data can be fetched directly using get_oembed_response_data().

Fixes #36767 for trunk.

#4 @swissspidy
8 years ago

  • Keywords fixed-major added
  • Milestone changed from 4.6 to 4.5.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

#5 @swissspidy
8 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 37709:

Embeds: Improve performance when embedding a post of the current site.

When the post being embedded is from the same site, there's no reason to do an HTTP request for it. The data can be fetched directly using get_oembed_response_data().

Merge of [37708] to the 4.5 branch.

Fixes #36767.

#6 @swissspidy
8 years ago

In 37710:

Fix tests after [37708].

See #36767.

#7 @swissspidy
8 years ago

In 37711:

Fix tests after [37709].

Avoids using assertNotFalse() which is only available in PHPUnit 4.0 and above.

See [37708], [37710].
See #36767.

#8 @kraftbj
8 years ago

  • Keywords has-patch added; fixed-major removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

This needs to be tweaked slightly.

url_to_postid will return 0 when none is found. Then we're passing get_oembed_response_data, which then uses get_post. When an empty value is passed to get_post, the post global is used.

This results in some embeds being returned as an embed of the post instead of the intended result.

Test plugin: ​https://gist.github.com/kraftbj/535c3cfd2df6402f5f0024d035b68a59
In a post, add [ted id=1969].
Expected: The ID is converted to an oEmbed of a TED video.
Actual: A self-embed of the post.

Last edited 8 years ago by kraftbj (previous) (diff)

@kraftbj
8 years ago

Return out early if there's no post ID found.

This ticket was mentioned in ​Slack in #core by kraft. ​View the logs.


8 years ago

#10 @swissspidy
8 years ago

In 37729:

Embeds: Enforce a valid post ID when embedding a post from the current site.

Otherwise wp_filter_pre_oembed_result() could erroneously return the HTML of the current post instead of the intended result.

Props kraftbj.
See #36767.

#11 @swissspidy
8 years ago

In 37732:

Embeds: Enforce a valid post ID when embedding a post from the current site.

Otherwise wp_filter_pre_oembed_result() could erroneously return the HTML of the current post instead of the intended result.

Merge of [37729] to the 4.5 branch.

Props kraftbj.
See #36767.

#12 @swissspidy
8 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

#13 @swissspidy
8 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#14 @swissspidy
8 years ago

  • Keywords fixed-major added

#15 @swissspidy
8 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 37798:

Embeds: Improve performance when embedding a post from the current site.

When the post being embedded is from the same site, there's no reason to do an HTTP request for it. The data can be fetched directly using get_oembed_response_data().

Merge of [37708], [37710] and [37729] to the 4.4 branch.

Fixes #36767.

#16 @swissspidy
6 years ago

In 41600:

Embeds: Improve performance when embedding a post on Multisite.

After [37798], this fixes embeds coming from a different site in the network.

Props imath.
Fixes #40673. See #36767.

Note: See TracTickets for help on using tickets.