Pack GlobalHandles::Node::state_ and three flags into uint8_t

src/global-handles.cc

Index: src/global-handles.cc
diff --git a/src/global-handles.cc b/src/global-handles.cc
index 392a1810bef62c46ec5a999682d89951e362c677..09704b16ccef336cda87c25bc06de516eec902d1 100644
--- a/src/global-handles.cc
+++ b/src/global-handles.cc
@@ -41,12 +41,21 @@ ObjectGroup::~ObjectGroup() {
 }
 
 
+const uint8_t NodeIndependentFlag = 6;
+const uint8_t NodePartiallyDependentFlag = 5;
+const uint8_t NodeInNewSpaceListFlag = 4;
+const uint8_t NodeIndependentFlagMask = 1 << NodeIndependentFlag;
+const uint8_t NodePartiallyDependentFlagMask = 1 << NodePartiallyDependentFlag;
+const uint8_t NodeInNewSpaceListFlagMask = 1 << NodeInNewSpaceListFlag;
+const uint8_t NodeStateMask = 0xf;
+
+
 class GlobalHandles::Node {
  public:
   // State transition diagram:
   // FREE -> NORMAL <-> WEAK -> PENDING -> NEAR_DEATH -> { NORMAL, WEAK, FREE }
   enum State {
-    FREE,
+    FREE = 0,
     NORMAL,     // Normal global handle.
     WEAK,       // Flagged as weak but not yet finalized.
     PENDING,    // Has been recognized as only reachable by weak handles.
@@ -68,9 +77,9 @@ class GlobalHandles::Node {
     object_ = NULL;
     class_id_ = v8::HeapProfiler::kPersistentHandleNoClassId;
     index_ = 0;
-    independent_ = false;
-    partially_dependent_ = false;
-    in_new_space_list_ = false;
+    set_independent(false);
+    set_partially_dependent_(false);
+    set_in_new_space_list_(false);
     parameter_or_next_free_.next_free = NULL;
     callback_ = NULL;
   }
@@ -79,33 +88,33 @@ class GlobalHandles::Node {
   void Initialize(int index, Node** first_free) {
     index_ = static_cast<uint8_t>(index);
     ASSERT(static_cast<int>(index_) == index);
-    state_ = FREE;
-    in_new_space_list_ = false;
+    set_state(FREE);
+    set_in_new_space_list(false);
     parameter_or_next_free_.next_free = *first_free;
     *first_free = this;
   }
 
   void Acquire(Object* object, GlobalHandles* global_handles) {
-    ASSERT(state_ == FREE);
+    ASSERT(state() == FREE);
     object_ = object;
     class_id_ = v8::HeapProfiler::kPersistentHandleNoClassId;
-    independent_ = false;
-    partially_dependent_ = false;
-    state_  = NORMAL;
+    set_independent(false);
+    set_partially_dependent(false);
+    set_state(NORMAL);
     parameter_or_next_free_.parameter = NULL;
     callback_ = NULL;
     IncreaseBlockUses(global_handles);
   }
 
   void Release(GlobalHandles* global_handles) {
-    ASSERT(state_ != FREE);
+    ASSERT(state() != FREE);
     if (IsWeakRetainer()) {
       global_handles->number_of_weak_handles_--;
       if (object_->IsJSGlobalObject()) {
         global_handles->number_of_global_object_weak_handles_--;
       }
     }
-    state_ = FREE;
+    set_state(FREE);
     parameter_or_next_free_.next_free = global_handles->first_free_;
     global_handles->first_free_ = this;
     DecreaseBlockUses(global_handles);
@@ -120,115 +129,143 @@ class GlobalHandles::Node {
   bool has_wrapper_class_id() const {
     return class_id_ != v8::HeapProfiler::kPersistentHandleNoClassId;
   }
+
   uint16_t wrapper_class_id() const { return class_id_; }
   void set_wrapper_class_id(uint16_t class_id) {
-    class_id_ = class_id;
+    class_id_ = static_cast<uint8_t>(class_id);

[Michael Starzinger, 2013/01/15 09:06:41]

The class_id is a full 16 bit, we can drop this cast.

[haraken, 2013/01/15 09:40:39]

Done.

+  }
+
+  // State and flag accessors.
+
+  State state() const {
+    return static_cast<State>(flags_and_state_ & NodeStateMask);

[Michael Starzinger, 2013/01/15 09:06:41]

Instead of using all of this hand-written magic, consider using our BitField
class, see objects.h for some examples. This allows you to encode arbitrary
types (i.e. bool and State) inside a word.

[haraken, 2013/01/15 09:40:39]

Done.

+  }
+  void set_state(State state) {
+    flags_and_state_ =
+      (flags_and_state_ & ~NodeStateMask) | state;
+  }
+
+  bool is_independent() {
+    return flags_and_state_ & NodeIndependentFlagMask;
+  }
+  void set_independent(bool v) {
+    flags_and_state_ =
+      (flags_and_state_ & ~NodeIndependentFlagMask) |
+      (static_cast<uint8_t>(v) << NodeIndependentFlag);
   }
 
-  // State accessors.
+  bool is_partially_dependent() {
+    return flags_and_state_ & NodePartiallyDependentFlagMask;
+  }
+  void set_partially_dependent(bool v) {
+    flags_and_state_ =
+      (flags_and_state_ & ~NodePartiallyDependentFlagMask) |
+      (static_cast<uint8_t>(v) << NodePartiallyDependentFlag);
+  }
 
-  State state() const { return state_; }
+  bool is_in_new_space_list() {
+    return flags_and_state_ & NodeInNewSpaceListFlagMask;
+  }
+  void set_in_new_space_list(bool v) {
+    flags_and_state_ =
+      (flags_and_state_ & ~NodeInNewSpaceListFlagMask) |
+      (static_cast<uint8_t>(v) << NodeInNewSpaceListFlag);
+  }
 
   bool IsNearDeath() const {
     // Check for PENDING to ensure correct answer when processing callbacks.
-    return state_ == PENDING || state_ == NEAR_DEATH;
+    return state() == PENDING || state() == NEAR_DEATH;
   }
 
-  bool IsWeak() const { return state_ == WEAK; }
+  bool IsWeak() const { return state() == WEAK; }
 
-  bool IsRetainer() const { return state_ != FREE; }
+  bool IsRetainer() const { return state() != FREE; }
 
-  bool IsStrongRetainer() const { return state_ == NORMAL; }
+  bool IsStrongRetainer() const { return state() == NORMAL; }
 
   bool IsWeakRetainer() const {
-    return state_ == WEAK || state_ == PENDING || state_ == NEAR_DEATH;
+    return state() == WEAK || state() == PENDING || state() == NEAR_DEATH;
   }
 
   void MarkPending() {
-    ASSERT(state_ == WEAK);
-    state_ = PENDING;
+    ASSERT(state() == WEAK);
+    set_state(PENDING);
   }
 
   // Independent flag accessors.
   void MarkIndependent() {
-    ASSERT(state_ != FREE);
-    independent_ = true;
+    ASSERT(state() != FREE);
+    set_independent(true);
   }
-  bool is_independent() const { return independent_; }
 
   void MarkPartiallyDependent(GlobalHandles* global_handles) {
-    ASSERT(state_ != FREE);
+    ASSERT(state() != FREE);
     if (global_handles->isolate()->heap()->InNewSpace(object_)) {
-      partially_dependent_ = true;
+      set_partially_dependent(true);
     }
   }
-  bool is_partially_dependent() const { return partially_dependent_; }
-  void clear_partially_dependent() { partially_dependent_ = false; }
-
-  // In-new-space-list flag accessors.
-  void set_in_new_space_list(bool v) { in_new_space_list_ = v; }
-  bool is_in_new_space_list() const { return in_new_space_list_; }
+  void clear_partially_dependent() { set_partially_dependent(false); }
 
   // Callback accessor.
   WeakReferenceCallback callback() { return callback_; }
 
   // Callback parameter accessors.
   void set_parameter(void* parameter) {
-    ASSERT(state_ != FREE);
+    ASSERT(state() != FREE);
     parameter_or_next_free_.parameter = parameter;
   }
   void* parameter() const {
-    ASSERT(state_ != FREE);
+    ASSERT(state() != FREE);
     return parameter_or_next_free_.parameter;
   }
 
   // Accessors for next free node in the free list.
   Node* next_free() {
-    ASSERT(state_ == FREE);
+    ASSERT(state() == FREE);
     return parameter_or_next_free_.next_free;
   }
   void set_next_free(Node* value) {
-    ASSERT(state_ == FREE);
+    ASSERT(state() == FREE);
     parameter_or_next_free_.next_free = value;
   }
 
   void MakeWeak(GlobalHandles* global_handles,
                 void* parameter,
                 WeakReferenceCallback callback) {
-    ASSERT(state_ != FREE);
+    ASSERT(state() != FREE);
     if (!IsWeakRetainer()) {
       global_handles->number_of_weak_handles_++;
       if (object_->IsJSGlobalObject()) {
         global_handles->number_of_global_object_weak_handles_++;
       }
     }
-    state_ = WEAK;
+    set_state(WEAK);
     set_parameter(parameter);
     callback_ = callback;
   }
 
   void ClearWeakness(GlobalHandles* global_handles) {
-    ASSERT(state_ != FREE);
+    ASSERT(state() != FREE);
     if (IsWeakRetainer()) {
       global_handles->number_of_weak_handles_--;
       if (object_->IsJSGlobalObject()) {
         global_handles->number_of_global_object_weak_handles_--;
       }
     }
-    state_ = NORMAL;
+    set_state(NORMAL);
     set_parameter(NULL);
   }
 
   bool PostGarbageCollectionProcessing(Isolate* isolate,
                                        GlobalHandles* global_handles) {
-    if (state_ != Node::PENDING) return false;
+    if (state() != Node::PENDING) return false;
     WeakReferenceCallback func = callback();
     if (func == NULL) {
       Release(global_handles);
       return false;
     }
     void* par = parameter();
-    state_ = NEAR_DEATH;
+    set_state(NEAR_DEATH);
     set_parameter(NULL);
 
     v8::Persistent<v8::Object> object = ToApi<v8::Object>(handle());
@@ -245,7 +282,7 @@ class GlobalHandles::Node {
     }
     // Absence of explicit cleanup or revival of weak handle
     // in most of the cases would lead to memory leak.
-    ASSERT(state_ != NEAR_DEATH);
+    ASSERT(state() != NEAR_DEATH);
     return true;
   }
 
@@ -267,12 +304,9 @@ class GlobalHandles::Node {
   // Index in the containing handle block.
   uint8_t index_;
 
-  // Need one more bit for MSVC as it treats enums as signed.
-  State state_ : 4;
-
-  bool independent_ : 1;
-  bool partially_dependent_ : 1;
-  bool in_new_space_list_ : 1;
+  // This stores three flags (independent, partially_dependent and
+  // in_new_space_list) and a State.
+  uint8_t flags_and_state_;

[Michael Starzinger, 2013/01/15 09:06:41]

Let's just call this field "flags_".

[haraken, 2013/01/15 09:40:39]

Done.

 
   // Handle specific callback.
   WeakReferenceCallback callback_;