You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It would be good to reduce the number of HTTP requests necessary to load the icons. Currently every icon is its own file. Now that we don’t have PNG anymore it’s high time to optimize the SVG delivery.
Can anyone check if that's something that might work for us? From the text it is not clear at which point they are injecting SVG code into HTML...They were using icon fonts before, an approach that I was pondering as well, and didn't notice any performance change after making the switch.
Icon font is still an option for us, but there are some a11y problems related to it.
Is it possible to measure performance impact of #5985 to server?
Is it possible to measure performance impact of #5985 to server?
The only problem I see there is, that the browser can not cache that asset and thus it is loaded all the time. On the other side: the server should have this in memory all the time anyways and just serves it. Only problem is, that we also do a lot of JS injection of icons, which is not possible with that approach. and would require to load the asset via browser anyway. Nevertheless it's worth a try.
@pixelipo, we add some strings to the svg on the colour api, so it's logical that it's bigger.
Though the caching is working fine, so first load is long, second is instant :)
you're misreading it - in the first line, actual image size is 176bytes, but a total of 1.07 kilobytes were used up to fetch it.
In case of a direct SVG fetch (line 5), that difference is practically negligible.
I looked at some requests and the ~1kb of header data seems pretty reasonable, but there is not much we could do about this, besides merging icons to a sprite.
@pixelipo I also don't see any huge difference between the svg api endpoint and the direclty fetched svgs. Probably something related to the webserver setup?
There's a definitely a visible performance difference - the two icons in the screenshot bellow are very similar in size and the api version takes more than 10 times as long to load:
@pixelipo I noticed the php script is slowing things down yes!
I could use someone to check how to optimise things :)
I'm guessing since we also get the svg and read the filesystem there is multiple factors here:
Activity
pixelipo commentedon Nov 13, 2017
I did some more research regarding this problem. I still think that the #5985 is the direction we should head towards. It seems that Github is actually using a very similar solution:
https://github.com/blog/2112-delivering-octicons-with-svg
Can anyone check if that's something that might work for us? From the text it is not clear at which point they are injecting SVG code into HTML...They were using icon fonts before, an approach that I was pondering as well, and didn't notice any performance change after making the switch.
Icon font is still an option for us, but there are some a11y problems related to it.
Is it possible to measure performance impact of #5985 to server?
@skjnldsv @nickvergessen @rullzer @MorrisJobke
EDIT: here's Github's injector helper script primer/octicons_helper
MorrisJobke commentedon Nov 13, 2017
They do it in their ruby server. So it's basically the same as #5985
MorrisJobke commentedon Nov 13, 2017
The only problem I see there is, that the browser can not cache that asset and thus it is loaded all the time. On the other side: the server should have this in memory all the time anyways and just serves it. Only problem is, that we also do a lot of JS injection of icons, which is not possible with that approach. and would require to load the asset via browser anyway. Nevertheless it's worth a try.
13 remaining items
skjnldsv commentedon Oct 3, 2018
@pixelipo, we add some strings to the svg on the colour api, so it's logical that it's bigger.
Though the caching is working fine, so first load is long, second is instant :)
pixelipo commentedon Oct 3, 2018
it's 10 times the size, which is a bit concerning...
skjnldsv commentedon Oct 3, 2018
Hum, it's more like 100Bytes more, or am I misreading your screenshot?
pixelipo commentedon Oct 3, 2018
you're misreading it - in the first line, actual image size is 176bytes, but a total of 1.07 kilobytes were used up to fetch it.
In case of a direct SVG fetch (line 5), that difference is practically negligible.
skjnldsv commentedon Oct 3, 2018
Ah yes! How is that possible then? The data comes from the headers or something?
Packet size?
rullzer commentedon Oct 3, 2018
I don't buy that we add 900kb of headers.
@pixelipo any way to obtain more info on the request?
skjnldsv commentedon Oct 3, 2018
But it's 900 bytes, not KB?
1.07kb => 1070bytes, right?
rullzer commentedon Oct 3, 2018
Right. I should not answer so fast.
That might be. But all of it should be pretty aggressively cached as well
juliusknorr commentedon Oct 4, 2018
I looked at some requests and the ~1kb of header data seems pretty reasonable, but there is not much we could do about this, besides merging icons to a sprite.
@pixelipo I also don't see any huge difference between the svg api endpoint and the direclty fetched svgs. Probably something related to the webserver setup?
pixelipo commentedon Oct 4, 2018
There's a definitely a visible performance difference - the two icons in the screenshot bellow are very similar in size and the api version takes more than 10 times as long to load:
Here are few more examples of various filesizes:

skjnldsv commentedon Oct 4, 2018
@pixelipo I noticed the php script is slowing things down yes!
I could use someone to check how to optimise things :)
I'm guessing since we also get the svg and read the filesystem there is multiple factors here:
That's why we have hard caching :)
rullzer commentedon Oct 4, 2018
Well yes there is overhead of course. But it should be 1 time.
I assume there is some theming going on and that is why we don't ship it as static asset right?
juliusknorr commentedon Oct 26, 2018
Fixed with #12016 (using inline images in css instead of a sprite)