Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't waste time providing data not actually required in a REST request #120

Open
bobbingwide opened this issue Jan 31, 2019 · 8 comments
Open

Comments

@bobbingwide
Copy link
Owner

bobbingwide commented Jan 31, 2019

I re-raised Gutenberg issue 5376 as WordPress/gutenberg#13618 but don't know if the problem will be addressed quickly.

So I need to investigate ways to avoid wasting time providing answers to the REST APIs' rhetorical questions.

i.e. Don't expand shortcodes during the_content processing when the client doesn't need the information to do its job.

Requirement

A performant Page Attributes block when using the Block editor to edit a hierarchical post type's post which is likely to contain a number of shortcodes that could take a long time to expand.

Proposed solution

I'm working on it.

@bobbingwide
Copy link
Owner Author

The elapsed time to populate the Parent Page: dropdown for the oik_file post type was approximately 4.5 seconds per REST request.

https://s.b/oikcom/wp-json/wp/v2/oik_file?per_page=100&exclude%5B0%5D=23607&parent_exclude%5B0%5D=23607 &orderby=menu_order&order=asc&context=edit&_locale=user&page=2

Each post contained the following content

<!--more -->[file][bw_fields]

For each post found the REST api returned the content as both raw and rendered.
Preventing the server from processing the_content reduced the server elapsed time to 0.9 secs.

@bobbingwide
Copy link
Owner Author

The first part of this fix is to ensure that the REST requests do actually complete. There were a couple of routines that failed due to functions not being available.

@bobbingwide
Copy link
Owner Author

bobbingwide commented Jun 5, 2019

but don't know if the problem will addressed quickly.

Nope. It hasn't been addressed yet. It's still a problem. I reproduced the issue in my local environment in s.b/hm with just 54 pages. The server timed out after 122 seconds. The page for which the excerpt was being produced at the time contained a shortcode that was satisfied by HTTP requests to other sites since the transient data that's normally used had expired.

Workaround

That page did not have a hand cranked excerpt. I added one to reduce the time taken to perform respond to get_the_excerpt.

Proposed solution

In the hook to respond to rest_api_init, if the request is with context=edit then disable all filter functions for both the_content and get_the_excerpt.

@bobbingwide
Copy link
Owner Author

[11-Aug-2019 19:02:31 UTC] PHP Fatal error: Uncaught Error: Call to undefined function bw_trace_get_attached_hooks() in /home/susan/public_html/wp-content/plugins/oik/oik.php:465
Stack trace:
#0 /home/susan/public_html/wp-includes/class-wp-hook.php(286): oik_rest_api_init(Object(WP_REST_Server))
#1 /home/susan/public_html/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(NULL, Array)
#2 /home/susan/public_html/wp-includes/plugin.php(465): WP_Hook->do_action(Array)
#3 /home/susan/public_html/wp-includes/rest-api.php(475): do_action('rest_api_init', Object(WP_REST_Server))
#4 /home/susan/public_html/wp-includes/rest-api.php(302): rest_get_server()
#5 /home/susan/public_html/wp-includes/class-wp-hook.php(286): rest_api_loaded(Object(WP))
#6 /home/susan/public_html/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters('', Array)
#7 /home/susan/public_html/wp-includes/plugin.php(531): WP_Hook->do_action(Array)
#8 /home/susan/public_html/wp-includes/class-wp.php(387): do_action_ref_array('parse_request', Array)
#9 /home/susan/public_html/wp-inc in /home/susan/public_html/wp-content/plugins/oik/oik.php on line 465

@bobbingwide
Copy link
Owner Author

bobbingwide commented Nov 29, 2019

This problem reared its ugly head yesterday when I created a page containing a shortcode to display a table of posts which included 2 virtual fields which both perform a lot of processing. Totally unnecessary processing when all the editor needs is the raw content.

[bw_table post_type=oik-plugins fields=title,_oikp_block_count,blocks_delivered,cloned]

I need to check what type of request this was.
If it’s the editor then it’s probably OK to NOT expand the content.
Can we use WP_screen::is_block_editor ?

If it’s server side rendering then we do need to expand the content.

@bobbingwide bobbingwide added this to In progress in Oik v4.2.0 Feb 5, 2020
@bobbingwide
Copy link
Owner Author

but don't know if the problem will addressed quickly.

Nearly one year on, and approaching WordCamp Europe contributor day. I think it’s time to revisit the problem and see what the current situation is with a) Gutenberg and b) oik

Note: Check what causes wp-pompey.org.uk to display ‘Not today thank you’ when displaying the Fields block in the editor.

@bobbingwide
Copy link
Owner Author

but don't know if the problem will addressed quickly.

My PR for Gutenberg issue 13618 has been merged into Gutenberg. The serious performance issue should soon become a thing of the past.
Soon I’ll be able to undo any changes I’d implemented as quick fixes.

@bobbingwide
Copy link
Owner Author

bobbingwide commented Mar 5, 2021

Proposed solution
In the hook to respond to rest_api_init, if the request is with context=edit then disable all filter functions for both the_content and get_the_excerpt.

This solution causes a problem when the block editor invokes a Server Side Rendered function to render a block and that block needs to get the excerpt. The value of post_excerpt is returned, not the excerpt that's trimmed from the post_content.
See bobbingwide/sb-coming-up#1 (comment)

One solution would be to reinstate the wp_trim_excerpt filter function for get_the_excerpt.

I can remove all the filters for get_the_excerpt then re-add wp_trim_excerpt with the same priority.
I need to test whether or not I can still get away with removing all the filters for the_content.

Another solution is to implement the fix in the block's rendering logic.
I'll need to try that as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Oik v4.2.0
  
In progress
Development

No branches or pull requests

1 participant