WordPress.org

Make WordPress Core

Opened 18 months ago

Closed 16 months ago

Last modified 3 weeks ago

#36767 closed defect (bug) (fixed)

oEmbed performance optimisation

Reported by: bactisme Owned by: 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 16 months ago.
Return out early if there's no post ID found.

Download all attachments as: .zip

Change History (17)

#1 @swissspidy
17 months 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
17 months ago

  • Milestone changed from Awaiting Review to 4.6

#3 @swissspidy
16 months 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
16 months 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
16 months 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
16 months ago

In 37710:

Fix tests after [37708].

See #36767.

#7 @swissspidy
16 months 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
16 months 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 16 months ago by kraftbj (previous) (diff)

@kraftbj
16 months ago

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

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


16 months ago

#10 @swissspidy
16 months 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
16 months 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
16 months ago

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

#13 @swissspidy
16 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#14 @swissspidy
16 months ago

  • Keywords fixed-major added

#15 @swissspidy
16 months 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
3 weeks 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.