Refactor Map::ConstructionCount.

src/objects.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_OBJECTS_H_
 #define V8_OBJECTS_H_
 
 #include <iosfwd>
 
 #include "src/allocation.h"
@@ skipping 5643 matching lines @@
   class NumberOfOwnDescriptorsBits: public BitField<int,
       kDescriptorIndexBitCount, kDescriptorIndexBitCount> {};  // NOLINT
   STATIC_ASSERT(kDescriptorIndexBitCount + kDescriptorIndexBitCount == 20);
   class DictionaryMap : public BitField<bool, 20, 1> {};
   class OwnsDescriptors : public BitField<bool, 21, 1> {};
   class HasInstanceCallHandler : public BitField<bool, 22, 1> {};
   class Deprecated : public BitField<bool, 23, 1> {};
   class IsFrozen : public BitField<bool, 24, 1> {};
   class IsUnstable : public BitField<bool, 25, 1> {};
   class IsMigrationTarget : public BitField<bool, 26, 1> {};
-  class DoneInobjectSlackTracking : public BitField<bool, 27, 1> {};
-  // Bit 28 is free.
+  // Bit 27 is free.
 
   // Keep this bit field at the very end for better code in
   // Builtins::kJSConstructStubGeneric stub.
-  class ConstructionCount:          public BitField<int, 29, 3> {};
+  class Counter : public BitField<bool, 28, 4> {};

[Igor Sheludko, 2014/12/04 10:11:09]

Please add a comment what is this counter for and please mention that the slack
tracking is considered enabled when the counter is in the range
[kSlackTrackingCounterStart, kSlackTrackingCounterEnd].

[ulan, 2014/12/05 14:04:44]

Done.

+  static const int kSlackTrackingCounterStart = 15;

[Igor Sheludko, 2014/12/04 10:11:09]

Counter::kMax

[ulan, 2014/12/05 14:04:44]

Set it 14, so that we don't change the current number of iterations.

+  static const int kSlackTrackingCounterEnd = 1 << (Counter::kSize - 1);
+  static const int kRetainingCounterStart = kSlackTrackingCounterEnd - 1;
+  static const int kRetainingCounterEnd = 0;
+  STATIC_ASSERT((kSlackTrackingCounterStart >> (Counter::kSize - 1)) ==
+                (kSlackTrackingCounterEnd >> (Counter::kSize - 1)));
+  STATIC_ASSERT((kRetainingCounterStart >> (Counter::kSize - 1)) ==
+                (kRetainingCounterEnd >> (Counter::kSize - 1)));
+  STATIC_ASSERT((kSlackTrackingCounterEnd >> (Counter::kSize - 1)) !=
+                (kRetainingCounterEnd >> (Counter::kSize - 1)));

[Igor Sheludko, 2014/12/04 10:11:09]

I think we don't need this asserts since we don't have
|done_inobject_slack_tracking| bit anymore and we compare counter values against
ranges explicitly.

[ulan, 2014/12/05 14:04:44]

Done.

 
   // Tells whether the object in the prototype property will be used
   // for instances created from this function.  If the prototype
   // property is set to a value that is not a JSObject, the prototype
   // property will not be used to create instances of the function.
   // See ECMA-262, 13.2.2.
   inline void set_non_instance_prototype(bool value);
   inline bool has_non_instance_prototype();
 
   // Tells whether function has special prototype property. If not, prototype
@@ skipping 321 matching lines @@
   inline bool owns_descriptors();
   inline void set_owns_descriptors(bool owns_descriptors);
   inline bool has_instance_call_handler();
   inline void set_has_instance_call_handler();
   inline void freeze();
   inline bool is_frozen();
   inline void mark_unstable();
   inline bool is_stable();
   inline void set_migration_target(bool value);
   inline bool is_migration_target();
-  inline void set_done_inobject_slack_tracking(bool value);
-  inline bool done_inobject_slack_tracking();
-  inline void set_construction_count(int value);
-  inline int construction_count();
+  inline void set_counter(int value);
+  inline int counter();
   inline void deprecate();
   inline bool is_deprecated();
   inline bool CanBeDeprecated();
   // Returns a non-deprecated version of the input. If the input was not
   // deprecated, it is directly returned. Otherwise, the non-deprecated version
   // is found by re-transitioning from the root of the transition tree using the
   // descriptor array of the map. Returns NULL if no updated map is found.
   // This method also applies any pending migrations along the prototype chain.
   static MaybeHandle<Map> TryUpdate(Handle<Map> map) WARN_UNUSED_RESULT;
   // Same as above, but does not touch the prototype chain.
@@ skipping 1328 matching lines @@
   // Inobject slack tracking is the way to reclaim unused inobject space.
   //
   // The instance size is initially determined by adding some slack to
   // expected_nof_properties (to allow for a few extra properties added
   // after the constructor). There is no guarantee that the extra space
   // will not be wasted.
   //
   // Here is the algorithm to reclaim the unused inobject space:
   // - Detect the first constructor call for this JSFunction.
   //   When it happens enter the "in progress" state: initialize construction
-  //   counter in the initial_map and set the |done_inobject_slack_tracking|
+  //   counter in the initial_map and clear the |is_inobject_slack_tracking_off|

[Igor Sheludko, 2014/12/04 10:11:09]

We don't have such a flag.

[ulan, 2014/12/05 14:04:44]

Done.

   //   flag.
   // - While the tracking is in progress create objects filled with
   //   one_pointer_filler_map instead of undefined_value. This way they can be
   //   resized quickly and safely.
-  // - Once enough (kGenerousAllocationCount) objects have been created
-  //   compute the 'slack' (traverse the map transition tree starting from the
+  // - Once enough objects have been created  compute the 'slack'
+  //   (traverse the map transition tree starting from the
   //   initial_map and find the lowest value of unused_property_fields).
   // - Traverse the transition tree again and decrease the instance size
   //   of every map. Existing objects will resize automatically (they are
   //   filled with one_pointer_filler_map). All further allocations will
   //   use the adjusted instance size.
   // - SharedFunctionInfo's expected_nof_properties left unmodified since
   //   allocations made using different closures could actually create different
   //   kind of objects (see prototype inheritance pattern).
   //
   //  Important: inobject slack tracking is not attempted during the snapshot
   //  creation.
 
-  static const int kGenerousAllocationCount = Map::ConstructionCount::kMax;
-  static const int kFinishSlackTracking     = 1;
-  static const int kNoSlackTracking         = 0;
-
   // True if the initial_map is set and the object constructions countdown
   // counter is not zero.
+  static const int kGenerousAllocationCount =
+      Map::kSlackTrackingCounterStart - Map::kSlackTrackingCounterEnd;

[Igor Sheludko, 2014/12/04 10:11:09]

I think we need + 1 here, since the range in inclusive at both ends.

[ulan, 2014/12/05 14:04:44]

Done.

   inline bool IsInobjectSlackTrackingInProgress();
 
   // Starts the tracking.
   // Initializes object constructions countdown counter in the initial map.
-  // IsInobjectSlackTrackingInProgress is normally true after this call,
-  // except when tracking have not been started (e.g. the map has no unused
-  // properties or the snapshot is being built).
   void StartInobjectSlackTracking();
 
   // Completes the tracking.
-  // IsInobjectSlackTrackingInProgress is false after this call.
   void CompleteInobjectSlackTracking();
 
   // [literals_or_bindings]: Fixed array holding either
   // the materialized literals or the bindings of a bound function.
   //
   // If the function contains object, regexp or array literals, the
   // literals array prefix contains the object, regexp, and array
   // function to be used when creating these literals.  This is
   // necessary so that we do not dynamically lookup the object, regexp
   // or array functions.  Performing a dynamic lookup, we might end up
@@ skipping 3588 matching lines @@
     } else {
       value &= ~(1 << bit_position);
     }
     return value;
   }
 };
 
 } }  // namespace v8::internal
 
 #endif  // V8_OBJECTS_H_