Bug 208741 - Generate a trie parser for tag names
Summary: Generate a trie parser for tag names
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tadeu Zagallo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-06 15:41 PST by Tadeu Zagallo
Modified: 2021-08-05 15:20 PDT (History)
8 users (show)

See Also:


Attachments
Patch (26.83 KB, patch)
2020-03-06 15:44 PST, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
patch (24.98 KB, patch)
2021-08-02 17:03 PDT, Saam Barati
no flags Details | Formatted Diff | Diff
WIP (32.44 KB, patch)
2021-08-05 15:20 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tadeu Zagallo 2020-03-06 15:41:48 PST
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.
Comment 1 Tadeu Zagallo 2020-03-06 15:44:15 PST
Created attachment 392792 [details]
Patch
Comment 2 Sam Weinig 2020-03-07 11:33:31 PST
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?
Comment 3 Yusuke Suzuki 2020-03-09 00:39:43 PDT
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.
Comment 4 Tadeu Zagallo 2020-03-09 11:49:50 PDT
(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.
Comment 5 Saam Barati 2021-08-02 17:03:24 PDT
Created attachment 434805 [details]
patch

rebased, but it has a hang. Gonna figure out why. Wanna test perf of this against ToT.
Comment 6 Saam Barati 2021-08-05 15:20:11 PDT
Created attachment 435029 [details]
WIP