Looking good already ...
https://codereview.chromium.org/13798002/diff/2002/src/spaces.cc
File src/spaces.cc (right):
https://codereview.chromium.org/13798002/diff/2002/src/spaces.cc#newcode2225
src/spaces.cc:2225:
Page::FromAddress(node->address())->AddAvailableInSmallFreeList(
nit: Can we pull out the page into a local "page" variable here. I think
that increases readability. Same goes for the four occurrences below.
https://codereview.chromium.org/13798002/diff/2002/src/spaces.h
File src/spaces.h (right):
https://codereview.chromium.org/13798002/diff/2002/src/spaces.h#newcode551
src/spaces.h:551: 5 * kPointerSize;
As discussed offline: Can we check whether that still fits into the
current page header alignment, or if it bumps the size of the page
header? If it does bump the size we have wiggle-room with the unused
"non_available_small_blocks" field and there should be some room to
squeeze the other fields together.
https://codereview.chromium.org/13798002/diff/2002/src/spaces.h#newcode728
src/spaces.h:728: Page();
There should be no constructor for Pages, they should only be
constructed through the FromAddress() accessors.
https://codereview.chromium.org/13798002/diff/2002/src/spaces.h#newcode808
src/spaces.h:808: void set_available_in_small_free_list(intptr_t
available_in_small_free_list) {
Just an idea. We could use macros to generate this long tail of
accessors. I would prefer the macro approach here, but I'll leave the
final decision up to you.
https://codereview.chromium.org/13798002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
