Pretty neat

http://codereview.chromium.org/3329019/diff/1/8
File src/ia32/builtins-ia32.cc (right):

http://codereview.chromium.org/3329019/diff/1/8#newcode191
src/ia32/builtins-ia32.cc:191: // To allow for truncation
nit: missing dot at the end of the comment.

http://codereview.chromium.org/3329019/diff/1/8#newcode193
src/ia32/builtins-ia32.cc:193: __ mov(edx,
Factory::one_pointer_filler_map());
if we allocate API objects with the code, it might be a problem due to
internal fields usage (see also Mads' comment in C++ part).  Overall
this difference in the logic might be dangerous

http://codereview.chromium.org/3329019/diff/1/10
File src/objects.cc (right):

http://codereview.chromium.org/3329019/diff/1/10#newcode3294
src/objects.cc:3294:
map->set_visitor_id(StaticVisitorBase::GetVisitorId(map));
why is this needed?  could you add a comment?

http://codereview.chromium.org/3329019/diff/1/10#newcode5417
src/objects.cc:5417: if (unused_inobject_properties != 0) {
if unused_inobject_properties == 0, how would this code behave?
apparently you'll go into started, but not in progress state, is it
intended?

http://codereview.chromium.org/3329019/diff/1/10#newcode5447
src/objects.cc:5447:
set_construct_stub(Builtins::builtin(Builtins::JSConstructStubGeneric));
do we need an assert that current constuct_stub is
JSConstructStubCountdown?

http://codereview.chromium.org/3329019/diff/1/10#newcode5454
src/objects.cc:5454: // Make the counter runs out the next time.
nit: runs -> run

http://codereview.chromium.org/3329019/diff/1/11
File src/objects.h (right):

http://codereview.chromium.org/3329019/diff/1/11#newcode3239
src/objects.h:3239: // via map transitions. Should be applied only to a
map at the top of the
nit: top -> root (of the transition tree)

http://codereview.chromium.org/3329019/diff/1/11#newcode3248
src/objects.h:3248: static const int kMaxShrinkableTreeSize = 50;
just curious: do we have any data on tree size distribution?

http://codereview.chromium.org/3329019/diff/1/11#newcode3713
src/objects.h:3713: #warning Unknown byte ordering
#error?

http://codereview.chromium.org/3329019/diff/1/12
File src/runtime.cc (right):

http://codereview.chromium.org/3329019/diff/1/12#newcode6763
src/runtime.cc:6763: if (first_allocation &&
!shared->IsInobjectSlackTrackingInProgress()) {
isn't it true that IsInobjectSlackTrackingInProgress => Started, hence
you don't need && at all?

http://codereview.chromium.org/3329019/show

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

Reply via email to