Make GlobalHandle::NodeBlock deletable

Make GlobalHandle::NodeBlock deletable

R=mstarzinger@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=16040

[Michael Starzinger, 2013-08-01 12:29:01]

LGTM (with a few comments). This is a maintenance nightmare.

https://codereview.chromium.org/21042004/diff/17001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/21042004/diff/17001/src/global-handles.cc#new...
src/global-handles.cc:61: LAST_STATE = NEAR_DEATH
Let just rename this to STATE_COUNT or NUMBER_OF_STATES and drop the explicit
assignment of this constant.

https://codereview.chromium.org/21042004/diff/17001/src/global-handles.cc#new...
src/global-handles.cc:554: int total_slots =
global_handles->number_of_blocks_*NodeBlock::kSize;
nit: Whitespace around operators. Here and several other occurrences throughout
the file.

https://codereview.chromium.org/21042004/diff/17001/src/global-handles.cc#new...
src/global-handles.cc:640: static const int kCountArraySize = Node::LAST_STATE +
1;
This constant is obsolete, see comment at the top of this file.

https://codereview.chromium.org/21042004/diff/17001/src/global-handles.h
File src/global-handles.h (right):

https://codereview.chromium.org/21042004/diff/17001/src/global-handles.h#newc...
src/global-handles.h:159: int block_count() const { return number_of_blocks_; }
nit: Let's add a short one-liner comment to the accessor as well.

https://codereview.chromium.org/21042004/diff/17001/test/cctest/test-global-h...
File test/cctest/test-global-handles.cc (right):

https://codereview.chromium.org/21042004/diff/17001/test/cctest/test-global-h...
test/cctest/test-global-handles.cc:362: for (int i = 0; i < kBlockSize*100; i++)
{
nit: Whitespace around the multiplication operator.

https://codereview.chromium.org/21042004/diff/17001/test/cctest/test-global-h...
test/cctest/test-global-handles.cc:371: #ifdef DEBUG
nit: The C macro pragmas should not be indented.

[Sven Panne, 2013-08-01 12:44:49]

As discussed offline with Michael: Wouldn't it be much easier and far more
readable with:

   * Use 2 lists instead of a partitioned one. Iterators would make it easy to
iterate over all nodes or only over one of the parts.

   * Use a circular doubly linked list with the standard "looped" sentinel for
the empty list.

[dcarney, 2013-08-02 11:12:36]

please review this lastest round of awesome

[Michael Starzinger, 2013-08-02 11:59:51]

Much better, a few more comments and nits.

https://codereview.chromium.org/21042004/diff/25001/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/21042004/diff/25001/src/global-handles.cc#new...
src/global-handles.cc:316: : anchor_(anchor), current_(anchor->next()) {
nit: Indentation is off.

https://codereview.chromium.org/21042004/diff/25001/src/global-handles.cc#new...
src/global-handles.cc:689: BlockList* anchors[2] = { &full_blocks_,
&non_full_blocks_ };
Can we hoist out this list of anchors to be a field on GlobalHandles so that we
have a single place where all anchors are defined?

https://codereview.chromium.org/21042004/diff/25001/src/global-handles.cc#new...
src/global-handles.cc:902: BlockList* anchors[2] = {&full_blocks_,
&non_full_blocks_};
See comment about.

https://codereview.chromium.org/21042004/diff/25001/src/global-handles.h
File src/global-handles.h (right):

https://codereview.chromium.org/21042004/diff/25001/src/global-handles.h#newc...
src/global-handles.h:328: int LengthOfFreeList();
nit: Indentation is off.

[dcarney, 2013-08-05 07:34:37]

Committed patchset #7 manually as r16040 (presubmit successful).