[v8-dev] Use handle lists in Map::FindTransitionedMap. (issue 8373030)

Mon, 24 Oct 2011 03:03:09 -0700 

Reviewers: Kevin Millikin,

Message:
Please take a look.

Description:
Use handle lists in Map::FindTransitionedMap.


BUG=
TEST=


Please review this at http://codereview.chromium.org/8373030/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/hydrogen.cc
  M src/objects.cc


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 0b843bd45b32eb6d57c1efa86ee784c59290c817..d758895c37b1fef3e4afc3570f1454b72780c102 100644 
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -4095,21 +4095,21 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object, 
   }

   // Elements_kind transition support.
-  MapList transition_target(maps->length());
+  MapHandleList transition_target(maps->length());
   // Collect possible transition targets.
-  MapList possible_transitioned_maps(maps->length());
+  MapHandleList possible_transitioned_maps(maps->length());
   for (int i = 0; i < maps->length(); ++i) {
     Handle<Map> map = maps->at(i);
     ElementsKind elements_kind = map->elements_kind();
     if (elements_kind == FAST_DOUBLE_ELEMENTS ||
         elements_kind == FAST_ELEMENTS) {
-      possible_transitioned_maps.Add(*map);
+      possible_transitioned_maps.Add(map);
     }
   }
   // Get transition target for each map (NULL == no transition).
   for (int i = 0; i < maps->length(); ++i) {
     Handle<Map> map = maps->at(i);
-    Map* transitioned_map =
+    Handle<Map> transitioned_map =
         map->FindTransitionedMap(&possible_transitioned_maps);
     transition_target.Add(transitioned_map);
   }
@@ -4119,9 +4119,9 @@ HValue* HGraphBuilder::HandlePolymorphicElementAccess(HValue* object, 
   for (int i = 0; i < maps->length(); ++i) {
     Handle<Map> map = maps->at(i);
     ASSERT(map->IsMap());
-    if (transition_target.at(i) != NULL) {
+    if (!transition_target.at(i).is_null()) {
       object = AddInstruction(new(zone()) HTransitionElementsKind(
-          object, map, Handle<Map>(transition_target.at(i))));
+          object, map, transition_target.at(i)));
     } else {
       type_todo[map->elements_kind()] = true;
       if (map->elements_kind() >= FIRST_EXTERNAL_ARRAY_ELEMENTS_KIND) {
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 8db40f098b7cd1ef6e43ec2e014538fb4aa762b6..11e2d4de606825898dffeb8b556f3d78c53cb437 100644 
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -2181,47 +2181,51 @@ void Map::LookupInDescriptors(JSObject* holder,
 }
-// If |map| is contained in |maps_list|, returns |map|; otherwise returns NULL. 
-static bool ContainsMap(MapList* maps_list, Map* map) {
-  for (int i = 0; i < maps_list->length(); ++i) {
-    if (maps_list->at(i) == map) return true;
+static bool ContainsMap(MapHandleList* maps, Handle<Map> map) {
+  ASSERT(!map.is_null());
+  for (int i = 0; i < maps->length(); ++i) {
+ if (!maps->at(i).is_null() && maps->at(i).is_identical_to(map)) return true; 
   }
   return false;
 }


-Handle<Map> Map::FindTransitionedMap(MapHandleList* candidates) {
-  MapList raw_candidates(candidates->length());
-  Map* result = FindTransitionedMap(UnwrapHandleList(&raw_candidates,
-                                                     candidates));
-  return (result == NULL) ? Handle<Map>::null() : Handle<Map>(result);
+template <class T>
+static Handle<T> MaybeNull(T* p) {
+  if (p == NULL) return Handle<T>::null();
+  return Handle<T>(p);
 }


-Map* Map::FindTransitionedMap(MapList* candidates) {
+Handle<Map> Map::FindTransitionedMap(MapHandleList* candidates) {
   ElementsKind elms_kind = elements_kind();
   if (elms_kind == FAST_DOUBLE_ELEMENTS) {
     bool dummy = true;
-    Map* fast_map = LookupElementsTransitionMap(FAST_ELEMENTS, &dummy);
-    if (fast_map == NULL) return NULL;
-    if (ContainsMap(candidates, fast_map)) return fast_map;
-    return NULL;
+    Handle<Map> fast_map =
+        MaybeNull(LookupElementsTransitionMap(FAST_ELEMENTS, &dummy));
+    if (!fast_map.is_null() && ContainsMap(candidates, fast_map)) {
+      return fast_map;
+    }
+    return Handle<Map>::null();
   }
   if (elms_kind == FAST_SMI_ONLY_ELEMENTS) {
     bool dummy = true;
- Map* double_map = LookupElementsTransitionMap(FAST_DOUBLE_ELEMENTS, &dummy); 
+    Handle<Map> double_map =
+ MaybeNull(LookupElementsTransitionMap(FAST_DOUBLE_ELEMENTS, &dummy)); 
     // In the current implementation, if the DOUBLE map doesn't exist, the
     // FAST map can't exist either.
-    if (double_map == NULL) return NULL;
-    Map* fast_map = double_map->LookupElementsTransitionMap(FAST_ELEMENTS,
-                                                            &dummy);
- if (fast_map != NULL && ContainsMap(candidates, fast_map)) return fast_map; 
+    if (double_map.is_null()) return Handle<Map>::null();
+    Handle<Map> fast_map =
+        MaybeNull(double_map->LookupElementsTransitionMap(FAST_ELEMENTS,
+                                                          &dummy));
+    if (!fast_map.is_null() && ContainsMap(candidates, fast_map)) {
+      return fast_map;
+    }
     if (ContainsMap(candidates, double_map)) return double_map;
   }
-  return NULL;
+  return Handle<Map>::null();
 }

-
static Map* GetElementsTransitionMapFromDescriptor(Object* descriptor_contents, ElementsKind elements_kind) { 
   if (descriptor_contents->IsMap()) {


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to