Inobject slack tracking is done on a per-closure basis instead of per-shared info basis.

src/objects.h

Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index d642e1e7da88b4f57bf49f79b485031f771a1ef3..ea9a71174c86108b4c360586d8cf835400beddac 100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -5964,6 +5964,10 @@ class Map: public HeapObject {
   class IsFrozen:                   public BitField<bool, 25,  1> {};
   class IsUnstable:                 public BitField<bool, 26,  1> {};
   class IsMigrationTarget:          public BitField<bool, 27,  1> {};
+  class DoneInobjectSlackTracking:  public BitField<bool, 28, 1> {};

[Michael Starzinger, 2014/05/21 10:42:01]

nit: Alignment looks off.

[Igor Sheludko, 2014/05/22 08:05:42]

Done.

+  // Keep this bit field at the very end for better code in
+  // Builtins::kJSConstructStubGeneric stub.
+  class ConstructionCount:          public BitField<unsigned, 29, 3> {};

[Michael Starzinger, 2014/05/21 10:42:01]

nit: With three bits it should be fine to use plain "int" here.

[Igor Sheludko, 2014/05/22 08:05:42]

Done.

 
   // Tells whether the object in the prototype property will be used
   // for instances created from this function.  If the prototype
@@ -6145,12 +6149,6 @@ class Map: public HeapObject {
   // function that was used to instantiate the object).
   String* constructor_name();
 
-  // Tells whether the map is attached to SharedFunctionInfo
-  // (for inobject slack tracking).
-  inline void set_attached_to_shared_function_info(bool value);
-
-  inline bool attached_to_shared_function_info();
-
   // Tells whether the map is shared between objects that may have different
   // behavior. If true, the map should never be modified, instead a clone
   // should be created and modified.
@@ -6285,6 +6283,10 @@ class Map: public HeapObject {
   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(unsigned value);
+  inline unsigned construction_count();
   inline void deprecate();
   inline bool is_deprecated();
   inline bool CanBeDeprecated();
@@ -6525,7 +6527,7 @@ class Map: public HeapObject {
   // Bit positions for bit field 2
   static const int kIsExtensible = 0;
   static const int kStringWrapperSafeForDefaultValueOf = 1;
-  static const int kAttachedToSharedFunctionInfo = 2;
+  // Currently bit 2 is not used.
   // No bits can be used after kElementsKindFirstBit, they are all reserved for
   // storing ElementKind.
   static const int kElementsKindShift = 3;
@@ -6918,108 +6920,16 @@ class SharedFunctionInfo: public HeapObject {
   inline int expected_nof_properties();
   inline void set_expected_nof_properties(int value);
 
-  // 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 SharedFunctionInfo.
-  //   When it happens enter the "in progress" state: remember the
-  //   constructor's initial_map and install a special construct stub that
-  //   counts constructor calls.
-  // - 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
-  //   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.
-  // - Decrease expected_nof_properties so that an allocations made from
-  //   another context will use the adjusted instance size too.
-  // - Exit "in progress" state by clearing the reference to the initial_map
-  //   and setting the regular construct stub (generic or inline).
-  //
-  //  The above is the main event sequence. Some special cases are possible
-  //  while the tracking is in progress:
-  //
-  // - GC occurs.
-  //   Check if the initial_map is referenced by any live objects (except this
-  //   SharedFunctionInfo). If it is, continue tracking as usual.
-  //   If it is not, clear the reference and reset the tracking state. The
-  //   tracking will be initiated again on the next constructor call.
-  //
-  // - The constructor is called from another context.
-  //   Immediately complete the tracking, perform all the necessary changes
-  //   to maps. This is  necessary because there is no efficient way to track
-  //   multiple initial_maps.
-  //   Proceed to create an object in the current context (with the adjusted
-  //   size).
-  //
-  // - A different constructor function sharing the same SharedFunctionInfo is
-  //   called in the same context. This could be another closure in the same
-  //   context, or the first function could have been disposed.
-  //   This is handled the same way as the previous case.
-  //
-  //  Important: inobject slack tracking is not attempted during the snapshot
-  //  creation.
-
-  static const int kGenerousAllocationCount = 8;
-
-  // [construction_count]: Counter for constructor calls made during
-  // the tracking phase.
-  inline int construction_count();
-  inline void set_construction_count(int value);
-
   // [feedback_vector] - accumulates ast node feedback from full-codegen and
   // (increasingly) from crankshafted code where sufficient feedback isn't
   // available. Currently the field is duplicated in
   // TypeFeedbackInfo::feedback_vector, but the allocation is done here.
   DECL_ACCESSORS(feedback_vector, FixedArray)
 
-  // [initial_map]: initial map of the first function called as a constructor.
-  // Saved for the duration of the tracking phase.
-  // This is a weak link (GC resets it to undefined_value if no other live
-  // object reference this map).
-  DECL_ACCESSORS(initial_map, Object)
-
-  // True if the initial_map is not undefined and the countdown stub is
-  // installed.
-  inline bool IsInobjectSlackTrackingInProgress();
-
-  // Starts the tracking.
-  // Stores the initial map and installs the countdown stub.
-  // 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(Map* map);
-
-  // Completes the tracking.
-  // IsInobjectSlackTrackingInProgress is false after this call.
-  void CompleteInobjectSlackTracking();
-
   // Invoked before pointers in SharedFunctionInfo are being marked.
   // Also clears the optimized code map.
   inline void BeforeVisitingPointers();

[Michael Starzinger, 2014/05/21 10:42:01]

The only reason we needed this callback was for detaching the initial map from
the SharedFunctionInfo. Since we ne longer do that I propose to remove this
callback, this in turn makes the BeforeVisitingSharedFunctionInfo function in
the static visitors obsolete as well.

I am totally fine to do that in a follow-up CL, because this one is already
complex enough.

[Igor Sheludko, 2014/05/22 08:05:42]

I'll do the cleanup in a next CL.

 
-  // Clears the initial_map before the GC marking phase to ensure the reference
-  // is weak. IsInobjectSlackTrackingInProgress is false after this call.
-  void DetachInitialMap();
-
-  // Restores the link to the initial map after the GC marking phase.
-  // IsInobjectSlackTrackingInProgress is true after this call.
-  void AttachInitialMap(Map* map);
-
-  // False if there are definitely no live objects created from this function.
-  // True if live objects _may_ exist (existence not guaranteed).
-  // May go back from true to false after GC.
-  DECL_BOOLEAN_ACCESSORS(live_objects_may_exist)
-
   // [instance class name]: class name for instances.
   DECL_ACCESSORS(instance_class_name, Object)
 
@@ -7265,12 +7175,10 @@ class SharedFunctionInfo: public HeapObject {
   static const int kInferredNameOffset = kDebugInfoOffset + kPointerSize;
   static const int kFeedbackVectorOffset =
       kInferredNameOffset + kPointerSize;
-  static const int kInitialMapOffset =
-      kFeedbackVectorOffset + kPointerSize;
 #if V8_HOST_ARCH_32_BIT
   // Smi fields.
   static const int kLengthOffset =
-      kInitialMapOffset + kPointerSize;
+      kFeedbackVectorOffset + kPointerSize;
   static const int kFormalParameterCountOffset = kLengthOffset + kPointerSize;
   static const int kExpectedNofPropertiesOffset =
       kFormalParameterCountOffset + kPointerSize;
@@ -7306,7 +7214,7 @@ class SharedFunctionInfo: public HeapObject {
   // word is not set and thus this word cannot be treated as pointer
   // to HeapObject during old space traversal.
   static const int kLengthOffset =
-      kInitialMapOffset + kPointerSize;
+      kFeedbackVectorOffset + kPointerSize;
   static const int kFormalParameterCountOffset =
       kLengthOffset + kIntSize;
 
@@ -7340,21 +7248,10 @@ class SharedFunctionInfo: public HeapObject {
 
 #endif
 
-  // The construction counter for inobject slack tracking is stored in the
-  // most significant byte of compiler_hints which is otherwise unused.
-  // Its offset depends on the endian-ness of the architecture.
-#if defined(V8_TARGET_LITTLE_ENDIAN)
-  static const int kConstructionCountOffset = kCompilerHintsOffset + 3;
-#elif defined(V8_TARGET_BIG_ENDIAN)
-  static const int kConstructionCountOffset = kCompilerHintsOffset + 0;
-#else
-#error Unknown byte ordering
-#endif
-
   static const int kAlignedSize = POINTER_SIZE_ALIGN(kSize);
 
   typedef FixedBodyDescriptor<kNameOffset,
-                              kInitialMapOffset + kPointerSize,
+                              kFeedbackVectorOffset + kPointerSize,
                               kSize> BodyDescriptor;
 
   // Bit positions in start_position_and_type.
@@ -7369,7 +7266,6 @@ class SharedFunctionInfo: public HeapObject {
   enum CompilerHints {
     kAllowLazyCompilation,
     kAllowLazyCompilationWithoutContext,
-    kLiveObjectsMayExist,
     kOptimizationDisabled,
     kStrictModeFunction,
     kUsesArguments,
@@ -7583,6 +7479,55 @@ class JSFunction: public JSObject {
   // Tells whether or not the function is on the concurrent recompilation queue.
   inline bool IsInOptimizationQueue();
 
+  // 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|
+  //   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
+  //   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 unsigned kGenerousAllocationCount =

[Michael Starzinger, 2014/05/21 10:42:01]

nit: Likewise, plain "int" should do it here.

[Igor Sheludko, 2014/05/22 08:05:42]

Done.

+      (1 << Map::ConstructionCount::kSize) - 1;

[Michael Starzinger, 2014/05/21 10:42:01]

nit: Just use "Map::ConstructionCount::kMax" here.

[Igor Sheludko, 2014/05/22 08:05:42]

Done.

+  static const unsigned kFinishSlackTracking     = 1;
+  static const unsigned kNoSlackTracking         = 0;
+
+  // True if the initial_map is set and the object constructions countdown
+  // counter is not zero.
+  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.
   //