Bug 208741 – Generate a trie parser for tag names

• Status: NEW
• Component: WebCore Misc.

[Tadeu Zagallo]

Instead of adding the chars to a vector than using it to create an AtomString we can generate a trie parser for builtin tags that will give us the AtomString directly (and the element constructor). This skips hashing the string, the atomic string table look and element table lookup.

[Tadeu Zagallo]

Created attachment 392792 [details]
Patch

[Sam Weinig]

Comment on attachment 392792 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Instead of adding the chars to a vector than using it to create an AtomString we can generate a trie
> +        parser for builtin tags that will give us the AtomString directly (and the element constructor). This
> +        skips hashing the string, the atomic string table look and element table lookup.
> +
> +        No new tests since no functionality changed.

Does this end up being a win on any benchmarks? What is the underlying goal of making this change?

[Yusuke Suzuki]

Comment on attachment 392792 [details]
Patch

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

> Source/WebCore/html/parser/AtomicHTMLToken.h:253
> +            if (!(++counter % 1000))
> +                dataLogLn("Counter: ", counter, " (", makeString(m_name), ")");

Let's remove this debugging purpose code.

[Tadeu Zagallo]

(In reply to Sam Weinig from comment #2)
> Does this end up being a win on any benchmarks? What is the underlying goal
> of making this change?

Sorry, didn't mean to r? yet. I'm still running the benchmarks, but the idea is that it may help with Speedometer.

[Saam Barati]

Created attachment 434805 [details]
patch

rebased, but it has a hang. Gonna figure out why. Wanna test perf of this against ToT.

[Saam Barati]

Created attachment 435029 [details]
WIP