Bug 236570 – Cache an entire attribute QualifiedName when parsing HTML, not just its local name AtomString

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Cameron McCormack (:heycam)]

Bug 229907 added HTMLAtomStringCache, which uses a fast to compute hash that works well to cache HTML tag names, attribute names, and attribute value.  When AtomHTMLToken initializes its list of Attributes, it uses HTMLAtomStringCache to look up or create an AtomString for the attribute's local name, and then creates a QualifiedName to wrap it.  QualifiedName construction involves looking up QualifiedNameCache, which is a thread-specific cache of QualifiedNameImpl objects.  If we make HTMLAtomStringCache responsible for caching an attribute's QualifiedName instead of just its local name AtomString, we can avoid the work of looking up the QualifiedNameCache.

Doing this results in a 0.2-0.3% improvement on Speedometer 2, and a 0.3-0.4% improvement on PLT5.

[Radar WebKit Bug Importer]

<rdar://problem/88876545>

[Cameron McCormack (:heycam)]

Created attachment 451841 [details]
Patch

[Darin Adler]

Comment on attachment 451841 [details]
Patch

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

> Source/WebCore/html/parser/HTMLNameCache.h:38
> +    ALWAYS_INLINE static AtomString makeTagName(const Vector<UChar, inlineCapacity>& string)

Can this take a Span<UChar> instead? Should be just as fast, although we’d need to test.

> Source/WebCore/html/parser/HTMLNameCache.h:44
> +    ALWAYS_INLINE static QualifiedName makeAttributeQualifiedName(const Vector<UChar, inlineCapacity>& string)

Ditto.

> Source/WebCore/html/parser/HTMLNameCache.h:50
> +    ALWAYS_INLINE static AtomString makeAttributeValue(const Vector<UChar, inlineCapacity>& string)

Ditto.

> Source/WebCore/html/parser/HTMLNameCache.h:67
> +    ALWAYS_INLINE static AtomString makeAtomString(const Vector<UChar, inlineCapacity>& string)

Ditto.

> Source/WebCore/html/parser/HTMLNameCache.h:89
> +    ALWAYS_INLINE static QualifiedName makeQualifiedName(const Vector<UChar, inlineCapacity>& string)

Ditto.

> Source/WebCore/html/parser/HTMLNameCache.h:134
> +    using QualifiedNameCache = std::array<RefPtr<QualifiedName::QualifiedNameImpl>, capacity>;

Why is this RefPtr<Impl> instead of just QualifiedName?

[Cameron McCormack (:heycam)]

(In reply to Darin Adler from comment #3)
> Can this take a Span<UChar> instead? Should be just as fast, although we’d
> need to test.

Yes, should be able to.  Then it could be untemplatized.

> > Source/WebCore/html/parser/HTMLNameCache.h:134
> > +    using QualifiedNameCache = std::array<RefPtr<QualifiedName::QualifiedNameImpl>, capacity>;
> 
> Why is this RefPtr<Impl> instead of just QualifiedName?

std::array<QualifiedName> would require QualifiedName have a default constructor, which it doesn't currently, since it doesn't support its m_impl being null.

[Cameron McCormack (:heycam)]

Created attachment 451849 [details]
Patch

[EWS]

Committed r289991 (247377@main): <https://commits.webkit.org/247377@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451849 [details].