Refactor the way we collect the information for associating type-related infos
authorsvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 20 Jun 2011 12:33:08 +0000 (12:33 +0000)
committersvenpanne@chromium.org <svenpanne@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Mon, 20 Jun 2011 12:33:08 +0000 (12:33 +0000)
with AST IDs. Previously 3 different places had to match in how they handle a
given case, now we are down to 2, with an even simpler logic.

The downside is that due to this simpler logic the allocated dictionary could be
larger than before, but test have shown that this happens *very* rarely, because
its capacity is rounded to the next power of 2, anyway. Furthermore, the oracle
doesn't live long enough that we should really care.

The whole oracle is probably still a bit too tricky in its details, but this is
at least a step into the right direction.
Review URL: http://codereview.chromium.org/7204003

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

src/type-info.cc
src/type-info.h

index df657cb..4c34ff8 100644 (file)
@@ -62,7 +62,7 @@ TypeInfo TypeInfo::TypeFromValue(Handle<Object> value) {
 TypeFeedbackOracle::TypeFeedbackOracle(Handle<Code> code,
                                        Handle<Context> global_context) {
   global_context_ = global_context;
-  PopulateMap(code);
+  BuildDictionary(code);
   ASSERT(reinterpret_cast<Address>(*dictionary_.location()) != kHandleZapValue);
 }
 
@@ -438,104 +438,109 @@ void TypeFeedbackOracle::CollectKeyedReceiverTypes(
 }
 
 
-void TypeFeedbackOracle::SetInfo(unsigned ast_id, Object* target) {
-  ASSERT(dictionary_->FindEntry(ast_id) == NumberDictionary::kNotFound);
-  MaybeObject* maybe_result = dictionary_->AtNumberPut(ast_id, target);
-  USE(maybe_result);
-#ifdef DEBUG
-  Object* result = NULL;
-  // Dictionary has been allocated with sufficient size for all elements.
-  ASSERT(maybe_result->ToObject(&result));
-  ASSERT(*dictionary_ == result);
-#endif
+// Things are a bit tricky here: The iterator for the RelocInfos and the infos
+// themselves are not GC-safe, so we first get all infos, then we create the
+// dictionary (possibly triggering GC), and finally we relocate the collected
+// infos before we process them.
+void TypeFeedbackOracle::BuildDictionary(Handle<Code> code) {
+  AssertNoAllocation no_allocation;
+  ZoneList<RelocInfo> infos(16);
+  HandleScope scope;
+  GetRelocInfos(code, &infos);
+  CreateDictionary(code, &infos);
+  ProcessRelocInfos(&infos);
+  // Allocate handle in the parent scope.
+  dictionary_ = scope.CloseAndEscape(dictionary_);
 }
 
 
-void TypeFeedbackOracle::PopulateMap(Handle<Code> code) {
-  Isolate* isolate = Isolate::Current();
-  HandleScope scope(isolate);
+void TypeFeedbackOracle::GetRelocInfos(Handle<Code> code,
+                                       ZoneList<RelocInfo>* infos) {
+  int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET_WITH_ID);
+  for (RelocIterator it(*code, mask); !it.done(); it.next()) {
+    infos->Add(*it.rinfo());
+  }
+}
 
-  const int kInitialCapacity = 16;
-  List<int> code_positions(kInitialCapacity);
-  List<unsigned> ast_ids(kInitialCapacity);
-  CollectIds(*code, &code_positions, &ast_ids);
 
-  ASSERT(dictionary_.is_null());  // Only initialize once.
-  dictionary_ = isolate->factory()->NewNumberDictionary(
-      code_positions.length());
+void TypeFeedbackOracle::CreateDictionary(Handle<Code> code,
+                                          ZoneList<RelocInfo>* infos) {
+  DisableAssertNoAllocation allocation_allowed;
+  byte* old_start = code->instruction_start();
+  dictionary_ = FACTORY->NewNumberDictionary(infos->length());
+  byte* new_start = code->instruction_start();
+  RelocateRelocInfos(infos, old_start, new_start);
+}
 
-  const int length = code_positions.length();
-  ASSERT(ast_ids.length() == length);
-  for (int i = 0; i < length; i++) {
-    AssertNoAllocation no_allocation;
-    RelocInfo info(code->instruction_start() + code_positions[i],
-                   RelocInfo::CODE_TARGET, 0);
-    Code* target = Code::GetCodeFromTargetAddress(info.target_address());
-    unsigned id = ast_ids[i];
-    InlineCacheState state = target->ic_state();
-    Code::Kind kind = target->kind();
-
-    if (kind == Code::BINARY_OP_IC ||
-        kind == Code::UNARY_OP_IC ||
-        kind == Code::COMPARE_IC) {
-      SetInfo(id, target);
-    } else if (state == MONOMORPHIC) {
-      if (kind == Code::KEYED_LOAD_IC ||
-          kind == Code::KEYED_STORE_IC) {
-        SetInfo(id, target);
-      } else if (kind != Code::CALL_IC ||
-                 target->check_type() == RECEIVER_MAP_CHECK) {
-        Map* map = target->FindFirstMap();
-        if (map == NULL) {
-          SetInfo(id, target);
-        } else {
-          SetInfo(id, map);
-        }
-      } else {
-        ASSERT(target->kind() == Code::CALL_IC);
-        CheckType check = target->check_type();
-        ASSERT(check != RECEIVER_MAP_CHECK);
-        SetInfo(id,  Smi::FromInt(check));
-      }
-    } else if (state == MEGAMORPHIC) {
-      SetInfo(id, target);
-    }
+
+void TypeFeedbackOracle::RelocateRelocInfos(ZoneList<RelocInfo>* infos,
+                                            byte* old_start,
+                                            byte* new_start) {
+  for (int i = 0; i < infos->length(); i++) {
+    RelocInfo* info = &(*infos)[i];
+    info->set_pc(new_start + (info->pc() - old_start));
   }
-  // Allocate handle in the parent scope.
-  dictionary_ = scope.CloseAndEscape(dictionary_);
 }
 
 
-void TypeFeedbackOracle::CollectIds(Code* code,
-                                    List<int>* code_positions,
-                                    List<unsigned>* ast_ids) {
-  AssertNoAllocation no_allocation;
-  int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET_WITH_ID);
-  for (RelocIterator it(code, mask); !it.done(); it.next()) {
-    RelocInfo* info = it.rinfo();
-    ASSERT(RelocInfo::IsCodeTarget(info->rmode()));
-    Code* target = Code::GetCodeFromTargetAddress(info->target_address());
-    if (target->is_inline_cache_stub()) {
-      InlineCacheState state = target->ic_state();
-      Code::Kind kind = target->kind();
-      if (kind == Code::BINARY_OP_IC) {
-        if (target->binary_op_type() ==
-            BinaryOpIC::GENERIC) {
-          continue;
+void TypeFeedbackOracle::ProcessRelocInfos(ZoneList<RelocInfo>* infos) {
+  for (int i = 0; i < infos->length(); i++) {
+    unsigned ast_id = static_cast<unsigned>((*infos)[i].data());
+    Code* target = Code::GetCodeFromTargetAddress((*infos)[i].target_address());
+    ProcessTarget(ast_id, target);
+  }
+}
+
+
+void TypeFeedbackOracle::ProcessTarget(unsigned ast_id, Code* target) {
+  switch (target->kind()) {
+    case Code::LOAD_IC:
+    case Code::STORE_IC:
+    case Code::CALL_IC:
+    case Code::KEYED_CALL_IC:
+      if (target->ic_state() == MONOMORPHIC) {
+        if (target->kind() == Code::CALL_IC &&
+            target->check_type() != RECEIVER_MAP_CHECK) {
+          SetInfo(ast_id,  Smi::FromInt(target->check_type()));
+        } else {
+          Object* map = target->FindFirstMap();
+          SetInfo(ast_id, map == NULL ? static_cast<Object*>(target) : map);
         }
-      } else if (kind == Code::COMPARE_IC) {
-        if (target->compare_state() == CompareIC::GENERIC) continue;
-      } else {
-        if (state != MONOMORPHIC && state != MEGAMORPHIC) continue;
+      } else if (target->ic_state() == MEGAMORPHIC) {
+        SetInfo(ast_id, target);
       }
-      code_positions->Add(
-          static_cast<int>(info->pc() - code->instruction_start()));
-      ASSERT(ast_ids->length() == 0 ||
-             (*ast_ids)[ast_ids->length()-1] !=
-             static_cast<unsigned>(info->data()));
-      ast_ids->Add(static_cast<unsigned>(info->data()));
-    }
+      break;
+
+    case Code::KEYED_LOAD_IC:
+    case Code::KEYED_STORE_IC:
+      if (target->ic_state() == MONOMORPHIC ||
+          target->ic_state() == MEGAMORPHIC) {
+        SetInfo(ast_id, target);
+      }
+      break;
+
+    case Code::UNARY_OP_IC:
+    case Code::BINARY_OP_IC:
+    case Code::COMPARE_IC:
+      SetInfo(ast_id, target);
+      break;
+
+    default:
+      break;
   }
 }
 
+
+void TypeFeedbackOracle::SetInfo(unsigned ast_id, Object* target) {
+  ASSERT(dictionary_->FindEntry(ast_id) == NumberDictionary::kNotFound);
+  MaybeObject* maybe_result = dictionary_->AtNumberPut(ast_id, target);
+  USE(maybe_result);
+#ifdef DEBUG
+  Object* result = NULL;
+  // Dictionary has been allocated with sufficient size for all elements.
+  ASSERT(maybe_result->ToObject(&result));
+  ASSERT(*dictionary_ == result);
+#endif
+}
+
 } }  // namespace v8::internal
index 44f4adc..75aabe8 100644 (file)
@@ -254,11 +254,14 @@ class TypeFeedbackOracle BASE_EMBEDDED {
 
   void SetInfo(unsigned ast_id, Object* target);
 
-  void PopulateMap(Handle<Code> code);
-
-  void CollectIds(Code* code,
-                  List<int>* code_positions,
-                  List<unsigned>* ast_ids);
+  void BuildDictionary(Handle<Code> code);
+  void GetRelocInfos(Handle<Code> code, ZoneList<RelocInfo>* infos);
+  void CreateDictionary(Handle<Code> code, ZoneList<RelocInfo>* infos);
+  void RelocateRelocInfos(ZoneList<RelocInfo>* infos,
+                          byte* old_start,
+                          byte* new_start);
+  void ProcessRelocInfos(ZoneList<RelocInfo>* infos);
+  void ProcessTarget(unsigned ast_id, Code* target);
 
   // Returns an element from the backing store. Returns undefined if
   // there is no information.