Closed Bug 634959 Opened 14 years ago Closed 11 years ago

UndominateInitializers leaks fpvd.table

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

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.
Looks like a leak -- C++ RAII to the rescue! /be
(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}) ..
(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.
(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;" ?
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.
(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.
Blocks: 646820
Stealing. I am going to hack on this function in bug 646820. FindPropValData might go away entirely.
Assignee: igor → jorendorff
UndominateInitializers was removed in bug 767744. Can this bug be closed now?
Flags: needinfo?(jorendorff)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → WORKSFORME
Flags: needinfo?(jorendorff)
You need to log in before you can comment on or make changes to this bug.