Support tokenizing from Latin1 in the innerHTML/DOMParser case
Categories
(Core :: DOM: HTML Parser, enhancement)
Tracking
()
People
(Reporter: hsivonen, Assigned: hsivonen)
References
(Depends on 1 open bug, Blocks 3 open bugs)
Details
As documented in bug 1926474 comment 12, in Speedometer 3.1, most calls to the innerHTML setter (whether tokenizer bypasses or actual tokenizations) use Latin1 input (that does not contain ampersands).
We should support tokenizing directly from Latin1. There are two design axes that need decisions:
- Whether to support any Latin1 input or only Latin1 input that's guaranteed to generate Latin1 output, i.e. Latin1 input that does not contain '\0' or '&'.
- If any Latin1 input is supported, whether to flatten ropes or not.
In bug 1926474, I leaned towards supporting only Latin1 input that's guaranteed to produce Latin1 output. However, on further reflection, considering how element and attribute names are currently looked up with short buffers without SIMD and how attribute values are always UTF-16, it would probably not be too problematic to support dual text node buffers in the tree builder with checks if expansion to UTF-16 is needed.
Tentatively, I'm changing my opinion to try to support any Latin1 input, even '\0' or '&'.
In that case, it would be possible to tokenize per rope segment. The main downside of doing so is that it would defeat SIMD usage, because we'd fall off the SIMD path when reaching the tail of each rope segment instead of falling off the SIMD path only near the end of the whole input.
Tentatively, I think we should flatten the input and not let the tokenizer see the rope segments.
|
Assignee | |
Comment 1•10 months ago
|
||
It makes sense to do bug 1998353 first, so marking this one as dependent on that one, even if it's not a strict ordering.
|
|
Assignee | |
Comment 2•5 months ago
|
||
Base revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193692
Local revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193693
|
|
Assignee | |
Comment 3•5 months ago
|
||
WPT: https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod&landoCommitID=193694
This patch omits the ContainsMarkup check and walks the rope to parse it in the innerHTML setter. I'm worried about ropes with lots of tiny pieces, since there's a fixed cost per rope segment.
|
|
Assignee | |
Comment 4•5 months ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #1)
It makes sense to do bug 1998353 first, so marking this one as dependent on that one, even if it's not a strict ordering.
And it still makes sense to do that, but I'm pursuing this one first.
Furthermore, bug 2035718 and bug 2035717 look important pieces that we should do in order to optimize the perf here.
(In reply to Henri Sivonen (:hsivonen) from comment #3)
This patch omits the
ContainsMarkupcheck and walks the rope to parse it in theinnerHTMLsetter. I'm worried about ropes with lots of tiny pieces, since there's a fixed cost per rope segment.
One thing we could do is to have the ContainsMarkup rope walk discover what the rope is like so that we could make the decision to flatten the rope before tokenizing if it turns out to have lots of tiny pieces.
|
|
Assignee | |
Comment 5•5 months ago
|
||
WPT with ContainsMarkup fixed up:
https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod&landoCommitID=193700
Speedometer3:
https://perf.compare/compare-lando-results?baseLando=193692&newLando=193701&baseRepo=try&newRepo=try&framework=13
Base revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193692
Local revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193701
|
|
Assignee | |
Comment 6•5 months ago
|
||
Linearize the string before rope walk, so the rope walk becomes trivial.
Base revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193692
Local revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193702
|
|
Assignee | |
Comment 7•5 months ago
|
||
Hopefully builds also on try this time.
WPT: https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod&landoCommitID=193717
Base revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193692
Local revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193718
|
|
Assignee | |
Comment 8•5 months ago
|
||
Another try:
WPT: https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod&landoCommitID=193752
Base revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193692
Local revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193753
|
|
Assignee | |
Comment 9•5 months ago
|
||
Instead of walking ropes during parse, flatten to either UTF-16 or Latin1 StringBuffer.
Base revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193692
Local revision's try run: https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=193781
|
|
Assignee | |
Comment 10•5 months ago
|
||
|
|
Assignee | |
Comment 11•4 months ago
|
||
|
|
Assignee | |
Comment 12•4 months ago
|
||
This appears to double the throughput for Latin1 text nodes, but this regresses the performance of handling quoted attribute values.
My current unverified reasoning is this:
Attribute values get expanded to UTF-16, so something causes expansion on a per-attribute value basis to be worse that expanding the whole HTML input string to begin with. When expansion is on a per-attribute value basis, there's an opportunity for a non-SIMD suffix (which may cover the entire attribute value so that there is no SIMD-friendly prefix at all) for every attribute value instead of the opportunity for a non-SIMD suffix once for the whole input.
One way to test this would be to make the per-attribute-value expansion round up to a multiple of 16 (when allocation bounds permit) to convert some trailing garbage, but since bug 1998353 is already a recognized opportunity, it makes sense to implement that first and do trailing garbage writes as part of that work (making the bet that using SIMD for non-garbage prefix outweighs the cost of writing trailing garbage).
Description
•