Bug 63918 – [JSC] WebKit allocates gigabytes of memory when doing repeated string concatenation

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Matthijs van der Vleuten]

On http://files.myopera.com/emoller/blog/webkit.html, WebKit allocates several gigabytes of memory, while other browsers do not. This happens in the current WebKit nightly (r90370) as well as Safari 5.0.5 (6533.21.1), but not in Firefox 5.0 and Chrome 12.0.742.112.

[Dominic Cooney]

I debugged this a little, and I observed the JSC GC is allocating JSStrings with two fibers during string concatenation. These JSC heap objects are small, so JSC GC is not triggered. However there is a lot of string copying in the C++ heap, which is where I think the heap growth comes from.

OP: If you need a workaround, which is probably fragile, you can trick JSC into creating strings with three pieces, which have a different code path, by doing something like changing:

for (var i = offs; i < offs + 10; ++i)
  dest += dest[i];

to this:

for (var i = offs; i < offs + 10; i += 2)
  dest = dest + dest[i] + dest[i + 1];

[Gavin Barraclough]

(In reply to comment #1)
> I debugged this a little, and I observed the JSC GC is allocating JSStrings with two fibers during string concatenation. These JSC heap objects are small, so JSC GC is not triggered.

There is a mechanism that is meant to help deal with this, that looks a little broken.  One of the JSString constructors is calling reportExtraMemoryCost - this should probably be called on all UStrings & StringImpls being passed through the JSString constructors.  (That said, Geoff has been doing a lot of work in the heap lately, might be worth verifying this mechanism is still live & hooked up on the other side).

[Geoffrey Garen]

Yes, reportExtraMemoryCost is the way to fix this.

[Geoffrey Garen]

<rdar://problem/9796985>

[Mark Hahnenberg]

Created attachment 101339 [details]
Patch

[Darin Adler]

Comment on attachment 101339 [details]
Patch

Is there a good way for us to test this?

[Geoffrey Garen]

I'd recommend testing this patch with SunSpider and posting the results here. String concatenation can be a hot operation.

[Geoffrey Garen]

(But I can't think of a good way to unit test this, since the only symptom is memory footprint, and our unit testing framework doesn't have a mechanism for measuring that.)

[Mark Hahnenberg]

Created attachment 101356 [details]
SunSpider results

[Oliver Hunt]

We actually need results of sunspider-compare-results

[Mark Hahnenberg]

Created attachment 101359 [details]
SunSpider results comparison

[WebKit Review Bot]

Comment on attachment 101339 [details]
Patch

Clearing flags on attachment: 101339

Committed r91288: <http://trac.webkit.org/changeset/91288>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.