Only call to LogSymbol when needed.

R=marja@chromium.org

BUG=

Review URL: https://codereview.chromium.org/198903002

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

src/parser.cc

@@ -3491,99 +3491,6 @@ Handle<FixedArray> CompileTimeValue::GetElements(Handle<FixedArray> value) {
 }
 
 
-class SingletonLogger : public ParserRecorder {
- public:
-  SingletonLogger() : has_error_(false), start_(-1), end_(-1) { }
-  virtual ~SingletonLogger() { }
-
-  void Reset() { has_error_ = false; }
-
-  virtual void LogFunction(int start,
-                           int end,
-                           int literals,
-                           int properties,
-                           StrictMode strict_mode) {
-    ASSERT(!has_error_);
-    start_ = start;
-    end_ = end;
-    literals_ = literals;
-    properties_ = properties;
-    strict_mode_ = strict_mode;
-  };
-
-  // Logs a symbol creation of a literal or identifier.
-  virtual void LogOneByteSymbol(int start, Vector<const uint8_t> literal) { }
-  virtual void LogTwoByteSymbol(int start, Vector<const uint16_t> literal) { }
-
-  // Logs an error message and marks the log as containing an error.
-  // Further logging will be ignored, and ExtractData will return a vector
-  // representing the error only.
-  virtual void LogMessage(int start,
-                          int end,
-                          const char* message,
-                          const char* argument_opt) {
-    if (has_error_) return;
-    has_error_ = true;
-    start_ = start;
-    end_ = end;
-    message_ = message;
-    argument_opt_ = argument_opt;
-  }
-
-  virtual int function_position() { return 0; }
-
-  virtual int symbol_position() { return 0; }
-
-  virtual int symbol_ids() { return -1; }
-
-  virtual Vector<unsigned> ExtractData() {
-    UNREACHABLE();
-    return Vector<unsigned>();
-  }
-
-  virtual void PauseRecording() { }
-
-  virtual void ResumeRecording() { }
-
-  bool has_error() { return has_error_; }
-
-  int start() { return start_; }
-  int end() { return end_; }
-  int literals() {
-    ASSERT(!has_error_);
-    return literals_;
-  }
-  int properties() {
-    ASSERT(!has_error_);
-    return properties_;
-  }
-  StrictMode strict_mode() {
-    ASSERT(!has_error_);
-    return strict_mode_;
-  }
-  const char* message() {
-    ASSERT(has_error_);
-    return message_;
-  }
-  const char* argument_opt() {
-    ASSERT(has_error_);
-    return argument_opt_;
-  }
-
- private:
-  bool has_error_;
-  int start_;
-  int end_;
-  // For function entries.
-  int literals_;
-  int properties_;
-  StrictMode strict_mode_;
-  // For error messages.
-  const char* message_;
-  const char* argument_opt_;
-};
-
-
 FunctionLiteral* Parser::ParseFunctionLiteral(
     Handle<String> function_name,
     Scanner::Location function_name_location,

src/preparse-data.cc

@@ -37,13 +37,40 @@
 namespace v8 {
 namespace internal {
 
-// ----------------------------------------------------------------------------
-// FunctionLoggingParserRecorder
 
-FunctionLoggingParserRecorder::FunctionLoggingParserRecorder()
+template <typename Char>
+static int vector_hash(Vector<const Char> string) {
+  int hash = 0;
+  for (int i = 0; i < string.length(); i++) {
+    int c = static_cast<int>(string[i]);
+    hash += c;
+    hash += (hash << 10);
+    hash ^= (hash >> 6);
+  }
+  return hash;
+}
+
+
+static bool vector_compare(void* a, void* b) {
+  CompleteParserRecorder::Key* string1 =
+      reinterpret_cast<CompleteParserRecorder::Key*>(a);
+  CompleteParserRecorder::Key* string2 =
+      reinterpret_cast<CompleteParserRecorder::Key*>(b);
+  if (string1->is_one_byte != string2->is_one_byte) return false;
+  int length = string1->literal_bytes.length();
+  if (string2->literal_bytes.length() != length) return false;
+  return memcmp(string1->literal_bytes.start(),
+                string2->literal_bytes.start(), length) == 0;
+}
+
+
+CompleteParserRecorder::CompleteParserRecorder()
     : function_store_(0),
-      is_recording_(true),
-      pause_count_(0) {
+      literal_chars_(0),
+      symbol_store_(0),
+      symbol_keys_(0),
+      string_table_(vector_compare),
+      symbol_id_(0) {
   preamble_[PreparseDataConstants::kMagicOffset] =
       PreparseDataConstants::kMagicNumber;
   preamble_[PreparseDataConstants::kVersionOffset] =
@@ -56,10 +83,11 @@ FunctionLoggingParserRecorder::FunctionLoggingParserRecorder()
 #ifdef DEBUG
   prev_start_ = -1;
 #endif
+  should_log_symbols_ = true;
 }
 
 
-void FunctionLoggingParserRecorder::LogMessage(int start_pos,
+void CompleteParserRecorder::LogMessage(int start_pos,
                                                int end_pos,
                                                const char* message,
                                                const char* arg_opt) {
@@ -75,11 +103,11 @@ void FunctionLoggingParserRecorder::LogMessage(int start_pos,
   STATIC_ASSERT(PreparseDataConstants::kMessageTextPos == 3);
   WriteString(CStrVector(message));
   if (arg_opt != NULL) WriteString(CStrVector(arg_opt));
-  is_recording_ = false;
+  should_log_symbols_ = false;
 }
 
 
-void FunctionLoggingParserRecorder::WriteString(Vector<const char> str) {
+void CompleteParserRecorder::WriteString(Vector<const char> str) {
   function_store_.Add(str.length());
   for (int i = 0; i < str.length(); i++) {
     function_store_.Add(str[i]);
@@ -87,43 +115,27 @@ void FunctionLoggingParserRecorder::WriteString(Vector<const char> str) {
 }
 
 
-// ----------------------------------------------------------------------------
-// PartialParserRecorder -  Record both function entries and symbols.
-
-Vector<unsigned> PartialParserRecorder::ExtractData() {
-  int function_size = function_store_.size();
-  int total_size = PreparseDataConstants::kHeaderSize + function_size;
-  Vector<unsigned> data = Vector<unsigned>::New(total_size);
-  preamble_[PreparseDataConstants::kFunctionsSizeOffset] = function_size;
-  preamble_[PreparseDataConstants::kSymbolCountOffset] = 0;
-  OS::MemCopy(data.start(), preamble_, sizeof(preamble_));
-  int symbol_start = PreparseDataConstants::kHeaderSize + function_size;
-  if (function_size > 0) {
-    function_store_.WriteTo(data.SubVector(PreparseDataConstants::kHeaderSize,
-                                           symbol_start));
-  }
-  return data;
+void CompleteParserRecorder::LogOneByteSymbol(int start,
+                                              Vector<const uint8_t> literal) {
+  ASSERT(should_log_symbols_);
+  int hash = vector_hash(literal);
+  LogSymbol(start, hash, true, literal);
 }
 
 
-// ----------------------------------------------------------------------------
-// CompleteParserRecorder -  Record both function entries and symbols.
-
-CompleteParserRecorder::CompleteParserRecorder()
-    : FunctionLoggingParserRecorder(),
-      literal_chars_(0),
-      symbol_store_(0),
-      symbol_keys_(0),
-      string_table_(vector_compare),
-      symbol_id_(0) {
+void CompleteParserRecorder::LogTwoByteSymbol(int start,
+                                              Vector<const uint16_t> literal) {
+  ASSERT(should_log_symbols_);
+  int hash = vector_hash(literal);
+  LogSymbol(start, hash, false, Vector<const byte>::cast(literal));
 }
 
 
 void CompleteParserRecorder::LogSymbol(int start,
                                        int hash,
-                                       bool is_ascii,
+                                       bool is_one_byte,
                                        Vector<const byte> literal_bytes) {
-  Key key = { is_ascii, literal_bytes };
+  Key key = { is_one_byte, literal_bytes };
   HashMap::Entry* entry = string_table_.Lookup(&key, hash, true);
   int id = static_cast<int>(reinterpret_cast<intptr_t>(entry->value));
   if (id == 0) {

src/preparse-data.h

@@ -35,13 +35,11 @@
 namespace v8 {
 namespace internal {
 
-// ----------------------------------------------------------------------------
-// ParserRecorder - Logging of preparser data.
 
 // Abstract interface for preparse data recorder.
 class ParserRecorder {
  public:
-  ParserRecorder() { }
+  ParserRecorder() : should_log_symbols_(false) { }
   virtual ~ParserRecorder() { }
 
   // Logs the scope and some details of a function literal in the source.
@@ -51,10 +49,6 @@ class ParserRecorder {
                            int properties,
                            StrictMode strict_mode) = 0;
 
-  // Logs a symbol creation of a literal or identifier.
-  virtual void LogOneByteSymbol(int start, Vector<const uint8_t> literal) = 0;
-  virtual void LogTwoByteSymbol(int start, Vector<const uint16_t> literal) = 0;
-
   // Logs an error message and marks the log as containing an error.
   // Further logging will be ignored, and ExtractData will return a vector
   // representing the error only.
@@ -63,27 +57,110 @@ class ParserRecorder {
                           const char* message,
                           const char* argument_opt) = 0;
 
-  virtual int function_position() = 0;
+  // Logs a symbol creation of a literal or identifier.
+  bool ShouldLogSymbols() { return should_log_symbols_; }
+  // The following functions are only callable on CompleteParserRecorder
+  // and are guarded by calls to ShouldLogSymbols.
+  virtual void LogOneByteSymbol(int start, Vector<const uint8_t> literal) {
+    UNREACHABLE();
+  }
+  virtual void LogTwoByteSymbol(int start, Vector<const uint16_t> literal) {
+    UNREACHABLE();
+  }
+  virtual void PauseRecording() { UNREACHABLE(); }
+  virtual void ResumeRecording() { UNREACHABLE(); }
 
-  virtual int symbol_position() = 0;
+ protected:
+  bool should_log_symbols_;
 
-  virtual int symbol_ids() = 0;
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ParserRecorder);
+};
 
-  virtual Vector<unsigned> ExtractData() = 0;
 
-  virtual void PauseRecording() = 0;
+class SingletonLogger : public ParserRecorder {
+ public:
+  SingletonLogger() : has_error_(false), start_(-1), end_(-1) { }
+  virtual ~SingletonLogger() { }
 
-  virtual void ResumeRecording() = 0;
-};
+  void Reset() { has_error_ = false; }
+
+  virtual void LogFunction(int start,
+                           int end,
+                           int literals,
+                           int properties,
+                           StrictMode strict_mode) {
+    ASSERT(!has_error_);
+    start_ = start;
+    end_ = end;
+    literals_ = literals;
+    properties_ = properties;
+    strict_mode_ = strict_mode;
+  };
+
+  // Logs an error message and marks the log as containing an error.
+  // Further logging will be ignored, and ExtractData will return a vector
+  // representing the error only.
+  virtual void LogMessage(int start,
+                          int end,
+                          const char* message,
+                          const char* argument_opt) {
+    if (has_error_) return;
+    has_error_ = true;
+    start_ = start;
+    end_ = end;
+    message_ = message;
+    argument_opt_ = argument_opt;
+  }
+
+  bool has_error() { return has_error_; }
 
+  int start() { return start_; }
+  int end() { return end_; }
+  int literals() {
+    ASSERT(!has_error_);
+    return literals_;
+  }
+  int properties() {
+    ASSERT(!has_error_);
+    return properties_;
+  }
+  StrictMode strict_mode() {
+    ASSERT(!has_error_);
+    return strict_mode_;
+  }
+  const char* message() {
+    ASSERT(has_error_);
+    return message_;
+  }
+  const char* argument_opt() {
+    ASSERT(has_error_);
+    return argument_opt_;
+  }
 
-// ----------------------------------------------------------------------------
-// FunctionLoggingParserRecorder - Record only function entries
+ private:
+  bool has_error_;
+  int start_;
+  int end_;
+  // For function entries.
+  int literals_;
+  int properties_;
+  StrictMode strict_mode_;
+  // For error messages.
+  const char* message_;
+  const char* argument_opt_;
+};
 
-class FunctionLoggingParserRecorder : public ParserRecorder {
+
+class CompleteParserRecorder : public ParserRecorder {
  public:
-  FunctionLoggingParserRecorder();
-  virtual ~FunctionLoggingParserRecorder() {}
+  struct Key {
+    bool is_one_byte;
+    Vector<const byte> literal_bytes;
+  };
+
+  CompleteParserRecorder();
+  virtual ~CompleteParserRecorder() {}
 
   virtual void LogFunction(int start,
                            int end,
@@ -105,123 +182,45 @@ class FunctionLoggingParserRecorder : public ParserRecorder {
                           const char* message,
                           const char* argument_opt);
 
-  virtual int function_position() { return function_store_.size(); }
-
-
-  virtual Vector<unsigned> ExtractData() = 0;
-
   virtual void PauseRecording() {
-    pause_count_++;
-    is_recording_ = false;
+    ASSERT(should_log_symbols_);
+    should_log_symbols_ = false;
   }
 
   virtual void ResumeRecording() {
-    ASSERT(pause_count_ > 0);
-    if (--pause_count_ == 0) is_recording_ = !has_error();
+    ASSERT(!should_log_symbols_);
+    should_log_symbols_ = !has_error();
   }
 
- protected:
+  virtual void LogOneByteSymbol(int start, Vector<const uint8_t> literal);
+  virtual void LogTwoByteSymbol(int start, Vector<const uint16_t> literal);
+  Vector<unsigned> ExtractData();
+
+ private:
   bool has_error() {
     return static_cast<bool>(preamble_[PreparseDataConstants::kHasErrorOffset]);
   }
 
-  bool is_recording() {
-    return is_recording_;
-  }
-
   void WriteString(Vector<const char> str);
 
-  Collector<unsigned> function_store_;
-  unsigned preamble_[PreparseDataConstants::kHeaderSize];
-  bool is_recording_;
-  int pause_count_;
-
-#ifdef DEBUG
-  int prev_start_;
-#endif
-};
-
-
-// ----------------------------------------------------------------------------
-// PartialParserRecorder - Record only function entries
-
-class PartialParserRecorder : public FunctionLoggingParserRecorder {
- public:
-  PartialParserRecorder() : FunctionLoggingParserRecorder() { }
-  virtual void LogOneByteSymbol(int start, Vector<const uint8_t> literal) { }
-  virtual void LogTwoByteSymbol(int start, Vector<const uint16_t> literal) { }
-  virtual ~PartialParserRecorder() { }
-  virtual Vector<unsigned> ExtractData();
-  virtual int symbol_position() { return 0; }
-  virtual int symbol_ids() { return 0; }
-};
-
-
-// ----------------------------------------------------------------------------
-// CompleteParserRecorder -  Record both function entries and symbols.
-
-class CompleteParserRecorder: public FunctionLoggingParserRecorder {
- public:
-  CompleteParserRecorder();
-  virtual ~CompleteParserRecorder() { }
-
-  virtual void LogOneByteSymbol(int start, Vector<const uint8_t> literal) {
-    if (!is_recording_) return;
-    int hash = vector_hash(literal);
-    LogSymbol(start, hash, true, literal);
-  }
-
-  virtual void LogTwoByteSymbol(int start, Vector<const uint16_t> literal) {
-    if (!is_recording_) return;
-    int hash = vector_hash(literal);
-    LogSymbol(start, hash, false, Vector<const byte>::cast(literal));
-  }
-
-  virtual Vector<unsigned> ExtractData();
-
-  virtual int symbol_position() { return symbol_store_.size(); }
-  virtual int symbol_ids() { return symbol_id_; }
-
- private:
   // For testing. Defined in test-parsing.cc.
-  friend void FakeWritingSymbolIdInPreParseData(CompleteParserRecorder* log,
-                                                int number);
-
-  struct Key {
-    bool is_ascii;
-    Vector<const byte> literal_bytes;
-  };
+  friend struct CompleteParserRecorderFriend;
 
-  virtual void LogSymbol(int start,
+  void LogSymbol(int start,
                  int hash,
-                         bool is_ascii,
+                 bool is_one_byte,
                  Vector<const byte> literal);
 
-  template <typename Char>
-  static int vector_hash(Vector<const Char> string) {
-    int hash = 0;
-    for (int i = 0; i < string.length(); i++) {
-      int c = static_cast<int>(string[i]);
-      hash += c;
-      hash += (hash << 10);
-      hash ^= (hash >> 6);
-    }
-    return hash;
-  }
-
-  static bool vector_compare(void* a, void* b) {
-    Key* string1 = reinterpret_cast<Key*>(a);
-    Key* string2 = reinterpret_cast<Key*>(b);
-    if (string1->is_ascii != string2->is_ascii) return false;
-    int length = string1->literal_bytes.length();
-    if (string2->literal_bytes.length() != length) return false;
-    return memcmp(string1->literal_bytes.start(),
-                  string2->literal_bytes.start(), length) == 0;
-  }
-
   // Write a non-negative number to the symbol store.
   void WriteNumber(int number);
 
+  Collector<unsigned> function_store_;
+  unsigned preamble_[PreparseDataConstants::kHeaderSize];
+
+#ifdef DEBUG
+  int prev_start_;
+#endif
+
   Collector<byte> literal_chars_;
   Collector<byte> symbol_store_;
   Collector<Key> symbol_keys_;

src/preparser.cc

@@ -1232,9 +1232,10 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
 
 void PreParser::ParseLazyFunctionLiteralBody(bool* ok) {
   int body_start = position();
-  log_->PauseRecording();
+  bool is_logging = log_->ShouldLogSymbols();
+  if (is_logging) log_->PauseRecording();
   ParseSourceElements(Token::RBRACE, ok);
-  log_->ResumeRecording();
+  if (is_logging) log_->ResumeRecording();
   if (!*ok) return;
 
   // Position right after terminal '}'.
@@ -1266,7 +1267,9 @@ PreParser::Expression PreParser::ParseV8Intrinsic(bool* ok) {
 
 
 void PreParser::LogSymbol() {
+  if (log_->ShouldLogSymbols()) {
     scanner()->LogSymbol(log_, position());
+  }
 }
 
 

test/cctest/test-parsing.cc

@@ -364,13 +364,24 @@ TEST(PreparsingObjectLiterals) {
 namespace v8 {
 namespace internal {
 
-void FakeWritingSymbolIdInPreParseData(i::CompleteParserRecorder* log,
+struct CompleteParserRecorderFriend {
+  static void FakeWritingSymbolIdInPreParseData(CompleteParserRecorder* log,
                                                 int number) {
     log->WriteNumber(number);
     if (log->symbol_id_ < number + 1) {
       log->symbol_id_ = number + 1;
     }
-}
+  }
+  static int symbol_position(CompleteParserRecorder* log) {
+    return log->symbol_store_.size();
+  }
+  static int symbol_ids(CompleteParserRecorder* log) {
+    return log->symbol_id_;
+  }
+  static int function_position(CompleteParserRecorder* log) {
+    return log->function_store_.size();
+  }
+};
 
 }
 }
@@ -380,15 +391,16 @@ TEST(StoringNumbersInPreParseData) {
   // Symbol IDs are split into chunks of 7 bits for storing. This is a
   // regression test for a bug where a symbol id was incorrectly stored if some
   // of the chunks in the middle were all zeros.
+  typedef i::CompleteParserRecorderFriend F;
   i::CompleteParserRecorder log;
   for (int i = 0; i < 18; ++i) {
-    FakeWritingSymbolIdInPreParseData(&log, 1 << i);
+    F::FakeWritingSymbolIdInPreParseData(&log, 1 << i);
   }
   for (int i = 1; i < 18; ++i) {
-    FakeWritingSymbolIdInPreParseData(&log, (1 << i) + 1);
+    F::FakeWritingSymbolIdInPreParseData(&log, (1 << i) + 1);
   }
   for (int i = 6; i < 18; ++i) {
-    FakeWritingSymbolIdInPreParseData(&log, (3 << i) + (5 << (i - 6)));
+    F::FakeWritingSymbolIdInPreParseData(&log, (3 << i) + (5 << (i - 6)));
   }
   i::Vector<unsigned> store = log.ExtractData();
   i::ScriptDataImpl script_data(store);
@@ -2008,6 +2020,7 @@ TEST(DontRegressPreParserDataSizes) {
   // Each function adds 5 elements to the preparse function data.
   const int kDataPerFunction = 5;
 
+  typedef i::CompleteParserRecorderFriend F;
   uintptr_t stack_limit = CcTest::i_isolate()->stack_guard()->real_climit();
   for (int i = 0; test_cases[i].program; i++) {
     const char* program = test_cases[i].program;
@@ -2023,21 +2036,22 @@ TEST(DontRegressPreParserDataSizes) {
     preparser.set_allow_natives_syntax(true);
     i::PreParser::PreParseResult result = preparser.PreParseProgram();
     CHECK_EQ(i::PreParser::kPreParseSuccess, result);
-    if (log.symbol_ids() != test_cases[i].symbols) {
+    if (F::symbol_ids(&log) != test_cases[i].symbols) {
       i::OS::Print(
           "Expected preparse data for program:\n"
           "\t%s\n"
           "to contain %d symbols, however, received %d symbols.\n",
-          program, test_cases[i].symbols, log.symbol_ids());
+          program, test_cases[i].symbols, F::symbol_ids(&log));
       CHECK(false);
     }
-    if (log.function_position() != test_cases[i].functions * kDataPerFunction) {
+    if (F::function_position(&log) !=
+          test_cases[i].functions * kDataPerFunction) {
       i::OS::Print(
           "Expected preparse data for program:\n"
           "\t%s\n"
           "to contain %d functions, however, received %d functions.\n",
           program, test_cases[i].functions,
-          log.function_position() / kDataPerFunction);
+          F::function_position(&log) / kDataPerFunction);
       CHECK(false);
     }
     i::ScriptDataImpl data(log.ExtractData());