Another small comment: Kevin made me aware that Page has an is_valid()
method that can be used instead of explicit checking Page pointers
against NULL. Also, it seems that in most other code, we use
Page::FromAddress(NULL) instead of using NULL explicitly in
initializations.


Yeah, not using is_valid() is my fault because I was aware of it's
existence.

I will prepare small a cleanup CL including:
- using is_valid() instead of NULL-checks, using Page::FromAddress(NULL)
instead of initialization with NULL
- using CreateFillerArrayAt instead of current dummy-free-list-node travesty
[I was unaware of CreateFillerArrayAt before yesterday]
- Win64 build fix [probably I will need to use someone's Windows box to test
this build issues if it is possible]

--
Vyacheslav Egorov

On Fri, Apr 23, 2010 at 8:48 AM, <[email protected]> wrote:

> LGTM
>
>
> http://codereview.chromium.org/1700005/diff/6001/7004
> File src/mark-compact.cc (right):
>
> http://codereview.chromium.org/1700005/diff/6001/7004#newcode1396
> src/mark-compact.cc:1396: ASSERT(((first_empty_page == NULL) &&
> Now that you have an #ifdef DEBUG here in any case it would be more
> readable with ifs and multiple ASSERTS. :)
>
> Another small comment: Kevin made me aware that Page has an is_valid()
> method that can be used instead of explicit checking Page pointers
> against NULL. Also, it seems that in most other code, we use
> Page::FromAddress(NULL) instead of using NULL explicitly in
> initializations.
>
>
> http://codereview.chromium.org/1700005/show
>

-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to