[Icons] Optimize Icons by inlining SVGs

[pierredup]

Q	A
Bug fix?	no
New feature?	yes
Issues	N/A
License	MIT

When using a lot of icons on a single page, each icon will, at best, trigger a filesystem lookup and at worst, do a network call to Iconify to load the icon. This can be bad for performance when a lot of icons needs to be loaded on the page. Setting up a decent cache pool can help with this performance, but can still end up in hundreds of request to the cache server for large pages.

This PR helps to optimize icons by inlining the SVG into the compiled template. This will only be done when using string literals for the icon name and all attribute keys and values. Any dynamic value used in the ux_icon function will fall back to the default behaviour.

[nicolas-grekas]

Is this reasoning still true when considering HTTP/2? To me this prevents efficient use of HTTP caching and uses the network to send low-priority bytes.

[pierredup]

The network calls are done server-side and does not happen in parallel, so HTTP/2 and HTTP caching doesn't play a role here.

A quick benchmark, loading a page with ~250 icons:

Case	Twig Render Time	Comments
Initial page load (No locally cached icons)	47747 ms	This is expected, since icons needs to be downloaded from Iconify first
Locally cached icons	24 ms
Inline SVG icons (this patch)	8 ms	~63% improvement

[pierredup]

One thing I also just realised, when replacing the local SVG file, the Twig cache would need to be cleared for the change to take affect, so this would either need to be opt-in, or a note added to the documentation

[nicolas-grekas]

Oh, I completely misunderstood the PR. Makes sense to me.
Just one issue: when renderIcon fails, we should fallback to regular runtime-resolution IMHO.

[smnandre]

Hey @pierredup ! Thank you for this nice PR

Hot reaction: I'm indeed not sure we want to enable such behaviour without some opt-in/out first :)

Couple of questions / comments:

I suppose it does not work with the twig components for now ?
Right now we bypass the component renderer to call directly the IconRenderer, with decent results

I started something back in february to "compile" the UX Icon Twig component back into twig function.
So this kind of optimization was on our mind, so there is something to digg here :)

Now i'm wondering.. do you have a IRL example of these 250 icons on a page ? Are they repeated ? Customized ?

PS : I'll open in an hour or two (update: tomorrow) a PR about icon configuration, i'm curious to see if this PR change anything :)

[javiereguiluz]

@smnandre about examples of pages with lots of icons, this page --> https://symfony.com/components has more than 500 icons:

But, it's a rare edge-case and all the other pages have less than 10.

[smnandre]

Thank you @javiereguiluz!

I think this is very good example of what the "reuse/symbol" we're working on (see #1800) can solve.
And it demonstrate the importance to add steps in a way that does not prevent future features or break existing ones.

But depending on how we implement these things, inlining could reveal itself a great advantage in rendering performance... but also the contrary if it prevents either runtime changes or symbol optimization.

So to be clear: i'm very, very interested in what you did there @pierredup, and i'd love to integrate twig compilation where we can or need !

(we worked on theses subjects during the early dev of this component and decided to postone it for the same reasons.. I don't find the step3 branch no more, but inlining was the idea.. the step2 was about converting the twig component into the ux_icon function right in the code.. something i'll need to implement asap because i'm moving stuff in the TwigComponent implementation and so the current Listener hack won't work anymore..)

Would you be ok to wait 10 days @pierredup and see then ?

With the configuration options, whatever we end un with for the prerenderers, and some experimental reuse/symbol system (or what we will have done then of course) ... we'll have all the cards on the table to chose what/when/how this compilation would offer the best advantage.

--

Regarding HTTP / cachewarmer / etc.. we now download the icons one by one. We could create some queue/buffer system to optimize it: multiple icons can be downloaded at once, as long they are in the same icon set...
For people using a favorite set this could represent a major gain in total download time...

[smnandre]

I've been thinking a lot about this PR and the nice work done here..

What if we use a flag/argument for a first version ?

Something like

{# Twig function #} 
{{ ux_icon('bi:circle', inlined: true) }}

{# Twig Component #} 
<twig:ux:icon name="github" inlined />

wdty @pierredup ?

[kbond]

In this case we'd throw an exception if it can't be inclined then?

[pierredup]

What if we use a flag/argument for a first version ?

@smnandre Would there be a scenario where a user doesn't want some icons to be inlined?

I think I would prefer a config option to globally enable/disable this for all icons

ux_icons:
    inline: true # default value is false

[smnandre]

I'm thinking of 4 things here:

• cache (file or config) that people will not know about
• n/bad access to network during twig cache build (this would make the compilation really long)
• conflict with other features
• impossibility for us to go back for BC break reasons

I'll work tonight on the sprite/reuse thing to see (maybe there would be no conflict)

That's why i'd suggest the following (without

2.20

• we add an optional manual flag in the config - experimental
• we do not set it in the recipe (to let options open for the future)

ux_icons:

    # Compile static icons on warm-up  (experimental)
    # foobar: true
    

2.22

• we set a default value (true) in the recipe
• we deprecated no setting this value --> at this moment we decide the default value

3.x

• defaut behaviour uses this decided value

What do you think ?

---

@pierredup i agree with you, a global config is best

I'm not so sure about the inline name, especially as we already inline in the HTML.
And let's keep in mind the enable/disable would not be "runtime"

What do you think:

• compile_on_build
• compile_in_template

Any other idea welcome here :)

[smnandre]

@pierredup I was going to add the configuration I asked for and merge this PR ...

.. but two things during my tests

1/ the HTML syntax is not precompil-able for now. Not a deal breaker to me. But this will be something we need to explicitely state in documentation

 {# will be compiled #}
{{ ux_icon('foo') }} 

{# will NOT be compiled #}
<twig:ux:icon name="foo" />

**2/ ** i have no idea how to simply configure this behaviour, as the node should not be services and I would like to avoid, as much as possible, to add this step in the hot path.

First though here:

• register a different "extension" depending on the configuration... the compiling one would be instanciated/decorated on warm up before it provides a specific FunctionNode in getFunctions()
• we could use a node visitor to do the same job but only during compilation
• ... any other ideas ?

wdyt ?

[kbond]

What about another function: ux_icon_inline()? If we can't use with html syntax, let's opt in this way.

[pierredup]

Maybe we don't need to have this option enabled or configurable? This package can just provide the UXIconFunction class, with some instructions in the documentation on how to use it with a custom twig function if someone wants to

[kbond]

I think this is super slick and I'd like to include natively!

[smnandre]

Well i had my concerns... so i just made too a "quick benchmark"

And the difference is, to me, almost non existant. And way lower than any of the digits previously exposed.

To be sure i understood your metrics, i made a page with 10 or... 250 different icons, and rendered it with

• inlined SVG
• ux_icon() functions
• <twig:ux:icon /> component

Well, in fact idk how... but i had only 244 icons in the "250" tests 😅

All files available on this gist: https://gist.github.com/smnandre/7b25b609554f451cd792342c71640bd3

Here are the results. I share them, but everyone can try and reproduce.

# icons	Inline SVG	Functions	Components
10
250

Before someone says "look, the components are slower"... yeah they are a bit. that's why i work on their optimization on the same time. But did you see who is the longest for 10 icons ? Do i make a conclusion on it ? Absolutelty: there is no comparaison possible locally for tiny differences like this.

My machine here is a 4 years old iMac with a lot running (apart this little test).. Also all my measure have been done in dev/debug. We all know in prod all this should be divided at least per 2 or 3.

And here are the cache metrics for the function_250 and components_250 "run"

#	Inline SVG	Functions	Components
250	xxx

So i think you may have something else going on your 38ms difference ... :/

---

Right now, to me , pushing this optimization for everyone everywhere feels too unbalanced.

PRO:

• i also have several cases in mind where this could be very usefull, but with an explicit opt-in / configuration
• microseconds won

CONS:
- does not work with
- variable classes
- variable icon names
- variable attributes
- Twig Component
- cache warm x2 longer
- forces early instanciation of Icon runtime (bad)
- no change on config change
- ignore IconRenderer decoration

[smnandre]

This package can just provide the UXIconFunction class, with some instructions in the documentation on how to use it with a custom twig function if someone wants to

I think making configurable is easier for people wanting this behaviour.

Not sure about the "UXIconFunction" class tbh. Either we provide this behaviour, or i think users will be able to use the IconRendererInterface in their code. But very open to discussion :)

Regarding documentation, either we are really talking about 250 different icons, and this is a very "niche" need, so not sure we should document this usage...

...or in reality we are talking in fact of 250 times the same icon and i would then advice we do not recommand this solution.

250 identical icons in a page can be a valid personal choice... but definitively not something i'd recommand in a general, especially not for performance reasons. Alternatives would have a bigger and better impact, both back-end in render time, HTML size... and front end (download size, core vitals, DOM / memory usage, styling, etc..)

[smnandre]

ux_icon_inline

That is much easier in term of communication. And could be easily enabled/disabled in dev/debug, with a dedicated custom Twig Runtime and no penalty for other users.

[kbond]

Admittedly, the exceptions and gotcha's for this feature are adding up.

Let's see if we can nail down the benchmark differences: @pierredup, in your benchmark, was apcu enabled? Icons caching is done with cache.system which uses apcu so 500 cache lookups should be snappy (not hitting the filesystem after the first request).

Another question: when you have 100's of icons on a page, most are duplicates, correct? @javiereguiluz's example above certainly is, 500 icons but looks like just 2-3 distinct. I'm thinking this is the 90-99% case. If this is the case, I'd prefer to move forward with #1800 as the a generic solution.

[pierredup]

I think this is becoming way more complex than the initial intention of just trying to improve some icon lookups. UXIconFunction works well for me, and other users can just implement this themself if they feel the need.

[smnandre]

As i told @kbond multiple times: only a fraction of users will benefit from this feature, so even minor drawbacks need careful consideration.

Thank you very much for your work on this PR @pierredup —please know there's nothing against it specifically.

And rest assured, the time invested here won’t be entirely in vain. Through testing, benchmarking, and exploring configurability options, I discovered here a way to improve the performance of the <twig:ux:icons /> Twig component.