Cleanup memory ownership for LazyScriptCreationData
Categories
(Core :: JavaScript Engine, task, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox71 | --- | fixed |
People
(Reporter: tcampbell, Assigned: mgaudet)
References
Details
Attachments
(4 files, 1 obsolete file)
See Bug 1577580.
The arrays for the LazyScriptCreationData are current allocated on system allocator while the FunctionBoxes that own them are allocated on Parser LifoAlloc. I added a work-around to manually trace this during cleanup, but we should fix this in a more robust way.
Another thing to consider here is if we can allocate the LazyScriptData directly at this point and then later transfer ownership to the LazyScript. This would require some cleverness (or just a flag..) to start with inner functions as FunctionBox* and later re-placement-new that sub-array as GCPtr<JSFunction>.
|
Reporter | |
Comment 1•5 years ago
|
||
Matt, when you are back can you dig into this. We seem to have made a design error with how we manage some of the memory for FunctionBoxTree.
|
Assignee | |
Comment 2•5 years ago
|
||
-
Hosts the UsedNameTracker and and the FunctionTreeHolder into a new ParseInfo
strucutre. -
ParseInfo also handles the lifetime of the parser LifoAllocScope, which
gives us an opportunity to inspect and cleanup objects after the parse is
done. -
Split FunctionTree into its own header file.
-
Removes allocator argument to Parser construction in lieu of the LifoAllocScope
inside of ParseInfo.
Combined with the change in D44272, this fixes memory leaks from LazyScriptCreationData
both in eager and deferred mode.
|
|
Assignee | |
Updated•5 years ago
|
|
||
Updated•5 years ago
|
|
|
Assignee | |
Comment 3•5 years ago
|
||
|
|
Assignee | |
Comment 4•5 years ago
|
||
- Hoists the UsedNameTracker and and the FunctionTreeHolder into a new ParseInfo structure.
- Make LifoAllocScopes lifetimes lexically explicit throughout parser.
- Removes allocator argument to Parser construction in lieu of the LifoAllocScope inside of ParseInfo.
Depends on D45041
|
|
Assignee | |
Comment 5•5 years ago
|
||
FunctionBox is LifoAlloc'd, but holds on to non-LifoAlloc'd memory. This
ensures we clean up the memory owned by the FunctionBox before the LifoAlloc
cleans up the FunctionBox, to avoid leaking non-LifoAlloc'd memory.
Depends on D45042
|
|
Assignee | |
Comment 6•5 years ago
|
||
In the presence of FunctionBoxes with null gcThings, we cannot tell the type of
all TraceNodeList entries reliably. This adds a type tag to allow
disambiguation.
Depends on D45042
Pushed by mgaudet@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a67d5dbc29cc Hoist FunctionTree into its own header r=tcampbell https://hg.mozilla.org/integration/autoland/rev/6bedd2e286af Create ParseInfo class to manage Parser related information r=tcampbell https://hg.mozilla.org/integration/autoland/rev/6f60141452d1 Add a tag to TraceNodeList to disambiguate types r=tcampbell https://hg.mozilla.org/integration/autoland/rev/8e9904f026ee Free memory before LifoAlloc cleans up memory owner r=tcampbell
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/a67d5dbc29cc
https://hg.mozilla.org/mozilla-central/rev/6bedd2e286af
https://hg.mozilla.org/mozilla-central/rev/6f60141452d1
https://hg.mozilla.org/mozilla-central/rev/8e9904f026ee
Description
•