Created attachment 208249 [details]
Patch

Comment on attachment 208249 [details]
Patch

I withdraw this patch... for now.

Comment on attachment 208249 [details]
Patch

This optimisation helps by making attribute parsing faster and by allowing more style sharing (which both saves memory and speeds up style resolve). I'm not entirely convinced that presentation attributes are rare enough that this is completely worthless. I imagine that there are some sites with lots of <img width height> where the attributes are currently getting shared. It would be nice to get some actual data about the effectiveness.

It is pretty clear this is nor particularly valuable optimisation in any case. If removing this allows better optimisation of modern cases or cleans up architecture significantly then it definitely should go.

Created attachment 299062 [details]
Patch

Comment on attachment 299062 [details]
Patch

Let's try it!

Comment on attachment 299062 [details]
Patch

Clearing flags on attachment: 299062

Committed r210826: <http://trac.webkit.org/changeset/210826>

All reviewed patches have been landed.  Closing bug.

Looks like this regressed html-parser test by 4%: https://perf.webkit.org/v3/#/charts?since=1482318276369&zoom=(1484447791133.929-1484913327596.5723)&paneList=((18-15-26637537-null-(5-2.5-500)))&repository=1

Comment on attachment 299062 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=299062&action=review

> Source/WebCore/dom/StyledElement.cpp:223
> +    RefPtr<StyleProperties> style = MutableStyleProperties::create(isSVGElement() ? SVGAttributeMode : HTMLQuirksMode);

Should be a Ref, not a RefPtr. Normally I use auto in a case like this, which would give use Ref<MutableStyleProperties>.

> Source/WebCore/dom/StyledElement.cpp:224
> +    for (const Attribute& attribute : attributesIterator())

I like auto& here.

> Source/WebCore/dom/StyledElement.cpp:225
> +        collectStyleForPresentationAttribute(attribute.name(), attribute.value(), static_cast<MutableStyleProperties&>(*style));

Typecast would not be needed here if we used the correct type above. If we used auto, then I think the argument could just be "style".