832024 - Code generation in js::StringBuffer::infallibleAppend is not optimal

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Joe Olivas]

Attachment: Hot loop in js::StringBuffer::infallibleAppend

When looking at the V8 RegExp workload, I am seeing about 7-8% of active time spent in js::StringBuffer::infallibleAppend (vm/StringBuffer.h:69). The codegen for this loop is the following:

452A40	mov bx, word ptr [eax]
452A43	mov word ptr [edx], bx
452A46	add eax, 2
452A49	add edx, 2
452A4C	cmp eax, esi
452A4E	jnz 452a40

Our tools show this loop has an average trip count of 9, which means that it could benefit from larger copies than "word"s when the size parameter is larger.

The CPI of the loops is pretty optimal (0.32 vs. maximum of 0.25), so pushing through more data with fewer instructions would help.

I've attached a screenshot from our tools.

Comment 1

[Sean Stangl [:sstangl]]

When was the measurement made? Bug 820124 (landed on the 15th) added a new function str_replace_regexp_remove() that handles all benchmark str_replace() cases using Ropes. StringBuffer was used heavily by the old code, but the new situation should be much better.

Comment 2

[Joe Olivas]

I built this earlier this week, and just got around to measuring today, so perhaps this is invalid. Let me update and recheck.

Sorry about that.

Comment 3

[Jeff Walden [:Waldo]]

infallibleAppend is used for much more than just v8-regexp stuff, so it's unlikely this is flat-out invalid.  Not relevant to v8-regexp performance, perhaps, but almost certainly relevant to performance in other cases.

Comment 4

[Joe Olivas]

I re-tested a few minutes ago against v8-regexp, and this function disappeared from the hot functions. It appears that this is not critical for that benchmark, though may still be valid for other uses, as mentioned above.

As a bonus, the score in RegExp went from 1900 to 3000.