Bug 208741

Summary:

Generate a trie parser for tag names

Product:

WebKit

Reporter:

Tadeu Zagallo <tzagallo>

Component:

WebCore Misc.

Assignee:

Tadeu Zagallo <tzagallo>

Status:

NEW

Severity:

Normal

CC:

cdumez, esprehn+autocc, ews-watchlist, gyuyoung.kim, kangil.han, saam, sam, ysuzuki

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
patch	none
WIP	none

Tadeu Zagallo

Reported 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.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Tadeu Zagallo

Comment 1 2020-03-06 15:44:15 PST

Created attachment 392792 [details] Patch

Sam Weinig

Comment 2 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?

Yusuke Suzuki

Comment 3 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.

Tadeu Zagallo

Comment 4 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.

Saam Barati

Comment 5 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.

Saam Barati

Comment 6 2021-08-05 15:20:11 PDT

Created attachment 435029 [details] WIP

Note You need to log in before you can comment on or make changes to this bug.