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
