Reviewers: Toon Verwaest,

Message:
PTAL

Description:
Get rid of the function sorting in for polymorphic calls.

The idea of this code was to sort functions according to
ticks spend executing them, but now these ticks are always
zero and therefore we fall back to sorting by AST length all
the time, which is a bad arbitrary measure.

Please review this at https://codereview.chromium.org/159653003/

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

Affected files (+9, -52 lines):
  M src/hydrogen.cc


Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index b035ed1afa316f44037a8e4c7e12bb0d4c786896..2fa1f6e1c772162278c48ceb6bb94114ea09c7e2 100644
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -6772,83 +6772,40 @@ HInstruction* HOptimizedGraphBuilder::BuildCallConstantFunction(
 }


-class FunctionSorter {
- public:
- FunctionSorter() : index_(0), ticks_(0), ast_length_(0), src_length_(0) { }
-  FunctionSorter(int index, int ticks, int ast_length, int src_length)
-      : index_(index),
-        ticks_(ticks),
-        ast_length_(ast_length),
-        src_length_(src_length) { }
-
-  int index() const { return index_; }
-  int ticks() const { return ticks_; }
-  int ast_length() const { return ast_length_; }
-  int src_length() const { return src_length_; }
-
- private:
-  int index_;
-  int ticks_;
-  int ast_length_;
-  int src_length_;
-};
-
-
-inline bool operator<(const FunctionSorter& lhs, const FunctionSorter& rhs) {
-  int diff = lhs.ticks() - rhs.ticks();
-  if (diff != 0) return diff > 0;
-  diff = lhs.ast_length() - rhs.ast_length();
-  if (diff != 0) return diff < 0;
-  return lhs.src_length() < rhs.src_length();
-}
-
-
 void HOptimizedGraphBuilder::HandlePolymorphicCallNamed(
     Call* expr,
     HValue* receiver,
     SmallMapList* types,
     Handle<String> name) {
int argument_count = expr->arguments()->length() + 1; // Includes receiver.
-  FunctionSorter order[kMaxCallPolymorphism];

   bool handle_smi = false;
   bool handled_string = false;
-  int ordered_functions = 0;

-  for (int i = 0;
-       i < types->length() && ordered_functions < kMaxCallPolymorphism;
-       ++i) {
+  for (int i = 0; i < types->length(); ++i) {
     PropertyAccessInfo info(this, LOAD, ToType(types->at(i)), name);
     if (info.CanAccessMonomorphic() &&
         info.lookup()->IsConstant() &&
         info.constant()->IsJSFunction()) {
-      if (info.type()->Is(Type::String())) {
-        if (handled_string) continue;
-        handled_string = true;
-      }
- Handle<JSFunction> target = Handle<JSFunction>::cast(info.constant());
       if (info.type()->Is(Type::Number())) {
         handle_smi = true;
+        break;
       }
-      expr->set_target(target);
-      order[ordered_functions++] =
-          FunctionSorter(i,
-                         expr->target()->shared()->profiler_ticks(),
-                         InliningAstSize(expr->target()),
-                         expr->target()->shared()->SourceSize());
     }
   }

-  std::sort(order, order + ordered_functions);
-
   HBasicBlock* number_block = NULL;
   HBasicBlock* join = NULL;
   handled_string = false;
   int count = 0;

-  for (int fn = 0; fn < ordered_functions; ++fn) {
-    int i = order[fn].index();
+  for (int i = 0; i < types->length(); ++i) {
     PropertyAccessInfo info(this, LOAD, ToType(types->at(i)), name);
+    if (!info.CanAccessMonomorphic() ||
+        !info.lookup()->IsConstant() ||
+        !info.constant()->IsJSFunction()) {
+      continue;
+    }
     if (info.type()->Is(Type::String())) {
       if (handled_string) continue;
       handled_string = true;
@@ -6939,7 +6896,7 @@ void HOptimizedGraphBuilder::HandlePolymorphicCallNamed( // Finish up. Unconditionally deoptimize if we've handled all the maps we
   // know about and do not want to handle ones we've never seen.  Otherwise
   // use a generic IC.
- if (ordered_functions == types->length() && FLAG_deoptimize_uncommon_cases) {
+  if (count == types->length() && FLAG_deoptimize_uncommon_cases) {
// Because the deopt may be the only path in the polymorphic call, make sure // that the environment stack matches the depth on deopt that it otherwise
     // would have had after a successful call.


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to