Closed Bug 1575077 Opened 5 months ago Closed 5 months ago

Avoid duplicating URLs for eval/Function scripts

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: tcampbell, Assigned: tcampbell)

References

(Blocks 2 open bugs)

Details

Attachments

(5 files, 1 obsolete file)

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..

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.

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

Attachment #9086806 - Attachment is obsolete: true

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

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

Depends on D43182

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

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

Depends on D43184

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

Keywords: leave-open
Pushed by tcampbell@mozilla.com:
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

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.

Pushed by tcampbell@mozilla.com:
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
Blocks: 1576885
Keywords: leave-open
See Also: → 1576999
You need to log in before you can comment on or make changes to this bug.