refactor(tokenizer): Use decorators for even faster parsing - #1011
Conversation
Pull Request Test Coverage Report for Build 1454072605
💛 - Coveralls |
This leads to another ~5% speed-up. Not sure if it is worth it though, as the code does get quite a bit less understandable.
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/Tokenizer.ts">
<violation number="1" location="src/Tokenizer.ts:166">
P1: When a decorated state method receives a character, this wrapper does not enter that state. Consequently, `&&` remains in `Text`, and whitespace after `=` leaves `foo= bar` in `AfterAttributeName`, treating `bar` as another attribute; assign `this.state = state` before invoking `originalMethod`.</violation>
<violation number="2" location="src/Tokenizer.ts:628">
P1: When a CDATA opener is split across writes after its first character, `stateBeforeDeclaration` resets `sequenceIndex` after `stateCDATASequence` has consumed `C`, so the next chunk is parsed as a declaration. Initialize `sequenceIndex` before invoking `stateCDATASequence`.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| this.stateCDATASequence(this._next()!); | ||
| this.sequenceIndex = 0; |
There was a problem hiding this comment.
P1: When a CDATA opener is split across writes after its first character, stateBeforeDeclaration resets sequenceIndex after stateCDATASequence has consumed C, so the next chunk is parsed as a declaration. Initialize sequenceIndex before invoking stateCDATASequence.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/Tokenizer.ts, line 628:
<comment>When a CDATA opener is split across writes after its first character, `stateBeforeDeclaration` resets `sequenceIndex` after `stateCDATASequence` has consumed `C`, so the next chunk is parsed as a declaration. Initialize `sequenceIndex` before invoking `stateCDATASequence`.</comment>
<file context>
@@ -400,216 +438,253 @@ export default class Tokenizer {
+ private stateBeforeDeclaration(c: number): void {
if (c === CharCodes.OpeningSquareBracket) {
- this._state = State.CDATASequence;
+ this.stateCDATASequence(this._next()!);
this.sequenceIndex = 0;
+ } else if (c === CharCodes.Dash) {
</file context>
| this.stateCDATASequence(this._next()!); | |
| this.sequenceIndex = 0; | |
| this.sequenceIndex = 0; | |
| this.stateCDATASequence(this._next()!); |
| descriptor.value = function (this: Tokenizer, c: number | null) { | ||
| if (c !== null) { | ||
| originalMethod.call(this, c); | ||
| } else { | ||
| this.state = state; | ||
| } | ||
| }; |
There was a problem hiding this comment.
P1: When a decorated state method receives a character, this wrapper does not enter that state. Consequently, && remains in Text, and whitespace after = leaves foo= bar in AfterAttributeName, treating bar as another attribute; assign this.state = state before invoking originalMethod.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/Tokenizer.ts, line 166:
<comment>When a decorated state method receives a character, this wrapper does not enter that state. Consequently, `&&` remains in `Text`, and whitespace after `=` leaves `foo= bar` in `AfterAttributeName`, treating `bar` as another attribute; assign `this.state = state` before invoking `originalMethod`.</comment>
<file context>
@@ -139,9 +139,70 @@ const Sequences = {
+ const originalMethod: (this: Tokenizer, c: number) => void =
+ descriptor.value;
+
+ descriptor.value = function (this: Tokenizer, c: number | null) {
+ if (c !== null) {
+ originalMethod.call(this, c);
</file context>
| descriptor.value = function (this: Tokenizer, c: number | null) { | |
| if (c !== null) { | |
| originalMethod.call(this, c); | |
| } else { | |
| this.state = state; | |
| } | |
| }; | |
| descriptor.value = function (this: Tokenizer, c: number | null) { | |
| this.state = state; | |
| if (c !== null) { | |
| originalMethod.call(this, c); | |
| } | |
| }; |
This leads to another ~5% speed-up. Not sure if it is worth it though, as the code does get quite a bit less understandable.