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

Reduce Requests for the gallery block #10994

Closed
derweili opened this issue Oct 24, 2018 · 11 comments · Fixed by #34389
Closed

Reduce Requests for the gallery block #10994

derweili opened this issue Oct 24, 2018 · 11 comments · Fixed by #34389
Assignees
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [Feature] Media Anything that impacts the experience of managing media [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later REST API Interaction Related to REST API [Status] In Progress Tracking issues with work in progress
Milestone

Comments

@derweili
Copy link
Contributor

Describe the bug
The Gallery Block fires a GET request for each image that is uses in the gallery. This does not only happen when an image is added to the gallery at the first time but also when the edit screen is reloaded.

This is not a problem when only a few image are in a gallery but this can really become a problem when many images are added.

To Reproduce
Steps to reproduce the behavior:

  1. Add a gallery block
  2. Add multiple image to the gallery

Expected behavior
Instead firing a request for each image all requests should be combined into one.

So instead of this request:

GET /wp-json/wp/v2/media/5291?context=edit
GET /wp-json/wp/v2/media/5290?context=edit
GET /wp-json/wp/v2/media/5293?context=edit

the requests should be combined into this request:

GET /wp-json/wp/v2/media/?include=5291,5290,5293

@derweili derweili changed the title Reduce for the gallery block Reduce Requests for the gallery block Oct 24, 2018
@Soean Soean added [Feature] Media Anything that impacts the experience of managing media REST API Interaction Related to REST API labels Oct 24, 2018
@desrosj desrosj added this to the WordPress 5.0 milestone Oct 25, 2018
@Soean Soean added the [Block] Gallery Affects the Gallery Block - used to display groups of images label Oct 26, 2018
@dd32
Copy link
Member

dd32 commented Oct 31, 2018

This doesn't feel like an urgent fix is needed, just an optimisation that might not be as simple as it first looks (although someone far more familiar with the gallery code might be able to point where to look..)

Would a better option be to preload images contained within galleries when editing posts? That would still cause these extra API requests when inserting a gallery, but seems like it'll be better for editing image heavy posts.

An example of that being done is this hacky approach (A better approach would be to use the parsed block data, but I didn't immediately see how to get that data)

add_filter( 'block_editor_preload_paths', function( $preload_paths, $post ) {
	// Preload any image responses too
	if ( preg_match_all( '!wp-image-(\d+)!', $post->post_content, $m ) ) {
		foreach ( $m[1] as $id ) {
			$preload_paths[] = sprintf( '/wp/v2/media/%d?context=edit', $id );
		}
	}

	return $preload_paths;
}, 10, 2 );

@antpb
Copy link
Contributor

antpb commented Oct 31, 2018

@dd32 I completely agree, this is a bit of an optimization more than an urgent fix. Your assumption about this not being as simple as it first looks is absolutely correct. I spent some time going through it the last few days and my guess for the multiple requests is because the following is called for each image instance added to the gallery:

export default withSelect( ( select, ownProps ) => {
const { getMedia } = select( 'core' );
const { id } = ownProps;
return {
image: id ? getMedia( id ) : null,
};
} )( GalleryImage );

The map that is calling that component is here:

{ images.map( ( img, index ) => (
<li className="blocks-gallery-item" key={ img.id || img.url }>
<GalleryImage
url={ img.url }
alt={ img.alt }
id={ img.id }
isSelected={ isSelected && this.state.selectedImage === index }
onRemove={ this.onRemoveImage( index ) }
onSelect={ this.onSelectImage( index ) }
setAttributes={ ( attrs ) => this.setImageAttributes( index, attrs ) }
caption={ img.caption }
/>
</li>
) ) }

It would require a rethinking of how to create a gallery image for each image returned from the modal. I'd say lets leave this in the 5.0 milestone and we can reevaluate in the next week or so if this should be in a 5.0.x instead. Maybe someone has some ideas? :)

@antpb antpb added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Oct 31, 2018
@antpb antpb self-assigned this Nov 1, 2018
@derweili
Copy link
Contributor Author

derweili commented Nov 5, 2018

Another Challenge ist that if the gallery has more than 100 Images the API calls must be separated into two calls because of the API-limits.

@antpb @dd32 of cause this is just a optimization but don't this this is completely unimportant.

I testet the gallery one of my pages that uses the gallery very much.
The Site is running on a Common German Shared-Hosting.

The gallery contains 72 images

30 seconds pass between the first Media API Call and the Last Media API Call.

All Calls combined into one take only 1.5 seconds.

A load time > 30 Seconds makes the Editor un usable.

@dd32
Copy link
Member

dd32 commented Nov 6, 2018

A load time > 30 Seconds makes the Editor un usable.

That's one of the reasons that I think pre-loading the API calls for any galleries would be beneficial, that way the API calls aren't needed as they'd be bulk loaded with the post - Assuming that's performant with a gallery of that size

@danielbachhuber
Copy link
Member

Moving to WordPress 5.0.x Follow Ups because I don't see this as a 5.0 release blocker and we should focus on those first.

@joostdekeijzer
Copy link

Hi,

My server is hitting some server reached max_children setting warnings, turns out a client has many long pages with many galleries on it resulting in over 80 simultaneous /wp-json/wp/v2/media/<id> requests when an editor opens such a page in the WordPress backend.

Would some sort of lazy loading of the galleries be possible to help alleviate this?

@christianoliveira
Copy link

Hi! I have a travel blog and recently started using Gutenberg. I write very long, detailed posts and this week I experimented for the first time with Gutenberg, the new blocks system and Jetpack Galleries, and it has been, unfortunately, a horrible experience :(

The post is very long (>30.000 words, >500 images), during the writing, there was usually a 2-5 seconds lag between the keyword input and seeing it on screen, making it very unusable. Everything in general worked so so so so slow it was very difficult to use. I managed to finish the post and published it (you can check it here) but now I CAN NOT edit the post.

Whenever I try to open it in the wordpress editor, it keeps loading for minutes and my CPU goes 100% non-stop. If I open the Chrome Dev Tools, I can see in the network tab endless requests to /wp-json/wp/v2/media/ . You can see a video recording here of it, this is after minutes open: https://drive.google.com/file/d/1ZTloy4Q_jsAdeQ-tRTTGE4RRRD84mPkQ/view?usp=sharing (after that, it eventually stopped without even showing anything on screen.

I'm writing here as I was looking for possible causes and these requests may be related. I've been using wordpress in this blog since 6-7 years ago, I have several others long posts with also lots of images (although without jetpack galleries) and haven't had issues until know.

All the forums I check ask to deactivate all plugins, which is unrealistic as this is a live getting-traffic site and I prefer not to do that.

Any clues? Is this something more people are experiencing? Is Jetpack safe to use with Gutenberg?

I'm using the Newspack theme (https://newspack.pub/ ) and everything up to date (wordpress core, all the plugins, etc.)

Thanks in advance!

@christianoliveira
Copy link

I'm still trying to make sense of this, I've tried to install a REST API cache plugin (this specifically https://wordpress.org/plugins/wp-rest-cache/ ) but it didn't make any difference.

All the requests to /wp-json/wp/v2/media/ keep firing and take around 1s each to load. Is there any possibility to force a stop of those request (so I can open and edit the post!) and/or to only fire the requests for each specific gallery when the gallery is being edited?

@joostdekeijzer
Copy link

@christianoliveira I did some digging in the code and I think the main issue here is that a gallery is handled by the code as a bunch of separate images. And for each image a separate API call is done to retrieve the info needed. This is not easily solved by a cache plugin I guess, but I don't know the plugin your using.

@dd32 @antpb the current priority is "low". I also see a lot of "refactoring" issues for the gallery code. Is it possible to somewhere include this performance issue into the refactoring being done?

Thanks!

@christianoliveira
Copy link

christianoliveira commented Apr 28, 2021

@joostdekeijzer thanks for the fast response and for looking into ti!

The cache plugin is specifically for the REST API, so if it works (which I assume is not working in my case, I need to review it further) if should make those "/wp-json/wp/v2/media/" requests faster by avoiding querying the database (now, at least in my case, each /wp-json/wp/v2/media/ requests takes 1s average to load so it completely freezes my browser until it finishes if it does, as the post has lots of galleries and images. Still not optimal but I was hoping to at least be able to edit the post at this point.

I understand this may not be urgent, but it's so frustating as a user see that a "simple" functionality turns wordpress almost completely impossible to use. Also I'm worried that all the manual work invested into creating those galleries will maybe be for nothing as if there is no solution, I will have to try deleting the galleries :(

Also, seeing the last replies, this was an issue already 2-3 years ago!

@LordHelmchen
Copy link

Wouldn't it make sense to look into pagination or something like "load more" while working on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Gallery Affects the Gallery Block - used to display groups of images [Feature] Media Anything that impacts the experience of managing media [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later REST API Interaction Related to REST API [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.