WebKit / Speedometer

An open source repository for the Speedometer benchmark
Other
614 stars 73 forks source link

[Big-dom-generator] Change SVG to <use> element and Fix dom node count #295

Closed issackjohn closed 1 year ago

issackjohn commented 1 year ago

Changes in this PR include:

NOT READY FOR REVIEW [WIP]

julienw commented 1 year ago

TaskListIcon was converted to an SVG with a use element inside of it because each TaskListIcon would have 6 children. This would result in a significant amount of the children in the tree-area coming from the SVG which is not desired. The ChevronRight is not a problem because it only has 1 child.

Hey, I've seen this and was wondering why you decided to implement this change. Is there some real-world data backing up this change?

I did see some versions of this, but that can be subtly different. For example some are using <use> with an external file (eg decathlon.fr, fnac.com), while others are using it with using a hash to refer to an id for a <symbols> (eg: ebay.com). My understanding is that the performance characteristics can be very different between <use> and plain SVG, and between these 2 cases, so it would be good to be more explicit. I didn't do a thorough search on websites so I was wondering if you did.

Thanks for your answer.

lpardosixtosMs commented 1 year ago

TaskListIcon was converted to an SVG with a use element inside of it because each TaskListIcon would have 6 children. This would result in a significant amount of the children in the tree-area coming from the SVG which is not desired. The ChevronRight is not a problem because it only has 1 child.

Hey, I've seen this and was wondering why you decided to implement this change. Is there some real-world data backing up this change?

I did see some versions of this, but that can be subtly different. For example some are using <use> with an external file (eg decathlon.fr, fnac.com), while others are using it with using a hash to refer to an id for a <symbols> (eg: ebay.com). My understanding is that the performance characteristics can be very different between <use> and plain SVG, and between these 2 cases, so it would be good to be more explicit. I didn't do a thorough search on websites so I was wondering if you did.

Thanks for your answer.

We realized that around a third of the elements of the generated DOM were coming from these SVG items, this was not intended and I felt we should need a good reason to have that many svg elements. We didn't do a thorough search to justify going with use instead, beyond the fact that it generates less elements and that it is used in some sites. I don't have a preference for either solution, and earlier perf runs suggested that there was not much of a perf difference (at least in the measured time). But my latest run was different so I'm keeping this as draft until I can dig a little bit into the performance.

Perhaps the we could avoid the use syntax and also the large amount of svg children by just using a simpler icon instead.

rniwa commented 1 year ago

Do we have any data which suggests that SVG use element is commonly used?

lpardosixtosMs commented 1 year ago

Do we have any data which suggests that SVG use element is commonly used?

@rniwa Thanks for asking. I did a quick search across 20 sites, and I only found 3 using the <use> tag. I'm ok with dropping the <use> syntax given the low usage.

Sites visited:

Sites using the <use> tag:

lpardosixtosMs commented 1 year ago

Closing this PR. I'll open a new one for the bug fix only.