Closed Bug 563711 Opened 12 years ago Closed 12 years ago

Optimize nsHTMLContentSerializer::AppendAndTranslateEntities


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: smaug, Unassigned)


(Blocks 1 open bug)



(3 files, 1 obsolete file)

Attached file testcase
If there are lots of entities to translate, nsHTMLContentSerializer::AppendAndTranslateEntities takes
way too much time of ::GetInnerHTML (like 66%).
String modifications and memory (de)allocation are the ones which take
most of the time.
I'm trying to do something.
Attached patch wipSplinter Review
The result of the test on my computer with the current trunk is: total: 91999, avg 183.998
with this patch: total: 84945, avg 169.89

It's better, but not as better than safari: total: 63616, avg 127.232

Olli, do you have ideas for other improvements ?
Well, we should improve string handling in serializers in general, but in this 
particular case I think AppendASCIItoUTF16 is pretty heavy.
It needs to increase the capacity of aOutputStr all the time.

So, perhaps collecting strings to some array and then call
SetCapacity on the aOutputStr only once after loops and then Append strings to 
aOutputStr could help, maybe.
Also, I think string.AppendASCII(ascii) is actually faster than 
AppendASCIItoUTF16(ascii, string).
The latter one creates some extra temporary string objects.
I wonder if there is some other difference in the behavior.
Another testcase is in bug 561717
Laurent, do you want a review for the patch?
(gecko is already in many innerHTML cases faster than trunk webkit, and
would be great to get faster serializers. This would be one step.)
Attached patch small fixes (obsolete) — Splinter Review
This uses AppendASCII, which is faster and ok to use at least in this case.
Added also the missing Truncate call.
Attachment #444857 - Flags: review?(laurent)
Attached patch a bit simplerSplinter Review
Jonas, perhaps you could review this.
Attachment #444857 - Attachment is obsolete: true
Attachment #444961 - Flags: review?(jonas)
Attachment #444857 - Flags: review?(laurent)

Marking this fixed once this has passed the tests.
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.