Addressed comments, please take a look
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/factory.cc
File src/factory.cc (right):
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/factory.cc#newcode918
src/factory.cc:918: if (LinearSearchUnsorted(*result, *key,
descriptor_count) ==
On 2012/06/01 13:49:55, danno wrote:
Add a enum { EXPECT_SORTED, EXPECT_UNSORTED } and put both
implementations in
LinearSearch on the descriptor that take the enum selector as another
parameter.
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/mark-compact.cc
File src/mark-compact.cc (right):
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/mark-compact.cc#newcode1867
src/mark-compact.cc:1867: // printf("marking: %p\n",
static_cast<void*>(descriptors));
On 2012/06/01 13:49:55, danno wrote:
Remove debugging code.
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects-inl.h
File src/objects-inl.h (right):
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects-inl.h#newcode1874
src/objects-inl.h:1874: // immutable again.
On 2012/06/01 13:49:55, danno wrote:
Re-enable this assert appropriately.
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects-printer.cc
File src/objects-printer.cc (right):
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects-printer.cc#newcode275
src/objects-printer.cc:275: break;
On 2012/06/01 13:49:55, danno wrote:
Please add a printer for the transition map in the descriptor array so
that it's
possible to output this information.
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.cc
File src/objects.cc (right):
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.cc#newcode1571
src/objects.cc:1571: {
On 2012/06/01 13:49:55, danno wrote:
nit: No need for this whitespace change.
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.cc#newcode2321
src/objects.cc:2321: // Only remember the map transition if the object's
map is NOT equal to
On 2012/06/01 13:49:55, danno wrote:
nit: indentation
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.cc#newcode4039
src/objects.cc:4039: { MaybeObject* maybe =
map()->CopyDropTransitions(false);
On 2012/06/01 13:49:55, danno wrote:
This should be an enum rather than a bool, it's really difficult to
see what the
parameter means at the usage site.
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.cc#newcode4945
src/objects.cc:4945: if (descriptor_array_->MayContainTransitions())
As discussed offline, won't change since it is actually only related to
transitions, not so much to descriptors.
On 2012/06/01 13:49:55, danno wrote:
This is confusing. What does descriptor iteration have to do with
transition?
Perhaps another predicate name, even if it does the same thing?
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.h
File src/objects.h (right):
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.h#newcode2405
src/objects.h:2405: DECL_ACCESSORS(elements_transition, Map)
On 2012/06/01 13:49:55, danno wrote:
probably should call this elements_transition_map
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.h#newcode2409
src/objects.h:2409: ASSERT(length() >= kFirstIndex || IsEmpty());
On 2012/06/01 13:49:55, danno wrote:
ASSERT(length() > kFirstIndex || length() == kTransitionsIndex ||
IsEmpty());
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.h#newcode2565
src/objects.h:2565: bool mutable_transitions);
On 2012/06/01 13:49:55, danno wrote:
Turn this into an enum.
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/objects.h#newcode4792
src/objects.h:4792: MUST_USE_RESULT MaybeObject*
CopyDropTransitions(bool mutable_transitions);
On 2012/06/01 13:49:55, danno wrote:
Enum instead of bool
Done.
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/property.cc
File src/property.cc (right):
https://chromiumcodereview.appspot.com/10444055/diff/4005/src/property.cc#newcode60
src/property.cc:60: PrintF(out, " -map:\n");
Since this function is only to be used directly, not transitively, it is
not necessary to move the code to a higher level.
On 2012/06/01 13:49:55, danno wrote:
Make sure this debugging information is available somewhere.
https://chromiumcodereview.appspot.com/10444055/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
