I've looked at:
src/ic/*
src/objects*
src/runtime*
test/*
I liked what I saw, just a couple nits.
https://codereview.chromium.org/650073002/diff/500001/src/objects-debug.cc
File src/objects-debug.cc (right):
https://codereview.chromium.org/650073002/diff/500001/src/objects-debug.cc#newcode358
src/objects-debug.cc:358: feedback_vector()->IsTypeFeedbackVector();
CHECK(...)?
https://codereview.chromium.org/650073002/diff/500001/src/objects-printer.cc
File src/objects-printer.cc (right):
https://codereview.chromium.org/650073002/diff/500001/src/objects-printer.cc#newcode485
src/objects-printer.cc:485: os << "\n - ic_total_count: " <<
ic_total_count()
nit: no need to repeat "os", just keep streaming (like the lines below)
https://codereview.chromium.org/650073002/diff/500001/src/objects.cc
File src/objects.cc (right):
https://codereview.chromium.org/650073002/diff/500001/src/objects.cc#newcode10388
src/objects.cc:10388: target->kind() == FUNCTION ? type_feedback_info()
: NULL;
I think you're confused here (or is it a bad merge?). Per the check
above, |target| is an inline cache stub here, never a FUNCTION.
https://codereview.chromium.org/650073002/diff/500001/src/objects.cc#newcode10390
src/objects.cc:10390: feedback_info->IsTypeFeedbackInfo()
feedback_info is NULL here!
https://codereview.chromium.org/650073002/diff/500001/src/runtime-profiler.cc
File src/runtime-profiler.cc (right):
https://codereview.chromium.org/650073002/diff/500001/src/runtime-profiler.cc#newcode72
src/runtime-profiler.cc:72: // Harvest vector-ics as well
I think this should be moved out of the if-block (but maybe needs its
own vector presence check instead?).
https://codereview.chromium.org/650073002/diff/500001/src/type-info.h
File src/type-info.h (right):
https://codereview.chromium.org/650073002/diff/500001/src/type-info.h#newcode29
src/type-info.h:29: bool CallIsMonomorphic(TypeFeedbackId aid);
While you're here: this method doesn't seem to exist any more.
https://codereview.chromium.org/650073002/diff/500001/test/cctest/test-feedback-vector.cc
File test/cctest/test-feedback-vector.cc (right):
https://codereview.chromium.org/650073002/diff/500001/test/cctest/test-feedback-vector.cc#newcode1
test/cctest/test-feedback-vector.cc:1: // Copyright 2011 the V8 project
authors. All rights reserved.
nit: 2014, and use the short license header please.
https://codereview.chromium.org/650073002/diff/500001/test/cctest/test-feedback-vector.cc#newcode72
test/cctest/test-feedback-vector.cc:72: CHECK_EQ(index,
TypeFeedbackVector::kReservedIndexCount);
nit: order is "expected, actual".
https://codereview.chromium.org/650073002/diff/500001/test/cctest/test-feedback-vector.cc#newcode89
test/cctest/test-feedback-vector.cc:89: // Empty vectors are the empty
fixed array.
comment out of place?
https://codereview.chromium.org/650073002/diff/500001/test/cctest/test-feedback-vector.cc#newcode104
test/cctest/test-feedback-vector.cc:104:
CHECK_EQ(vector->Get(FeedbackVectorSlot(0)), Smi::FromInt(1));
nit: order is "expected, actual" (again below).
https://codereview.chromium.org/650073002/
--
--
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/d/optout.
