1275282 - 1.83MB of empty strings on irccloud

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

I just noticed this in my about:memory output for irccloud:

1.83 MB (00.13%) ── string(length=0, copies=79786, "")/gc-heap/latin1

That's 1.83MB of memory for empty strings.  Each string takes at least 24 bytes on 64-bit, so that numbers are correct.  It just seems sub-optimal to make so many copies of the same value.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Note, I'm on FF46.0.1 on windows 10.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Stack to where we allocate length-0 strings here (or at least _a_ place):

NewStringDeflated (cx=0x1282a7400, s=0x13ac02e90, n=0)
js::NewStringCopyN<(js::AllowGC)1, char16_t>(cx=0x1282a7400, s=0x13ac02e90, n=0)
js::JSONParser<char16_t>::readString<(js::JSONParserBase::StringType)1>()

Comment 3

[Luke Wagner [:luke]]

Comment on attachment 8756229 [details] [diff] [review]
Patch

Review of attachment 8756229 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

In some places we checked for length 1 static strings, but not length 0 and length 2. Measurements indicate static strings can be used for most of length 2 strings as well.

The patch cleans this up and checks |length <= 2|.

I also changed StaticStrings::init to call NewInlineString directly, to bypass a bunch of unnecessary checks.

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Patch

The n == 0 checks in JS_NewStringCopyN and JS_NewStringCopyZ are now redundant so we can remove them.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Patch

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f5902db90a7

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1f5902db90a7