Skip to content

perf: read text between tags in one slice and skip CRLF normalization when there is no CR - #918

Open
deadstrobe5 wants to merge 1 commit into
NaturalIntelligence:masterfrom
deadstrobe5:perf/text-slice
Open

deadstrobe5 wants to merge 1 commit into
NaturalIntelligence:masterfrom
deadstrobe5:perf/text-slice

Conversation

@deadstrobe5

Copy link
Copy Markdown

Purpose / Goal

Text content costs much more than it should. parseXml appends text one character per loop iteration (textData += xmlData[i]), and before that xmlData.replace(/\r\n?/g, ...) scans the whole input even when it has no \r at all. On documents with large text nodes (base64 payloads, prose, feeds) these two loops are most of the parse time.

This PR copies each run of text up to the next < with one indexOf and substring, then moves the loop index to the end of the run. The string handed to saveTextToParentTag is the same as before (same start, same end at the next < or the end of input), so trimming, entity decoding, value parsing and the callbacks see the same input. The CRLF replace now runs only when the input contains a \r; without one it was an identity replace, and crlfPositions stays empty as before.

Same-process A/B on Node 24, ignoreAttributes: false, median of 7 alternating rounds:

input before after
SOAP response, 64 KB base64 text node 1.51 ms 0.035 ms
SOAP response, 1 MB base64 text node 57.5 ms 0.11 ms
RSS, 200 items (217 KB) 13.5 ms 7.4 ms
S3 listing, 1000 keys (184 KB) 28.7 ms 21.4 ms
S3 error response (216 B) 29.5 us 20.9 us

The new spec checks long text with emoji and accents, mixed text around child tags, comments, entities and CDATA, with and without preserveOrder, and passes on master too. A differential fuzz against master (random documents with entities, CDATA, comments, PIs, CR and CRLF, captureMetaData, 9 option sets, 54,000 comparisons, including reused parser instances) gave identical output, metadata and errors.

Type

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant