XMLParser is much slower than it used to be

VERIFIED FIXED

Status

VERIFIED FIXED
9 years ago
9 years ago

People

(Reporter: stejohns, Assigned: stejohns)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
The String rewrite of a few months ago necessitated changes to XMLParser16.cpp; this ended up making it about half as fast as it used to be. Performance needs to be restored to (at least) old levels.
(Assignee)

Comment 1

9 years ago
Created attachment 394386 [details] [diff] [review]
Patch #1

Lots of optimizations to XMLParser to regain lost performance:

-- add String::intern_substring, which creates an interned substring in one step. This reduces the need to create an intermediate substring which is immediately thrown away. (As a result of this, added AvmCore::findStringLatin1).

-- added String::equals() as a fast-path for comparing entire strings, avoiding extra work done by String::Compare().

-- split String::matchesLatin1() into caseless and case-sensitive versions, since we never make this decision at runtime, and optimized the case-sensitive version.

-- added String::indexOfCharCode() to optimize indexOf() when len=1 

-- inlined and optimized String::isSpace()

-- inlined and optimized StringIndexer

-- rewrote code in XMLObject and XMLList to avoid calling substring()/substr() just to do a comparision; calling matchesLatin1() or Compare() with appropriate offsets is much faster than creating a temporary string.

-- re-added an optimization to XMLParser::unescape that had gotten lost: if the string has no "&", bail early.

-- change the various String::hashCode variants to accumulate in a signed (vs unsigned) int; produces much better hash code.
Attachment #394386 - Flags: review?(edwsmith)

Comment 2

9 years ago
benchmark results?

Comment 3

9 years ago
Comment on attachment 394386 [details] [diff] [review]
Patch #1

Some comments in the various cases in String::equals() would be helpful.

StringObject.cpp: "must be signed" comment is nice, but why must it be signed?
Attachment #394386 - Flags: superreview?(daumling)
Attachment #394386 - Flags: review?(edwsmith)
Attachment #394386 - Flags: review+
(Assignee)

Comment 4

9 years ago
(In reply to comment #3)
> (From update of attachment 394386 [details] [diff] [review])
> StringObject.cpp: "must be signed" comment is nice, but why must it be signed?

yeah, should read "must match signedness of the other hash functions", will update
(Assignee)

Comment 5

9 years ago
(In reply to comment #2)
> benchmark results?

Best benchmark was a torture-test in Flash that went from ~4000ms to ~2300ms.
(Assignee)

Comment 6

9 years ago
pushed to redux as changeset:   2377:26c273b026bc
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Updated

9 years ago
Attachment #394386 - Flags: superreview?(daumling)

Comment 7

9 years ago
Resolved fixed engineering / work item that has been pushed.  Setting status to verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.