bobbingwide / oik

OIK Information Kit
https://www.oik-plugins.com/oik-plugins/oik
GNU General Public License v2.0
2 stars 0 forks source link

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

Open bobbingwide opened 5 years ago

bobbingwide commented 5 years ago

I re-raised Gutenberg issue 5376 as https://github.com/WordPress/gutenberg/issues/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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

[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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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 https://github.com/bobbingwide/sb-coming-up/issues/1#issuecomment-791258921

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.