On Wed, Sep 15, 2010 at 11:05 AM, Vitaly Repeshko <[email protected]> wrote: > On Wed, Sep 15, 2010 at 11:33 AM, Mads Sig Ager <[email protected]> wrote: >> Sounds like a good plan to me! :) >> >> I don't think I understand the part about "by the time we decide to >> resize the map there might be no live objects created from this >> particular map". Does that matter? It seems to me that it would still >> be a good idea to update the inobject-property count? > > The problem is that if we don't track slack in the new maps as they > appear, we may get into a state (after a gc) where there are no live > objects related to the initial map and all the maps but the initial > map are gone, and so we have nothing to deduce the right slack size > from.
Good point. So we shouldn't do it if the subtree is empty which should be a rare case. Any other concerns? That one seems very simple to handle? Cheers, -- Mads >> On Tue, Sep 14, 2010 at 5:35 PM, Vladislav Kaznacheev >> <[email protected]> wrote: >>> Thanks for the comments! >>> We had a long discussion with Vitaly on how to proceed, and here is what we >>> came up with (let me know if you find anything of the following stupid): >>> - Updating the slack estimation on every map transition is too fragile and >>> does not provide any performance benefit. I will get rid of it and compute >>> the slack by traversing the tree when the counter runs out. Vitaly sent me a >>> piece of code that does the stackless magic :) >>> - Canceling the tracking when the second context starts creating objects is >>> a bad idea (too much potential for memory waste). In order to fix this I >>> will store the initial_map from the first context in shared function info, >>> so that when the second context arrives I can complete slack computation for >>> the first context and adjust expected_nof_properties for all contexts to >>> use. This will require adding one extra pointer to shared function info, >>> which is no big deal because I can stow away all other tracking info (one or >>> two flags and a counter) in the unused bits of some other fields. >>> - If the second context starts creating objects too soon after the first one >>> (e.g. after 1 or 2 allocations), we will have to do the resizing based on >>> rather limited feedback. The good news is that we do not have to squeeze out >>> all of the slack. We can leave 1 or 2 free inobject slots and basically have >>> the same performance as we have currently (today the default slack is 2). >>> - One interesting consequence of not tracking map transitions is that by the >>> time we decide to resize the map there might be no live objects created from >>> this particular map (this would mean that GC occurred after we started >>> tracking). Cutting out all of the slack would be too aggressive. Instead we >>> could continue tracking (resetting the counter to 16 or whatever) and hope >>> that GC does not hit too soon. If it keeps hitting often we end up having up >>> to 16 generously allocated objects all the time. Another option will be >>> keeping the default slack. >>> - We agreed that the state machine responsible for handling all this logic >>> should be clearly documented in one place. >>> Please let me know what you think, >>> Best regards, >>> Vlad >>> >>> On Tue, Sep 14, 2010 at 5:24 PM, <[email protected]> wrote: >>>> >>>> A few more comments. >>>> >>>> >>>> http://codereview.chromium.org/3329019/diff/1/10 >>>> File src/objects.cc (right): >>>> >>>> http://codereview.chromium.org/3329019/diff/1/10#newcode5434 >>>> src/objects.cc:5434: >>>> set_inobject_slack_estimation(unused_inobject_properties); >>>> If unused_inobject_properties is 0, then this half-cancels the tracking >>>> (IsInobjectSlackTrackingInProgress() becomes false). Is this a problem? >>>> If not, it's still too subtle and requires a comment. >>>> >>>> http://codereview.chromium.org/3329019/diff/1/10#newcode5454 >>>> src/objects.cc:5454: // Make the counter runs out the next time. >>>> Why do we have to postpone it? Can we call Complete...Tracking() here? >>>> >>>> http://codereview.chromium.org/3329019/diff/1/11 >>>> File src/objects.h (right): >>>> >>>> http://codereview.chromium.org/3329019/diff/1/11#newcode3246 >>>> src/objects.h:3246: // The size limit for a map transition tree that can >>>> be safely processed >>>> If we have a "class" with 51 functions, does this limit mean we'll >>>> always be too generous when allocating the instances? If yes, this could >>>> be bad for memory usage. I understand that this limit should not affect >>>> the normal usage of putting the functions on the prototype, but some OO >>>> frameworks put the functions on the instance. >>>> >>>> Please have a look at http://codereview.chromium.org/3385002 where I >>>> attempt to do a stackless traversal of the map tree. >>>> >>>> http://codereview.chromium.org/3329019/diff/1/11#newcode3248 >>>> src/objects.h:3248: static const int kMaxShrinkableTreeSize = 50; >>>> kMaxShrinkable*Transition*TreeSize? >>>> >>>> http://codereview.chromium.org/3329019/diff/1/11#newcode3457 >>>> src/objects.h:3457: // Inobject slack tracking: >>>> This comment needs to also explain how different contexts sharing a >>>> function are affected by tracking. >>>> >>>> http://codereview.chromium.org/3329019/diff/1/11#newcode3463 >>>> src/objects.h:3463: // To reclaims unused inobject space we: >>>> reclaims -> reclaim. >>>> >>>> http://codereview.chromium.org/3329019/diff/1/11#newcode3488 >>>> src/objects.h:3488: inline bool IsInobjectSlackTrackingInProgress(); >>>> Please describe the full state machine here. >>>> >>>> http://codereview.chromium.org/3329019/show >>> >>> >> > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
