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

Remove the remote HTTP connection to fetch the filesize #6

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

johnbillion
Copy link
Member

The performance of this plugin on my local Altis site was at a crawl. Each request to the attachments query Ajax endpoint was taking 20-30 seconds.

I narrowed down the problem to this call to get_headers(). It's called for each file in the result and performs a server side HTTP request to the URL in order to fetch its size. I can see that the file size is not exposed in the WordPress REST API, which I guess is the reason this code exists.

Removing this call brings the performance back to acceptable levels.

Removing this means the File size: section in the media manager contains no value, but this is a fair tradeoff to bring the performance back.

Regarding exposing the file size, we could:

  • Open a WordPress core ticket to get the file size added to the REST API response, or
  • Add an extra field to the REST API response via this plugin which would solve the problem for a Multisite network

In the meantime I propose simply removing this.

@johnbillion johnbillion requested review from onyekaa and tfrommen March 16, 2021 17:09
@onyekaa
Copy link
Contributor

onyekaa commented Mar 16, 2021

Fetching the size this way was supposed to be more performant but it's possible it could have been tested more aggressively. I'm surprised it took so long for you locally.

But I actually like your second suggestion. I think that would be quicker short-term solution.

@johnbillion
Copy link
Member Author

Cheers Onyeka, I opened #7 for that.

Copy link
Contributor

@tfrommen tfrommen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like Onyeka, I'm surprised that your local performance was that miserable. For me, it's also rather slow, bu "only" like about 5 seconds maybe.

Trying to add the filesize to the REST API response is something we can try, sure.

Let's remove the current request/info though. 👍

@johnbillion
Copy link
Member Author

I'm using Altis local-server FWIW.

@johnbillion johnbillion merged commit c00d444 into main Mar 16, 2021
@johnbillion johnbillion deleted the fix/filesize branch March 16, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants