1575077 - Avoid duplicating URLs for eval/Function scripts

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Ted Campbell [:tcampbell]]

When we implement 'new Function', we create a new ScriptSource and include a copy of filename() and introducerFilename(). This can use a lot of memory, particularly if the URL is a data-url. See Bug 1485078 for examples.

We can fix this by using either JSRopes or doing an adhoc form of that..

Comment 1

[Ted Campbell [:tcampbell]]

Attachment: Bug 1575077 - Use SharedImmutableStringsCache for ScriptSource filenames

Pages that use 'new Function' heavily create a lot of ScriptSource
objects and waste memory duplicating filenames. This is particularly
problematic if the filename is a data-url. Use the existing runtime
strings cache as a straightforward way to share this. The source text
already is using this cache.

Comment 2

[Ted Campbell [:tcampbell]]

This fixes the memory leak for me. I'm going to do a few tweaks tomorrow, but the approach seems like it should work.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1575077 - Factor out NoteContentParse calls in XDR

Move this record/replay-specific code out of the ScriptSource XDR code.

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1575077 - Return UniqueChars from FormatIntroducedName

This was already transferring ownership to caller so use appropriate
types instead.

Depends on D43182

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1575077 - Cleanup handling of ScriptSource::setFilename/setIntroducerFilename

Use accessors instead of directly accessing fields so that we can later
do automatic deduplication. Add setters that can be passed owned strings
when they are available. Remove deprecated xdr->codeCString method.

Depends on D43183

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1575077 - Cleanup ScriptSource::sourceMapURL/displayURL

Add accessors to avoid direct access to fields so storage can be changed
later.

Depends on D43184

Comment 7

[Ted Campbell [:tcampbell]]

Attachment: Bug 1575077 - Use SharedImmutableStringCache for SharedScript urls

Pages that use 'new Function' heavily create a lot of ScriptSource
objects and waste memory duplicating filenames. This is particularly
problematic if the filename is a data-url. Use the existing runtime
strings cache as a straightforward way to share this. The source text
already is using this cache.

Depends on D43185

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6f4e923fed90
Factor out NoteContentParse calls in XDR r=jandem
https://hg.mozilla.org/integration/autoland/rev/c65fa02ac7a4
Return UniqueChars from FormatIntroducedName r=jandem

Comment 9

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/6f4e923fed90
https://hg.mozilla.org/mozilla-central/rev/c65fa02ac7a4

Comment 10

[Ted Campbell [:tcampbell]]

I did a lot of massaging of the XDR code for these strings, but I think it is reasonable enough now. The remaining follow-up is to reduce the size of ScriptSource itself by tweaking the SharedImmutableString structure to not store a copy of cache pointer and to support null values to avoid needing to use Maybe outside.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac703b3adadb
Cleanup handling of ScriptSource::setFilename/setIntroducerFilename r=jandem
https://hg.mozilla.org/integration/autoland/rev/6a2ed5eebc87
Cleanup ScriptSource::sourceMapURL/displayURL r=jandem
https://hg.mozilla.org/integration/autoland/rev/13b9c22b05c8
Use SharedImmutableStringCache for SharedScript urls r=jandem

Comment 12

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/ac703b3adadb
https://hg.mozilla.org/mozilla-central/rev/6a2ed5eebc87
https://hg.mozilla.org/mozilla-central/rev/13b9c22b05c8