Match max property descriptor length to corresponding bit fields

BUG=v8:3010
R=verwaest@chromium.org
LOG=N

Review URL: https://codereview.chromium.org/72333004

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@17823 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc
index ad5bd76..bcc985a 100644 (file)
--- a/src/arm/macro-assembler-arm.cc
+++ b/src/arm/macro-assembler-arm.cc
@@ -3800,7 +3800,7 @@ void MacroAssembler::CheckEnumCache(Register null_value, Label* call_runtime) {
   ldr(r1, FieldMemOperand(r2, HeapObject::kMapOffset));
 
   EnumLength(r3, r1);
-  cmp(r3, Operand(Smi::FromInt(Map::kInvalidEnumCache)));
+  cmp(r3, Operand(Smi::FromInt(kInvalidEnumCacheSentinel)));
   b(eq, call_runtime);
 
   jmp(&start);

diff --git a/src/handles.cc b/src/handles.cc
index b86f19a..2d41402 100644 (file)
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -637,7 +637,7 @@ Handle<FixedArray> GetEnumPropertyKeys(Handle<JSObject> object,
       // present enum cache. The first step to using the cache is to set the
       // enum length of the map by counting the number of own descriptors that
       // are not DONT_ENUM or SYMBOLIC.
-      if (own_property_count == Map::kInvalidEnumCache) {
+      if (own_property_count == kInvalidEnumCacheSentinel) {
         own_property_count = object->map()->NumberOfDescribedProperties(
             OWN_DESCRIPTORS, DONT_SHOW);
 

diff --git a/src/heap.cc b/src/heap.cc
index dfb2478..b0ac8fe 100644 (file)
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -2482,7 +2482,7 @@ MaybeObject* Heap::AllocatePartialMap(InstanceType instance_type,
   reinterpret_cast<Map*>(result)->set_unused_property_fields(0);
   reinterpret_cast<Map*>(result)->set_bit_field(0);
   reinterpret_cast<Map*>(result)->set_bit_field2(0);
-  int bit_field3 = Map::EnumLengthBits::encode(Map::kInvalidEnumCache) |
+  int bit_field3 = Map::EnumLengthBits::encode(kInvalidEnumCacheSentinel) |
                    Map::OwnsDescriptors::encode(true);
   reinterpret_cast<Map*>(result)->set_bit_field3(bit_field3);
   return result;
@@ -2514,7 +2514,7 @@ MaybeObject* Heap::AllocateMap(InstanceType instance_type,
   map->set_instance_descriptors(empty_descriptor_array());
   map->set_bit_field(0);
   map->set_bit_field2(1 << Map::kIsExtensible);
-  int bit_field3 = Map::EnumLengthBits::encode(Map::kInvalidEnumCache) |
+  int bit_field3 = Map::EnumLengthBits::encode(kInvalidEnumCacheSentinel) |
                    Map::OwnsDescriptors::encode(true);
   map->set_bit_field3(bit_field3);
   map->set_elements_kind(elements_kind);

diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc
index a46f8d9..914a4c2 100644 (file)
--- a/src/ia32/macro-assembler-ia32.cc
+++ b/src/ia32/macro-assembler-ia32.cc
@@ -3546,7 +3546,7 @@ void MacroAssembler::CheckEnumCache(Label* call_runtime) {
   mov(ebx, FieldOperand(ecx, HeapObject::kMapOffset));
 
   EnumLength(edx, ebx);
-  cmp(edx, Immediate(Smi::FromInt(Map::kInvalidEnumCache)));
+  cmp(edx, Immediate(Smi::FromInt(kInvalidEnumCacheSentinel)));
   j(equal, call_runtime);
 
   jmp(&start);

diff --git a/src/objects.cc b/src/objects.cc
index 671d06f..2212e57 100644 (file)
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2137,8 +2137,7 @@ Handle<Object> JSObject::AddProperty(Handle<JSObject> object,
 
   if (object->HasFastProperties()) {
     // Ensure the descriptor array does not get too big.
-    if (object->map()->NumberOfOwnDescriptors() <
-        DescriptorArray::kMaxNumberOfDescriptors) {
+    if (object->map()->NumberOfOwnDescriptors() <= kMaxNumberOfDescriptors) {
       // TODO(verwaest): Support other constants.
       // if (mode == ALLOW_AS_CONSTANT &&
       //     !value->IsTheHole() &&
@@ -2560,7 +2559,7 @@ void Map::DeprecateTarget(Name* key, DescriptorArray* new_descriptors) {
   DescriptorArray* to_replace = instance_descriptors();
   Map* current = this;
   while (current->instance_descriptors() == to_replace) {
-    current->SetEnumLength(Map::kInvalidEnumCache);
+    current->SetEnumLength(kInvalidEnumCacheSentinel);
     current->set_instance_descriptors(new_descriptors);
     Object* next = current->GetBackPointer();
     if (next->IsUndefined()) break;
@@ -5915,7 +5914,7 @@ bool JSReceiver::IsSimpleEnum() {
     if (!o->IsJSObject()) return false;
     JSObject* curr = JSObject::cast(o);
     int enum_length = curr->map()->EnumLength();
-    if (enum_length == Map::kInvalidEnumCache) return false;
+    if (enum_length == kInvalidEnumCacheSentinel) return false;
     ASSERT(!curr->HasNamedInterceptor());
     ASSERT(!curr->HasIndexedInterceptor());
     ASSERT(!curr->IsAccessCheckNeeded());
@@ -6169,8 +6168,7 @@ void JSObject::DefinePropertyAccessor(Handle<JSObject> object,
   bool only_attribute_changes = getter->IsNull() && setter->IsNull();
   if (object->HasFastProperties() && !only_attribute_changes &&
       access_control == v8::DEFAULT &&
-      (object->map()->NumberOfOwnDescriptors() <
-       DescriptorArray::kMaxNumberOfDescriptors)) {
+      (object->map()->NumberOfOwnDescriptors() <= kMaxNumberOfDescriptors)) {
     bool getterOk = getter->IsNull() ||
         DefineFastAccessor(object, name, ACCESSOR_GETTER, getter, attributes);
     bool setterOk = !getterOk || setter->IsNull() ||
@@ -6687,7 +6685,8 @@ MaybeObject* Map::RawCopy(int instance_size) {
   int new_bit_field3 = bit_field3();
   new_bit_field3 = OwnsDescriptors::update(new_bit_field3, true);
   new_bit_field3 = NumberOfOwnDescriptorsBits::update(new_bit_field3, 0);
-  new_bit_field3 = EnumLengthBits::update(new_bit_field3, kInvalidEnumCache);
+  new_bit_field3 = EnumLengthBits::update(new_bit_field3,
+                                          kInvalidEnumCacheSentinel);
   new_bit_field3 = Deprecated::update(new_bit_field3, false);
   new_bit_field3 = IsUnstable::update(new_bit_field3, false);
   result->set_bit_field3(new_bit_field3);
@@ -9315,7 +9314,7 @@ void String::PrintOn(FILE* file) {
 
 static void TrimEnumCache(Heap* heap, Map* map, DescriptorArray* descriptors) {
   int live_enum = map->EnumLength();
-  if (live_enum == Map::kInvalidEnumCache) {
+  if (live_enum == kInvalidEnumCacheSentinel) {
     live_enum = map->NumberOfDescribedProperties(OWN_DESCRIPTORS, DONT_ENUM);
   }
   if (live_enum == 0) return descriptors->ClearEnumCache();
@@ -13401,7 +13400,7 @@ int JSObject::NumberOfLocalProperties(PropertyAttributes filter) {
     if (filter == NONE) return map->NumberOfOwnDescriptors();
     if (filter & DONT_ENUM) {
       int result = map->EnumLength();
-      if (result != Map::kInvalidEnumCache) return result;
+      if (result != kInvalidEnumCacheSentinel) return result;
     }
     return map->NumberOfDescribedProperties(OWN_DESCRIPTORS, filter);
   }
@@ -15768,7 +15767,7 @@ MaybeObject* NameDictionary::TransformPropertiesToFastFor(
   // Make sure we preserve dictionary representation if there are too many
   // descriptors.
   int number_of_elements = NumberOfElements();
-  if (number_of_elements > DescriptorArray::kMaxNumberOfDescriptors) return obj;
+  if (number_of_elements > kMaxNumberOfDescriptors) return obj;
 
   if (number_of_elements != NextEnumerationIndex()) {
     MaybeObject* maybe_result = GenerateNewEnumerationIndices();

diff --git a/src/objects.h b/src/objects.h
index 25c2210..1a2187f 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -3370,10 +3370,6 @@ class DescriptorArray: public FixedArray {
   bool IsEqualTo(DescriptorArray* other);
 #endif
 
-  // The maximum number of descriptors we want in a descriptor array (should
-  // fit in a page).
-  static const int kMaxNumberOfDescriptors = 1024 + 512;
-
   // Returns the fixed array length required to hold number_of_descriptors
   // descriptors.
   static int LengthFor(int number_of_descriptors) {
@@ -5709,17 +5705,20 @@ class Map: public HeapObject {
   inline uint32_t bit_field3();
   inline void set_bit_field3(uint32_t bits);
 
-  class EnumLengthBits:             public BitField<int,   0, 11> {};
-  class NumberOfOwnDescriptorsBits: public BitField<int,  11, 11> {};
-  class IsShared:                   public BitField<bool, 22,  1> {};
-  class FunctionWithPrototype:      public BitField<bool, 23,  1> {};
-  class DictionaryMap:              public BitField<bool, 24,  1> {};
-  class OwnsDescriptors:            public BitField<bool, 25,  1> {};
-  class HasInstanceCallHandler:     public BitField<bool, 26,  1> {};
-  class Deprecated:                 public BitField<bool, 27,  1> {};
-  class IsFrozen:                   public BitField<bool, 28,  1> {};
-  class IsUnstable:                 public BitField<bool, 29,  1> {};
-  class IsMigrationTarget:          public BitField<bool, 30,  1> {};
+  class EnumLengthBits:             public BitField<int,
+      0, kDescriptorIndexBitCount> {};  // NOLINT
+  class NumberOfOwnDescriptorsBits: public BitField<int,
+      kDescriptorIndexBitCount, kDescriptorIndexBitCount> {};  // NOLINT
+  STATIC_ASSERT(kDescriptorIndexBitCount + kDescriptorIndexBitCount == 20);
+  class IsShared:                   public BitField<bool, 20,  1> {};
+  class FunctionWithPrototype:      public BitField<bool, 21,  1> {};
+  class DictionaryMap:              public BitField<bool, 22,  1> {};
+  class OwnsDescriptors:            public BitField<bool, 23,  1> {};
+  class HasInstanceCallHandler:     public BitField<bool, 24,  1> {};
+  class Deprecated:                 public BitField<bool, 25,  1> {};
+  class IsFrozen:                   public BitField<bool, 26,  1> {};
+  class IsUnstable:                 public BitField<bool, 27,  1> {};
+  class IsMigrationTarget:          public BitField<bool, 28,  1> {};
 
   // Tells whether the object in the prototype property will be used
   // for instances created from this function.  If the prototype
@@ -6034,7 +6033,7 @@ class Map: public HeapObject {
   }
 
   void SetEnumLength(int length) {
-    if (length != kInvalidEnumCache) {
+    if (length != kInvalidEnumCacheSentinel) {
       ASSERT(length >= 0);
       ASSERT(length == 0 || instance_descriptors()->HasEnumCache());
       ASSERT(length <= NumberOfOwnDescriptors());
@@ -6252,9 +6251,6 @@ class Map: public HeapObject {
 
   static const int kMaxPreAllocatedPropertyFields = 255;
 
-  // Constant for denoting that the enum cache is not yet initialized.
-  static const int kInvalidEnumCache = EnumLengthBits::kMax;
-
   // Layout description.
   static const int kInstanceSizesOffset = HeapObject::kHeaderSize;
   static const int kInstanceAttributesOffset = kInstanceSizesOffset + kIntSize;

diff --git a/src/property-details.h b/src/property-details.h
index 92e4f81..36f1406 100644 (file)
--- a/src/property-details.h
+++ b/src/property-details.h
@@ -194,6 +194,15 @@ class Representation {
 };
 
 
+static const int kDescriptorIndexBitCount = 10;
+// The maximum number of descriptors we want in a descriptor array (should
+// fit in a page).
+static const int kMaxNumberOfDescriptors =
+    (1 << kDescriptorIndexBitCount) - 2;
+static const int kInvalidEnumCacheSentinel =
+    (1 << kDescriptorIndexBitCount) - 1;
+
+
 // PropertyDetails captures type and attributes for a property.
 // They are used both in property dictionaries and instance descriptors.
 class PropertyDetails BASE_EMBEDDED {
@@ -284,9 +293,14 @@ class PropertyDetails BASE_EMBEDDED {
   class DictionaryStorageField:   public BitField<uint32_t,           7, 24> {};
 
   // Bit fields for fast objects.
-  class DescriptorPointer:        public BitField<uint32_t,           6, 11> {};
-  class RepresentationField:      public BitField<uint32_t,          17,  4> {};
-  class FieldIndexField:          public BitField<uint32_t,          21, 10> {};
+  class RepresentationField:      public BitField<uint32_t,           6,  4> {};
+  class DescriptorPointer:        public BitField<uint32_t, 10,
+      kDescriptorIndexBitCount> {};  // NOLINT
+  class FieldIndexField:          public BitField<uint32_t,
+      10 + kDescriptorIndexBitCount,
+      kDescriptorIndexBitCount> {};  // NOLINT
+  // All bits for fast objects must fix in a smi.
+  STATIC_ASSERT(10 + kDescriptorIndexBitCount + kDescriptorIndexBitCount <= 31);
 
   static const int kInitialIndex = 1;
 

diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc
index fd857d3..5127ddf 100644 (file)
--- a/src/x64/macro-assembler-x64.cc
+++ b/src/x64/macro-assembler-x64.cc
@@ -4915,7 +4915,7 @@ void MacroAssembler::CheckEnumCache(Register null_value, Label* call_runtime) {
   movq(rbx, FieldOperand(rcx, HeapObject::kMapOffset));
 
   EnumLength(rdx, rbx);
-  Cmp(rdx, Smi::FromInt(Map::kInvalidEnumCache));
+  Cmp(rdx, Smi::FromInt(kInvalidEnumCacheSentinel));
   j(equal, call_runtime);
 
   jmp(&start);

diff --git a/test/mjsunit/regress/regress-3010.js b/test/mjsunit/regress/regress-3010.js
new file mode 100644 (file)
index 0000000..7aeec64
--- /dev/null
+++ b/test/mjsunit/regress/regress-3010.js
@@ -0,0 +1,65 @@
+// Copyright 2013 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+(function() {
+  function testOneSize(current_size) {
+    var eval_string = 'obj = {';
+    for (var current = 0; current <= current_size; ++current) {
+      eval_string += 'k' + current + ':' + current + ','
+    }
+    eval_string += '};';
+    eval(eval_string);
+    for (var i = 0; i <= current_size; i++) {
+      assertEquals(i, obj['k'+i]);
+    }
+    var current_number = 0;
+    for (var x in obj) {
+      assertEquals(current_number, obj[x]);
+      current_number++;
+    }
+  }
+
+  testOneSize(127);
+  testOneSize(128);
+  testOneSize(129);
+
+  testOneSize(255);
+  testOneSize(256);
+  testOneSize(257);
+
+  testOneSize(511);
+  testOneSize(512);
+  testOneSize(513);
+
+  testOneSize(1023);
+  testOneSize(1024);
+  testOneSize(1025);
+
+  testOneSize(2047);
+  testOneSize(2048);
+  testOneSize(2049);
+}())