510244 - XMLParser is much slower than it used to be

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

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.

Comment 1

[Steven Johnson]

Attachment: 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.

Comment 2

[Edwin Smith]

benchmark results?

Comment 3

[Edwin Smith]

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?

Comment 4

[Steven Johnson]

(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

Comment 5

[Steven Johnson]

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

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

Comment 6

[Steven Johnson]

pushed to redux as changeset:   2377:26c273b026bc

Comment 7

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.