Go into slow case when encountering object initialization on the top level to...

src/parser.cc

 // Copyright 2006-2008 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 51 matching lines @@
 
 
   // Returns NULL if parsing failed.
   FunctionLiteral* ParseProgram(Handle<String> source,
                                 unibrow::CharacterStream* stream,
                                 bool in_global_context);
   FunctionLiteral* ParseLazy(Handle<String> source,
                              Handle<String> name,
                              int start_position, bool is_expression);
 
+  // The minimum number of contiguous assignment that is needed to
+  // be treated as an initialization block.
+  static const int kMinInitializationBlock = 3;

[Mads Ager (chromium), 2009/02/25 13:11:07]

Is this limit based on benchmarking?  We should make sure to only perform the
two runtime calls if that overhead is less than the instance descriptor copying
overhead.

[olehougaard, 2009/02/26 07:31:44]

It's based on benchmarking. I've added a comment to that effect.

+
  protected:
 
   enum Mode {
     PARSE_LAZILY,
     PARSE_EAGERLY
   };
 
   // Report syntax error
   void ReportUnexpectedToken(Token::Value token);
 
@@ skipping 1126 matching lines @@
 }
 
 
 void PreParser::ReportMessageAt(Scanner::Location source_location,
                                 const char* type,
                                 Vector<const char*> args) {
   recorder()->LogMessage(source_location, type, args);
 }
 
 
+// An InitializationBlockFinder finds and marks sequences of statements of the
+// form x.y.z.a = ...; x.y.z.b = ...; etc.
+class InitializationBlockFinder {
+ public:
+  InitializationBlockFinder()
+    : first_in_block_(NULL), last_in_block_(NULL), block_size_(0) {}
+
+  ~InitializationBlockFinder() {
+    if (InBlock()) EndBlock();
+  }
+
+  void Update(Statement* stat) {
+    Assignment* assignment = AsAssignment(stat);
+    if (InBlock()) {
+      if (BlockContinues(assignment)) {
+        UpdateBlock(assignment);
+      } else {
+        EndBlock();
+      }
+    }
+    if (!InBlock() && (assignment != NULL)) StartBlock(assignment);

[Mads Ager (chromium), 2009/02/25 13:11:07]

In BlockContinues, you check if the operation is Token::ASSIGN.  You should do
that here (or in StartBlock) as well?

Please add a test case that hits this.

[olehougaard, 2009/02/26 07:31:44]

Fixed.

+  }
+
+ private:
+  static Assignment* AsAssignment(Statement* stat) {
+    if (stat == NULL) return NULL;
+    ExpressionStatement* exp_stat = stat->AsExpressionStatement();
+    if (exp_stat == NULL) return NULL;
+    return exp_stat->expression()->AsAssignment();
+  }
+
+  // Returns true if the expressions appear to denote the same object.
+  // In the context of initialization blocks, we only consider expressions
+  // of the form 'x.y.z'.
+  static bool SameObject(Expression* e1, Expression* e2);
+
+  // Returns true if the expressions appear to denote different properties
+  // of the same object.
+  static bool PropertyOfSameObject(Expression* e1, Expression* e2) {
+    Property* p1 = e1->AsProperty();
+    Property* p2 = e2->AsProperty();
+    if (p1 == NULL || p2 == NULL) return false;
+    return SameObject(p1->obj(), p2->obj());
+  }
+
+  bool BlockContinues(Assignment* assignment) {
+    if (assignment == NULL || first_in_block_ == NULL) return false;
+    if (assignment->op() != Token::ASSIGN) return false;

[iposva, 2009/02/25 12:54:45]

You check here that only ASSIGN statements are added when the block continues,
but you do not check for the initial node to be an ASSIGN statement anywhere I
can tell.

[olehougaard, 2009/02/26 07:31:44]

Fixed.

+    return PropertyOfSameObject(first_in_block_->target(),
+                                assignment->target());
+  }
+
+  void StartBlock(Assignment* assignment) {
+    first_in_block_ = assignment;
+    last_in_block_ = assignment;
+    block_size_ = 1;
+  }
+
+  void UpdateBlock(Assignment* assignment) {
+    last_in_block_ = assignment;
+    ++block_size_;
+  }
+
+  void EndBlock() {
+    if (block_size_ >= Parser::kMinInitializationBlock) {
+      first_in_block_->mark_block_start();
+      last_in_block_->mark_block_end();
+    }
+    last_in_block_ = first_in_block_ = NULL;
+    block_size_ = 0;
+  }
+
+  bool InBlock() { return first_in_block_ != NULL; }
+
+  Assignment* first_in_block_;
+  Assignment* last_in_block_;
+  int block_size_;
+
+  DISALLOW_COPY_AND_ASSIGN(InitializationBlockFinder);
+};
+
+
+bool InitializationBlockFinder::SameObject(Expression* e1, Expression* e2) {

[iposva, 2009/02/25 12:54:45]

What made you put this method outside the class declaration? Since this is a
private class I would just put all of the code in the declaration, or at least
pull all non-trivial (aka 1-liners) out to be consistent.

[olehougaard, 2009/02/26 07:31:44]

I've put all the code in the declaration for consistency.

+  VariableProxy* v1 = e1->AsVariableProxy();
+  VariableProxy* v2 = e2->AsVariableProxy();
+  if (v1 != NULL && v2 != NULL) {

[iposva, 2009/02/25 12:54:45]

In general I find code more readable when () are added around boolean
expressions such as

if ((v1 != NULL) && (v2 != NULL))

[olehougaard, 2009/02/26 07:31:44]

Fixed where found.

+    return v1->name()->Equals(*v2->name());
+  }
+  Property* p1 = e1->AsProperty();
+  Property* p2 = e2->AsProperty();
+  if (p1 == NULL || p2 == NULL) return false;
+  Literal* key1 = p1->key()->AsLiteral();
+  Literal* key2 = p2->key()->AsLiteral();
+  if (key1 == NULL || key2 == NULL) return false;
+  if (!key1->handle()->IsString() || !key2->handle()->IsString()) {
+    return false;
+  }
+  String* name1 = String::cast(*key1->handle());
+  String* name2 = String::cast(*key2->handle());
+  if (!name1->Equals(name2)) return false;
+  return SameObject(p1->obj(), p2->obj());
+}
+
+
 void* Parser::ParseSourceElements(ZoneListWrapper<Statement>* processor,
                                   int end_token,
                                   bool* ok) {
   // SourceElements ::
   //   (Statement)* <end_token>
 
   // Allocate a target stack to use for this set of source
   // elements. This way, all scripts and functions get their own
   // target stack thus avoiding illegal breaks and continues across
   // functions.
   TargetScope scope(this);
 
   ASSERT(processor != NULL);
+  InitializationBlockFinder block_finder;
   while (peek() != end_token) {
     Statement* stat = ParseStatement(NULL, CHECK_OK);
-    if (stat && !stat->IsEmpty()) processor->Add(stat);
+    if (stat == NULL || stat->IsEmpty()) continue;
+    // We find and mark the initialization blocks on top level code only.
+    // The optimizations of initialization blocks may result in a performance
+    // degradation in any other scopes.

[Mads Ager (chromium), 2009/02/25 13:11:07]

You should write here that the reason for only doing this in top level code is
that map transitions are not used in this case leading to different maps for all
objects going through this series of assignments.

[olehougaard, 2009/02/26 07:31:44]

Fixed.

+    if (top_scope_->is_global_scope()) block_finder.Update(stat);
+    processor->Add(stat);
   }
   return 0;
 }
 
 
 Statement* Parser::ParseStatement(ZoneStringList* labels, bool* ok) {
   // Statement ::
   //   Block
   //   VariableStatement
   //   EmptyStatement
@@ skipping 3152 matching lines @@
                        start_position,
                        is_expression);
   return result;
 }
 
 
 #undef NEW
 
 
 } }  // namespace v8::internal