Closed
Bug 634959
Opened 14 years ago
Closed 11 years ago
UndominateInitializers leaks fpvd.table
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: igor, Assigned: jorendorff)
References
Details
(Keywords: regression)
UndominateInitializers, introduced in bug 496134, does not call JS_DHashTableFinish(&fpvd.table) leading to a hashtable leak when a script uses complex destructuring patterns.
Comment 1•14 years ago
|
||
Looks like a leak -- C++ RAII to the rescue!
/be
Comment 2•14 years ago
|
||
(In reply to comment #0)
> leak when a script uses complex destructuring patterns
Curious, what is considered complex? I've been using a lot of:
let {document, gBrowser} = window;
function (someDefaultType) {
let {extra, special} = someDefaultType;
if (extra) ..
else .. someDefaultType
}
addEventListener("keydown", function({keyCode, shiftKey}) ..
Reporter | ||
Comment 3•14 years ago
|
||
(In reply to comment #2)
> (In reply to comment #0)
> > leak when a script uses complex destructuring patterns
> Curious, what is considered complex?
The code switches to the hashtable at http://hg.mozilla.org/tracemonkey/file/1491ae1877c6/js/src/jsparse.cpp#l4327 with the constants defined at http://hg.mozilla.org/tracemonkey/file/1491ae1877c6/js/src/jsparse.cpp#l4267 as:
#define STEP_HASH_THRESHOLD 10
#define BIG_DESTRUCTURING 5
#define BIG_OBJECT_INIT 20
So you need at least 20 elements with at least 5 valid destructing names to hit the bug.
Comment 4•14 years ago
|
||
(In reply to comment #3)
> #define BIG_DESTRUCTURING 5
> #define BIG_OBJECT_INIT 20
> So you need at least 20 elements with at least 5 valid destructing names to hit
> the bug.
Hrmm. Good to know. Thanks. window/event/dom nodes as the RHS will definitely exceed the BIG_OBJECT_INIT. And even with fewer than 5 LHS names, it'll iterate through O(RHS) that many times.
Other than avoiding this leak, does it make sense to convert:
let {altKey, button, ctrlKey, metaKey, shiftKey} = event;
To a bunch of individual assignments? "let altKey = event.altKey;" ?
Comment 5•14 years ago
|
||
Oh... And the leak is O(# RHS properties / .75) or so? JS_DHASH_DEFAULT_CAPACITY(pn->pn_count)
Right now my code does this on every link click:
let {altKey, button, ctrlKey, metaKey, shiftKey} = event;
And [i for (i in event)].length ~100
I don't think I have any destructuring like that for KeyEvents, but those are ~200.
I do have this for every navigator:browser on load:
let {clearInterval, clearTimeout, document, gBrowser, setInterval, setTimeout} = window;
with window having ~500 properties.
Reporter | ||
Comment 6•14 years ago
|
||
(In reply to comment #4)
> (In reply to comment #3)
> > #define BIG_DESTRUCTURING 5
> > #define BIG_OBJECT_INIT 20
> > So you need at least 20 elements with at least 5 valid destructing names to hit
> > the bug.
> Hrmm. Good to know. Thanks. window/event/dom nodes as the RHS will definitely
> exceed the BIG_OBJECT_INIT.
BIG_OBJECT_INIT refers to the complexity of the left-hand side during the parse time, it has nothing to do with the runtime size of the object itself. So the leak is possible only if the destructing pattern contains at least 20 elements.
Assignee | ||
Comment 7•14 years ago
|
||
Stealing. I am going to hack on this function in bug 646820. FindPropValData might go away entirely.
Assignee: igor → jorendorff
Comment 8•11 years ago
|
||
UndominateInitializers was removed in bug 767744. Can this bug be closed now?
Updated•11 years ago
|
Flags: needinfo?(jorendorff)
Assignee | ||
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Updated•11 years ago
|
Flags: needinfo?(jorendorff)
You need to log in
before you can comment on or make changes to this bug.
Description
•