Reordered function entries in PreParse data to be ordered by start position.

Also add skip to entry, to skip pre-data for the body of the function.
Preparser data is now only accessed linearly, in the same order it was created.

Review URL: http://codereview.chromium.org/3185026

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

src/parser.cc

@@ -867,11 +867,10 @@ class ParserLog BASE_EMBEDDED {
  public:
   virtual ~ParserLog() { }
 
-  // Records the occurrence of a function.  The returned object is
-  // only guaranteed to be valid until the next function has been
-  // logged.
+  // Records the occurrence of a function.
   virtual FunctionEntry LogFunction(int start) { return FunctionEntry(); }
-
+  // Return the current position in the function entry log.
+  virtual int position() { return 0; }
   virtual void LogError() { }
 };
 
@@ -913,30 +912,45 @@ class ParserRecorder: public ParserLog {
                           const char* message,
                           Vector<const char*> args);
   Vector<unsigned> ExtractData() {
-    return store_.ToVector();
+    int total_size = ScriptDataImpl::kHeaderSize + store_.size();
+    Vector<unsigned> data = Vector<unsigned>::New(total_size);
+    memcpy(data.start(), preamble_, sizeof(preamble_));
+    if (ScriptDataImpl::kHeaderSize < total_size) {
+      store_.WriteTo(data.SubVector(ScriptDataImpl::kHeaderSize, total_size));
+    }
+    return data;
   }
+  virtual int position() { return store_.size(); }
  private:
-  bool has_error_;
   Collector<unsigned> store_;
-  Vector<unsigned> preamble_;
+  unsigned preamble_[ScriptDataImpl::kHeaderSize];
+#ifdef DEBUG
+  int prev_start;
+#endif
 
-  Collector<unsigned>* store() { return &store_; }
+  bool has_error() {
+    return static_cast<bool>(preamble_[ScriptDataImpl::kHasErrorOffset]);
+  }
   void WriteString(Vector<const char> str);
 };
 
 
-FunctionEntry ScriptDataImpl::GetFunctionEnd(int start) {
-  if (nth(last_entry_).start_pos() > start) {
-    // If the last entry we looked up is higher than what we're
-    // looking for then it's useless and we reset it.
-    last_entry_ = 0;
-  }
-  for (int i = last_entry_; i < EntryCount(); i++) {
-    FunctionEntry entry = nth(i);
-    if (entry.start_pos() == start) {
-      last_entry_ = i;
-      return entry;
-    }
+void ScriptDataImpl::SkipFunctionEntry(int start) {
+  ASSERT(index_ + FunctionEntry::kSize <= store_.length());
+  ASSERT(static_cast<int>(store_[index_]) == start);
+  index_ += FunctionEntry::kSize;
+}
+
+
+FunctionEntry ScriptDataImpl::GetFunctionEntry(int start) {
+  // The current pre-data entry must be a FunctionEntry with the given
+  // start position.
+    if ((index_ + FunctionEntry::kSize <= store_.length())
+        && (static_cast<int>(store_[index_]) == start)) {
+    int index = index_;
+    index_ += FunctionEntry::kSize;
+    return FunctionEntry(store_.SubVector(index,
+                                          index + FunctionEntry::kSize));
   }
   return FunctionEntry();
 }
@@ -952,31 +966,23 @@ bool ScriptDataImpl::SanityCheck() {
 }
 
 
-int ScriptDataImpl::EntryCount() {
-  return (store_.length() - kHeaderSize) / FunctionEntry::kSize;
-}
-
-
-FunctionEntry ScriptDataImpl::nth(int n) {
-  int offset = kHeaderSize + n * FunctionEntry::kSize;
-  return FunctionEntry(Vector<unsigned>(store_.start() + offset,
-                                        FunctionEntry::kSize));
-}
-
-
 ParserRecorder::ParserRecorder()
-    : has_error_(false), store_(ScriptDataImpl::kHeaderSize) {
-  preamble_ = store()->AddBlock(ScriptDataImpl::kHeaderSize, 0);
+  : store_(0) {
+#ifdef DEBUG
+  prev_start = -1;
+#endif
   preamble_[ScriptDataImpl::kMagicOffset] = ScriptDataImpl::kMagicNumber;
   preamble_[ScriptDataImpl::kVersionOffset] = ScriptDataImpl::kCurrentVersion;
   preamble_[ScriptDataImpl::kHasErrorOffset] = false;
+  preamble_[ScriptDataImpl::kSizeOffset] = 0;
+  ASSERT_EQ(4, ScriptDataImpl::kHeaderSize);
 }
 
 
 void ParserRecorder::WriteString(Vector<const char> str) {
-  store()->Add(str.length());
+  store_.Add(str.length());
   for (int i = 0; i < str.length(); i++) {
-    store()->Add(str[i]);
+    store_.Add(str[i]);
   }
 }
 
@@ -995,15 +1001,12 @@ const char* ScriptDataImpl::ReadString(unsigned* start, int* chars) {
 
 void ParserRecorder::LogMessage(Scanner::Location loc, const char* message,
                                 Vector<const char*> args) {
-  if (has_error_) return;
-  store()->Reset();
-  preamble_ = store()->AddBlock(ScriptDataImpl::kHeaderSize, 0);
-  preamble_[ScriptDataImpl::kMagicOffset] = ScriptDataImpl::kMagicNumber;
-  preamble_[ScriptDataImpl::kVersionOffset] = ScriptDataImpl::kCurrentVersion;
+  if (has_error()) return;
   preamble_[ScriptDataImpl::kHasErrorOffset] = true;
-  store()->Add(loc.beg_pos);
-  store()->Add(loc.end_pos);
-  store()->Add(args.length());
+  store_.Reset();
+  store_.Add(loc.beg_pos);
+  store_.Add(loc.end_pos);
+  store_.Add(args.length());
   WriteString(CStrVector(message));
   for (int i = 0; i < args.length(); i++) {
     WriteString(CStrVector(args[i]));
@@ -1046,10 +1049,22 @@ unsigned* ScriptDataImpl::ReadAddress(int position) {
   return &store_[ScriptDataImpl::kHeaderSize + position];
 }
 
+void ScriptDataImpl::FindStart(int position) {
+  // Only search forwards, and linearly for now.
+  while ((index_ < store_.length())
+      && (static_cast<int>(store_[index_])) < position) {
+    index_ += FunctionEntry::kSize;
+  }
+}
+
 
 FunctionEntry ParserRecorder::LogFunction(int start) {
-  if (has_error_) return FunctionEntry();
-  FunctionEntry result(store()->AddBlock(FunctionEntry::kSize, 0));
+#ifdef DEBUG
+  ASSERT(start > prev_start);
+  prev_start = start;
+#endif
+  if (has_error()) return FunctionEntry();
+  FunctionEntry result(store_.AddBlock(FunctionEntry::kSize, 0));
   result.set_start_pos(start);
   return result;
 }
@@ -3939,43 +3954,52 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
     bool is_lazily_compiled =
         mode() == PARSE_LAZILY && top_scope_->HasTrivialOuterContext();
 
+    int function_block_pos = scanner_.location().beg_pos;
     int materialized_literal_count;
     int expected_property_count;
+    int end_pos;
     bool only_simple_this_property_assignments;
     Handle<FixedArray> this_property_assignments;
     if (is_lazily_compiled && pre_data() != NULL) {
-      FunctionEntry entry = pre_data()->GetFunctionEnd(start_pos);
+      FunctionEntry entry = pre_data()->GetFunctionEntry(function_block_pos);
       if (!entry.is_valid()) {
         ReportInvalidPreparseData(name, CHECK_OK);
       }
-      int end_pos = entry.end_pos();
-      if (end_pos <= start_pos) {
+      end_pos = entry.end_pos();
+      if (end_pos <= function_block_pos) {
         // End position greater than end of stream is safe, and hard to check.
         ReportInvalidPreparseData(name, CHECK_OK);
       }
-      Counters::total_preparse_skipped.Increment(end_pos - start_pos);
+      Counters::total_preparse_skipped.Increment(end_pos - function_block_pos);
       scanner_.SeekForward(end_pos);
+      pre_data()->Skip(entry.predata_skip());
       materialized_literal_count = entry.literal_count();
       expected_property_count = entry.property_count();
       only_simple_this_property_assignments = false;
       this_property_assignments = Factory::empty_fixed_array();
+      Expect(Token::RBRACE, CHECK_OK);
     } else {
+      if (pre_data() != NULL) {
+        // Skip pre-data entry for non-lazily compiled function.
+        pre_data()->SkipFunctionEntry(function_block_pos);
+      }
+      FunctionEntry entry = log()->LogFunction(function_block_pos);
+      int predata_position_before = log()->position();
       ParseSourceElements(&body, Token::RBRACE, CHECK_OK);
       materialized_literal_count = temp_scope.materialized_literal_count();
       expected_property_count = temp_scope.expected_property_count();
       only_simple_this_property_assignments =
           temp_scope.only_simple_this_property_assignments();
       this_property_assignments = temp_scope.this_property_assignments();
-    }
-
-    Expect(Token::RBRACE, CHECK_OK);
-    int end_pos = scanner_.location().end_pos;
 
-    FunctionEntry entry = log()->LogFunction(start_pos);
-    if (entry.is_valid()) {
-      entry.set_end_pos(end_pos);
-      entry.set_literal_count(materialized_literal_count);
-      entry.set_property_count(expected_property_count);
+      Expect(Token::RBRACE, CHECK_OK);
+      end_pos = scanner_.location().end_pos;
+      if (entry.is_valid()) {
+        entry.set_end_pos(end_pos);
+        entry.set_literal_count(materialized_literal_count);
+        entry.set_property_count(expected_property_count);
+        entry.set_predata_skip(log()->position() - predata_position_before);
+      }
     }
 
     FunctionLiteral* function_literal =

src/parser.h

@@ -68,11 +68,18 @@ class FunctionEntry BASE_EMBEDDED {
   void set_literal_count(int value) { backing_[kLiteralCountOffset] = value; }
 
   int property_count() { return backing_[kPropertyCountOffset]; }
-  void set_property_count(int value) { backing_[kPropertyCountOffset] = value; }
+  void set_property_count(int value) {
+    backing_[kPropertyCountOffset] = value;
+  }
+
+  int predata_skip() { return backing_[kPredataSkipOffset]; }
+  void set_predata_skip(int value) {
+    backing_[kPredataSkipOffset] = value;
+  }
 
   bool is_valid() { return backing_.length() > 0; }
 
-  static const int kSize = 4;
+  static const int kSize = 5;
 
  private:
   Vector<unsigned> backing_;
@@ -80,6 +87,7 @@ class FunctionEntry BASE_EMBEDDED {
   static const int kEndPosOffset = 1;
   static const int kLiteralCountOffset = 2;
   static const int kPropertyCountOffset = 3;
+  static const int kPredataSkipOffset = 4;
 };
 
 
@@ -87,12 +95,13 @@ class ScriptDataImpl : public ScriptData {
  public:
   explicit ScriptDataImpl(Vector<unsigned> store)
       : store_(store),
-        last_entry_(0) { }
+        index_(kHeaderSize) { }
   virtual ~ScriptDataImpl();
   virtual int Length();
   virtual const char* Data();
   virtual bool HasError();
-  FunctionEntry GetFunctionEnd(int start);
+  FunctionEntry GetFunctionEntry(int start);
+  void SkipFunctionEntry(int start);
   bool SanityCheck();
 
   Scanner::Location MessageLocation();
@@ -102,31 +111,33 @@ class ScriptDataImpl : public ScriptData {
   bool has_error() { return store_[kHasErrorOffset]; }
   unsigned magic() { return store_[kMagicOffset]; }
   unsigned version() { return store_[kVersionOffset]; }
+  // Skip forward in the preparser data by the given number
+  // of unsigned ints.
+  virtual void Skip(int entries) {
+    ASSERT(entries >= 0);
+    ASSERT(entries <= store_.length() - index_);
+    index_ += entries;
+  }
 
   static const unsigned kMagicNumber = 0xBadDead;
   static const unsigned kCurrentVersion = 1;
 
-  static const unsigned kMagicOffset = 0;
-  static const unsigned kVersionOffset = 1;
-  static const unsigned kHasErrorOffset = 2;
-  static const unsigned kSizeOffset = 3;
-  static const unsigned kHeaderSize = 4;
+  static const int kMagicOffset = 0;
+  static const int kVersionOffset = 1;
+  static const int kHasErrorOffset = 2;
+  static const int kSizeOffset = 3;
+  static const int kHeaderSize = 4;
 
  private:
+  Vector<unsigned> store_;
+  int index_;
+
   unsigned Read(int position);
   unsigned* ReadAddress(int position);
-  int EntryCount();
-  FunctionEntry nth(int n);
-
-  Vector<unsigned> store_;
 
+  void FindStart(int position);
   // Read strings written by ParserRecorder::WriteString.
   static const char* ReadString(unsigned* start, int* chars);
-
-  // The last entry returned.  This is used to make lookup faster:
-  // the next entry to return is typically the next entry so lookup
-  // will usually be much faster if we start from the last entry.
-  int last_entry_;
 };
 
 

src/scanner.h

@@ -77,7 +77,7 @@ class UTF8Buffer {
   static const char kEndMarker = '\x00';
  private:
   static const int kInitialCapacity = 256;
-  SequenceCollector<char> buffer_;
+  SequenceCollector<char, 4> buffer_;
 
   void AddCharSlow(uc32 c);
 };

src/utils.h

@@ -326,9 +326,9 @@ class Vector {
   // Returns a vector using the same backing storage as this one,
   // spanning from and including 'from', to but not including 'to'.
   Vector<T> SubVector(int from, int to) {
-    ASSERT(from < length_);
     ASSERT(to <= length_);
     ASSERT(from < to);
+    ASSERT(0 <= from);
     return Vector<T>(start() + from, to - from);
   }
 
@@ -485,24 +485,20 @@ inline Vector< Handle<Object> > HandleVector(v8::internal::Handle<T>* elms,
  * in memory). The collector may move elements unless it has guaranteed not
  * to.
  */
-template <typename T>
+template <typename T, int growth_factor = 2, int max_growth = 1 * MB>
 class Collector {
  public:
-  Collector(int initial_capacity = kMinCapacity,
-            int growth_factor = 2,
-            int max_growth = 1 * MB)
-      : growth_factor_(growth_factor), max_growth_(max_growth) {
+  explicit Collector(int initial_capacity = kMinCapacity)
+      : index_(0), size_(0) {
     if (initial_capacity < kMinCapacity) {
       initial_capacity = kMinCapacity;
     }
-    current_chunk_ = NewArray<T>(initial_capacity);
-    current_capacity_ = initial_capacity;
-    index_ = 0;
+    current_chunk_ = Vector<T>::New(initial_capacity);
   }
 
   virtual ~Collector() {
     // Free backing store (in reverse allocation order).
-    DeleteArray(current_chunk_);
+    current_chunk_.Dispose();
     for (int i = chunks_.length() - 1; i >= 0; i--) {
       chunks_.at(i).Dispose();
     }
@@ -510,11 +506,12 @@ class Collector {
 
   // Add a single element.
   inline void Add(T value) {
-    if (index_ >= current_capacity_) {
+    if (index_ >= current_chunk_.length()) {
       Grow(1);
     }
     current_chunk_[index_] = value;
     index_++;
+    size_++;
   }
 
   // Add a block of contiguous elements and return a Vector backed by the
@@ -522,11 +519,13 @@ class Collector {
   // A basic Collector will keep this vector valid as long as the Collector
   // is alive.
   inline Vector<T> AddBlock(int size, T initial_value) {
-    if (index_ + size > current_capacity_) {
+    ASSERT(size > 0);
+    if (size > current_chunk_.length() - index_) {
       Grow(size);
     }
-    T* position = current_chunk_ + index_;
+    T* position = current_chunk_.start() + index_;
     index_ += size;
+    size_ += size;
     for (int i = 0; i < size; i++) {
       position[i] = initial_value;
     }
@@ -534,30 +533,31 @@ class Collector {
   }
 
 
-  // Allocate a single contiguous vector, copy all the collected
-  // elements to the vector, and return it.
-  // The caller is responsible for freeing the memory of the returned
-  // vector (e.g., using Vector::Dispose).
-  Vector<T> ToVector() {
-    // Find the total length.
-    int total_length = index_;
-    for (int i = 0; i < chunks_.length(); i++) {
-      total_length += chunks_.at(i).length();
-    }
-    T* new_store = NewArray<T>(total_length);
+  // Write the contents of the collector into the provided vector.
+  void WriteTo(Vector<T> destination) {
+    ASSERT(size_ <= destination.length());
     int position = 0;
     for (int i = 0; i < chunks_.length(); i++) {
       Vector<T> chunk = chunks_.at(i);
       for (int j = 0; j < chunk.length(); j++) {
-        new_store[position] = chunk[j];
+        destination[position] = chunk[j];
         position++;
       }
     }
     for (int i = 0; i < index_; i++) {
-      new_store[position] = current_chunk_[i];
+      destination[position] = current_chunk_[i];
       position++;
     }
-    return Vector<T>(new_store, total_length);
+  }
+
+  // Allocate a single contiguous vector, copy all the collected
+  // elements to the vector, and return it.
+  // The caller is responsible for freeing the memory of the returned
+  // vector (e.g., using Vector::Dispose).
+  Vector<T> ToVector() {
+    Vector<T> new_store = Vector<T>::New(size_);
+    WriteTo(new_store);
+    return new_store;
   }
 
   // Resets the collector to be empty.
@@ -567,35 +567,42 @@ class Collector {
     }
     chunks_.Rewind(0);
     index_ = 0;
+    size_ = 0;
   }
 
+  // Total number of elements added to collector so far.
+  inline int size() { return size_; }
+
  protected:
   static const int kMinCapacity = 16;
   List<Vector<T> > chunks_;
-  T* current_chunk_;
-  int growth_factor_;
-  int max_growth_;
-  int current_capacity_;
-  int index_;
+  Vector<T> current_chunk_;  // Block of memory currently being written into.
+  int index_;  // Current index in current chunk.
+  int size_;  // Total number of elements in collector.
 
   // Creates a new current chunk, and stores the old chunk in the chunks_ list.
   void Grow(int min_capacity) {
-    ASSERT(growth_factor_ > 1);
-    int growth = current_capacity_ * (growth_factor_ - 1);
-    if (growth > max_growth_) {
-      growth = max_growth_;
+    ASSERT(growth_factor > 1);
+    int growth = current_chunk_.length() * (growth_factor - 1);
+    if (growth > max_growth) {
+      growth = max_growth;
     }
-    int new_capacity = current_capacity_ + growth;
+    int new_capacity = current_chunk_.length() + growth;
     if (new_capacity < min_capacity) {
-      new_capacity = min_capacity;
+      new_capacity = min_capacity + growth;
+    }
+    Vector<T> new_chunk = Vector<T>::New(new_capacity);
+    int new_index = PrepareGrow(new_chunk);
+    if (index_ > 0) {
+      chunks_.Add(current_chunk_.SubVector(0, index_));
+    } else {
+      // Can happen if the call to PrepareGrow moves everything into
+      // the new chunk.
+      current_chunk_.Dispose();
     }
-    T* new_chunk = NewArray<T>(new_capacity);
-    int new_index = PrepareGrow(Vector<T>(new_chunk, new_capacity));
-    chunks_.Add(Vector<T>(current_chunk_, index_));
     current_chunk_ = new_chunk;
-    current_capacity_ = new_capacity;
     index_ = new_index;
-    ASSERT(index_ + min_capacity <= current_capacity_);
+    ASSERT(index_ + min_capacity <= current_chunk_.length());
   }
 
   // Before replacing the current chunk, give a subclass the option to move
@@ -617,13 +624,11 @@ class Collector {
  * NOTICE: Blocks allocated using Collector::AddBlock(int) can move
  * as well, if inside an active sequence where another element is added.
  */
-template <typename T>
-class SequenceCollector : public Collector<T> {
+template <typename T, int growth_factor = 2, int max_growth = 1 * MB>
+class SequenceCollector : public Collector<T, growth_factor, max_growth> {
  public:
-  SequenceCollector(int initial_capacity,
-                    int growth_factor = 2,
-                    int max_growth = 1 * MB)
-      : Collector<T>(initial_capacity, growth_factor, max_growth),
+  explicit SequenceCollector(int initial_capacity)
+      : Collector<T, growth_factor, max_growth>(initial_capacity),
         sequence_start_(kNoSequence) { }
 
   virtual ~SequenceCollector() {}
@@ -637,20 +642,22 @@ class SequenceCollector : public Collector<T> {
     ASSERT(sequence_start_ != kNoSequence);
     int sequence_start = sequence_start_;
     sequence_start_ = kNoSequence;
-    return Vector<T>(this->current_chunk_ + sequence_start,
-                     this->index_ - sequence_start);
+    if (sequence_start == this->index_) return Vector<T>();
+    return this->current_chunk_.SubVector(sequence_start, this->index_);
   }
 
   // Drops the currently added sequence, and all collected elements in it.
   void DropSequence() {
     ASSERT(sequence_start_ != kNoSequence);
+    int sequence_length = this->index_ - sequence_start_;
     this->index_ = sequence_start_;
+    this->size_ -= sequence_length;
     sequence_start_ = kNoSequence;
   }
 
   virtual void Reset() {
     sequence_start_ = kNoSequence;
-    this->Collector<T>::Reset();
+    this->Collector<T, growth_factor, max_growth>::Reset();
   }
 
  private:

test/cctest/test-api.cc

@@ -8603,7 +8603,7 @@ TEST(PreCompileInvalidPreparseDataError) {
   // ScriptDataImpl private implementation details
   const int kUnsignedSize = sizeof(unsigned);
   const int kHeaderSize = 4;
-  const int kFunctionEntrySize = 4;
+  const int kFunctionEntrySize = 5;
   const int kFunctionEntryStartOffset = 0;
   const int kFunctionEntryEndOffset = 1;
   unsigned* sd_data =
@@ -8625,6 +8625,8 @@ TEST(PreCompileInvalidPreparseDataError) {
   try_catch.Reset();
   // Overwrite function bar's start position with 200.  The function entry
   // will not be found when searching for it by position.
+  sd = v8::ScriptData::PreCompile(script, i::StrLength(script));
+  sd_data = reinterpret_cast<unsigned*>(const_cast<char*>(sd->Data()));
   sd_data[kHeaderSize + 1 * kFunctionEntrySize + kFunctionEntryStartOffset] =
       200;
   compiled_script = Script::New(source, NULL, sd);