Slow load times due to svg_dimensions

[cjyabraham]

Describe the bug

Some time ago I opened this issue regarding the performance of this plugin (also see this related issue). Basically when we have a lot of SVGs on a page populated by a Gutenberg block callback function, the load time for the page can be very long due to multiple calls to the svg_dimensions function. At the time the issue was not resolved so I edited the plugin as described to avoid overuse of the svg_dimensions function.

Now that this plugin is being developed again, could it be modified to include a filter to avoid this operation. You can see how I created the filter in my edited version of the plugin here. Alternatively, the performance issues could be addressed in some other way. Then I could stop using my custom version.

Steps to Reproduce

Create a page with a ton of SVGs that get populated using a custom Gutenberg block callback function. See this comment for more info.

Screenshots, screen recording, code snippet

See this image

Environment information

not relevant

WordPress information

This bug was reported 2 years ago so I was on an older version of WP then but I doubt the WP version is relevant.

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[darylldoyle]

@jeffpaul I wonder if a more performant approach here may be to re-work the svg_dimensions() call to store the data in attachment meta the first time it's called. Then on subsequent requests, we can pull from there, rather than loading the file and attempting to re-extract the sizes.

I can only think that it's the simplexml_load_file() call that's slowing everything down so much, so reducing that to a single call per SVG should help massively.

Thoughts?

[kamilgrzegorczyk]

Alternatively (or additionally) storing that in the object cache may help all of us who are using stuff like Redis.

[bmarshall511]

@darylldoyle Took a look at this & correct me if I'm wrong, but looks like this may no longer be an issue? Looking at the code, seems like we're storing it in meta & only being called in admin when a new image is uploaded.

[darylldoyle]

@bmarshall511 this is still an issue. We should address this by storing the width, height and orientation in the attachment meta and only re-calculating it if they don't exist.

This should be done within the safe_svg::svg_dimensions() method so that all callers benefit from the update. We should also add some tests to ensure that subsequent calls get the data from the meta rather than re-calculating it.

Additionally, it might be worth adding a short-circuit filter in case engineers want to implement their own calculations rather than using ours. This might be helpful when they're storing SVGs on third-party storage systems such as S3, where they're not immediately accessible in the local FS.

@bmarshall511 is this something that you're looking to pick up?

[bmarshall511]

@darylldoyle I'm no backend pro (more on the frontend side of things), but I'm all in for learning some new stuff. Bear with me if there's a better approach, but here's what I'm thinking:

1. In safe_svg::svg_dimensions(), we could use attachment_url_to_postid to fetch the attachment ID from the SVG url
2. If an ID, get the post meta for the dimensions and orientation (multiple records or all bundled up in one?)
3. If nothing available, go ahead with the parsing and stash the results accordingly (as long as we have an attachment ID)
4. For the return, send it through a filter (maybe name it something like safe_svg_dimensions) so that it's tweakable.

I'm new to the whole unit tests scene. Had a look at some of the ones we already have, and they seem pretty doable. If you have any advice or pointers, I'm all ears!

[darylldoyle]

@bmarshall511 that sounds like a great approach to me! Test-wise, if you get stuck, feel free to ping me on Slack and I can help 🙂