+++ This bug was initially created as a clone of Bug #411575 +++ The bug 411575 exposed that js_PutCallObject is slow because it creates properties representing variables and argument names. This is an expensive operation and the proposed solution in the bug is to create the properties lazily while using the reserved slots to store the value of frame variable and arguments on the moment of frame exit. Since js_PutBlockObject has exactly the same structure as the problematic js_PutCallObject, I suggest to optimize in the same way as the proposed solution for js_PutCallObject. That is, the block object should use the reserved slots to store the values of the let variables after the code leaves the let block.

Recording fix order code dependencies.

Marking blocking since it blocks a blocker.

Reversing patch dependency: bug 411575 is not 1.9 blocker and can be fixed after fixing this one.

This patch is build on top of the patch from bug 389605 comment 14.

I am getting errors about reserved slot beyond the range with this test. It seems like a bug in JSOP_SETNAME/JSOP_SETPROP implementation.

What's the latest status here? This is still blocking a relatively leak-prone blocker.

(In reply to comment #6) > What's the latest status here? This is still blocking a relatively leak-prone > blocker. The patch here needs a fix for bug 428282.

This what I will test before asking for a review.

Comment on attachment 318117 [details] [diff] [review] v1 Note that the reason the patch renames ReallocSlots into js_ReallocSlots is to minimize the patch for bug 411575. It will need to call the function from jsfun.h.

(In reply to comment #9) > (From update of attachment 318117 [details] [diff] [review]) > Note that the reason the patch renames ReallocSlots into js_ReallocSlots is to > minimize the patch for bug 411575. It will need to call the function from > jsfun.h. I meant from "jsfun.c"

Comment on attachment 318117 [details] [diff] [review] v1 >+ /* For a block object we have one available slot among fixed slots. */ >+ JS_STATIC_ASSERT(JS_INITIAL_NSLOTS == JSSLOT_BLOCK_DEPTH + 2); Comment might rather say "Blocks have one fixed slot available for the first local." >+ /* >+ * Block objects should never be exposed to scripts. Thus the clone should >+ * not own the property map and rather always share it with the original s/original/prototype/ >+ * object. It allows to skip updating OBJ_SCOPE(obj)->map.freeslot after s/It allows/This allows us/ >+ * we copy below the stack slots into reserved slots. s/below// >+ /* Block objects should not have reserve slots before they are put. */ s/reserve slots/reserved slots/ >+ JS_ASSERT(STOBJ_NSLOTS(obj) == JS_INITIAL_NSLOTS); >+ >+ /* Block and its locals must be on the current stack for GC safety. */ s/Block/The block/ >+ /* Reserve slots counts from the first slot after the private. */ s/Reserve slots counts from/Reserved slots start with/ Same below in block_setProperty. >@@ -2884,17 +2905,17 @@ js_FreeSlot(JSContext *cx, JSObject *obj > > map = obj->map; > JS_ASSERT(!MAP_IS_NATIVE(map) || ((JSScope *)map)->object == obj); > LOCKED_OBJ_SET_SLOT(obj, slot, JSVAL_VOID); > if (map->freeslot == slot + 1) { > map->freeslot = slot; > > /* When shrinking ReallocSlots always returns true. */ >- ReallocSlots(cx, obj, slot, JS_FALSE); >+ js_ReallocSlots(cx, obj, slot, JS_FALSE); Existing comemnt should say js_ReallocSlots -- would be nice to add a comma before that (after "shrinking") too. r=me with nits picked. Thanks, /be

The new version of the patch addresses the nit from the comment 11.

Comment on attachment 318872 [details] [diff] [review] v1b a=shaver. (Approving last JS blocker? I have lived my whole life for this day.)

I checked in the patch from the comment 12 to the trunk: http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1209679099&maxdate=1209679200&who=igor%25mir2.org

Bug 432172 possibly caused by this?

(In reply to comment #15) > Bug 432172 possibly caused by this? The patch affects only the code that defines closures or calls exec in a block with let variables. Moreover, in such cases it should save memory as the patch uses shared properties of the let block object that allows to skip creation of properties when the code leaves the block. So in theory the patch can not be responsible for the malloc jump. Of cause due to a bug the patch can cause a jump in malloc and to check this I can backout the patch.

/cvsroot/mozilla/js/tests/js1_8/regress/regress-427798.js,v <-- regress-427798.js initial revision: 1.1 mozilla-central: 16423:d516f037d2a6