Thanks for looking at this, Anton. See my comments inline.
Both Vitaly and me don't like the increasing complexity of the code, so VItaly is trying to see if there is a better approach. I tried/thought about several
last week but they really do not play well (single linked list is really not very flexible data structure). http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc File src/global-handles.cc (right): http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc#newcode114 src/global-handles.cc:114: void GlobalHandles::Node::ClearWeakness(GlobalHandles* global_handles) { On 2011/05/23 19:44:19, antonm wrote:
do we want to persist independence related state across
MakeWeak/ClearWeakness
calls, maybe they should reset independence flags?
I think independence is more a property of an object not a property of a handle so I don't think ClearWeak should clean it. It's just an unfortunate design tradeoff that independence became property of a handle.
And shouldn't clearweakness move the handler out of the suffix as it's
not
collectible anymore?
The invariant of the tail is: if object is young and independent it's in the tail. But converse of the invariant does not have to hold. I will update the comments to make it more clear. http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc#newcode221 src/global-handles.cc:221: } else if (first_deallocated()) { On 2011/05/23 19:44:19, antonm wrote:
just FYI: I recently thought about it, and chances really are we
don't need
both free and deallocated since we allocate the stuff from the pool
anyway. I
should have probably cleaned it up before. In any event it may
compensate for
newly added complexity.
Well if we can remove "free" stuff than I don't need a double linked list --- a single linked list will suffice. If there is not "reuse" in the middle of the list then the following assumption will hold --- a handle is mark as independent soon after creation so we can find it's "prev" by linear search. What do you think, Vitaly? http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc#newcode272 src/global-handles.cc:272: if (node->is_in_independent_tail_) { On 2011/05/23 19:44:19, antonm wrote:
is it possible for this to be true? if node wasn't independent, it
should not
have belonged to independent tail unless there is a bug, no?
Yes it is. If the handle is destroyed and reused without GC in between. Also see the comment about invariant of the tail above. http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc#newcode347 src/global-handles.cc:347: if (!current->independent_) continue; On 2011/05/23 19:44:19, antonm wrote:
shouldn't it always hold: ASSERT(current->independent_)? At least the
name
suggests that :)
It was an assertion initially but... see above :-) http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc#newcode361 src/global-handles.cc:361: if (!current->independent_) continue; On 2011/05/23 19:44:19, antonm wrote:
ditto
ditto :-) http://codereview.chromium.org/7062004/diff/1/src/global-handles.cc#newcode420 src/global-handles.cc:420: node->is_in_independent_tail_ = false; On 2011/05/23 19:44:19, antonm wrote:
is it mandatory here? apparently you overwrite it later anyway.
there is one callsite that calls UnlinkNode directly (see line 475). http://codereview.chromium.org/7062004/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
